From 84f4e7b39d17fea6d28c98bc748652ea4b279a14 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Tue, 17 Mar 2026 23:42:26 -0700 Subject: [PATCH] fix(subagents) share execpolicy by default (#13702) ## Summary If a subagent requests approval, and the user persists that approval to the execpolicy, it should (by default) propagate. We'll need to rethink this a bit in light of coming Permissions changes, though I think this is closer to the end state that we'd want, which is that execpolicy changes to one permissions profile should be synced across threads. ## Testing - [x] Added integration test --------- Co-authored-by: Codex --- codex-rs/core/src/agent/control.rs | 33 ++++ codex-rs/core/src/codex.rs | 16 +- codex-rs/core/src/codex_delegate.rs | 1 + codex-rs/core/src/codex_tests.rs | 6 +- codex-rs/core/src/codex_tests_guardian.rs | 1 + codex-rs/core/src/exec_policy.rs | 48 ++++- codex-rs/core/src/exec_policy_tests.rs | 94 +++++++++ codex-rs/core/src/state/service.rs | 2 +- codex-rs/core/src/thread_manager.rs | 12 ++ codex-rs/core/tests/suite/approvals.rs | 227 ++++++++++++++++++++++ 10 files changed, 427 insertions(+), 13 deletions(-) diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 83e6a3a04..eaee6e985 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -107,6 +107,9 @@ impl AgentControl { let inherited_shell_snapshot = self .inherited_shell_snapshot_for_source(&state, session_source.as_ref()) .await; + let inherited_exec_policy = self + .inherited_exec_policy_for_source(&state, session_source.as_ref(), &config) + .await; let session_source = match session_source { Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, @@ -189,6 +192,7 @@ impl AgentControl { session_source, /*persist_extended_history*/ false, inherited_shell_snapshot, + inherited_exec_policy, ) .await? } else { @@ -200,6 +204,7 @@ impl AgentControl { /*persist_extended_history*/ false, /*metrics_service_name*/ None, inherited_shell_snapshot, + inherited_exec_policy, ) .await? } @@ -271,6 +276,9 @@ impl AgentControl { let inherited_shell_snapshot = self .inherited_shell_snapshot_for_source(&state, Some(&session_source)) .await; + let inherited_exec_policy = self + .inherited_exec_policy_for_source(&state, Some(&session_source), &config) + .await; let rollout_path = find_thread_path_by_id_str(config.codex_home.as_path(), &thread_id.to_string()) .await? @@ -283,6 +291,7 @@ impl AgentControl { self.clone(), session_source, inherited_shell_snapshot, + inherited_exec_policy, ) .await?; reservation.commit(resumed_thread.thread_id); @@ -499,6 +508,30 @@ impl AgentControl { let parent_thread = state.get_thread(*parent_thread_id).await.ok()?; parent_thread.codex.session.user_shell().shell_snapshot() } + + async fn inherited_exec_policy_for_source( + &self, + state: &Arc, + session_source: Option<&SessionSource>, + child_config: &crate::config::Config, + ) -> Option> { + let Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id, .. + })) = session_source + else { + return None; + }; + + let parent_thread = state.get_thread(*parent_thread_id).await.ok()?; + let parent_config = parent_thread.codex.session.get_config().await; + if !crate::exec_policy::child_uses_parent_exec_policy(&parent_config, child_config) { + return None; + } + + Some(Arc::clone( + &parent_thread.codex.session.services.exec_policy, + )) + } } #[cfg(test)] #[path = "control_tests.rs"] diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5b936ee53..f38bf6d2f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -376,6 +376,7 @@ pub(crate) struct CodexSpawnArgs { pub(crate) persist_extended_history: bool, pub(crate) metrics_service_name: Option, pub(crate) inherited_shell_snapshot: Option>, + pub(crate) inherited_exec_policy: Option>, pub(crate) user_shell_override: Option, pub(crate) parent_trace: Option, } @@ -429,6 +430,7 @@ impl Codex { metrics_service_name, inherited_shell_snapshot, user_shell_override, + inherited_exec_policy, parent_trace: _, } = args; let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); @@ -485,11 +487,15 @@ impl Codex { // Guardian review should rely on the built-in shell safety checks, // not on caller-provided exec-policy rules that could shape the // reviewer or silently auto-approve commands. - ExecPolicyManager::default() + Arc::new(ExecPolicyManager::default()) + } else if let Some(exec_policy) = &inherited_exec_policy { + Arc::clone(exec_policy) } else { - ExecPolicyManager::load(&config.config_layer_stack) - .await - .map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))? + Arc::new( + ExecPolicyManager::load(&config.config_layer_stack) + .await + .map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?, + ) }; let config = Arc::new(config); @@ -1386,7 +1392,7 @@ impl Session { config: Arc, auth_manager: Arc, models_manager: Arc, - exec_policy: ExecPolicyManager, + exec_policy: Arc, tx_event: Sender, agent_status: watch::Sender, initial_history: InitialHistory, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 4369b81df..e560cd9c7 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -89,6 +89,7 @@ pub(crate) async fn run_codex_thread_interactive( metrics_service_name: None, inherited_shell_snapshot: None, user_shell_override: None, + inherited_exec_policy: Some(Arc::clone(&parent_session.services.exec_policy)), parent_trace: None, }) .await?; diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index c4c1e929d..f767c05f4 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2364,7 +2364,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { Arc::clone(&config), auth_manager, models_manager, - ExecPolicyManager::default(), + Arc::new(ExecPolicyManager::default()), tx_event, agent_status_tx, InitialHistory::New, @@ -2400,7 +2400,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { CollaborationModesConfig::default(), )); let agent_control = AgentControl::default(); - let exec_policy = ExecPolicyManager::default(); + let exec_policy = Arc::new(ExecPolicyManager::default()); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let model = ModelsManager::get_model_offline_for_tests(config.model.as_deref()); let model_info = ModelsManager::construct_model_info_offline_for_tests(model.as_str(), &config); @@ -3194,7 +3194,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( CollaborationModesConfig::default(), )); let agent_control = AgentControl::default(); - let exec_policy = ExecPolicyManager::default(); + let exec_policy = Arc::new(ExecPolicyManager::default()); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let model = ModelsManager::get_model_offline_for_tests(config.model.as_deref()); let model_info = ModelsManager::construct_model_info_offline_for_tests(model.as_str(), &config); diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index 20f09e759..677456ab4 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -452,6 +452,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { persist_extended_history: false, metrics_service_name: None, inherited_shell_snapshot: None, + inherited_exec_policy: Some(Arc::new(parent_exec_policy)), user_shell_override: None, parent_trace: None, }) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 49507585b..0c95af4c0 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -32,8 +32,10 @@ use tracing::instrument; use crate::bash::parse_shell_lc_plain_commands; use crate::bash::parse_shell_lc_single_command_prefix; +use crate::config::Config; use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ExecApprovalRequirement; +use codex_utils_absolute_path::AbsolutePathBuf; use shlex::try_join as shlex_try_join; const PROMPT_CONFLICT_REASON: &str = @@ -94,6 +96,24 @@ static BANNED_PREFIX_SUGGESTIONS: &[&[&str]] = &[ &["osascript"], ]; +pub(crate) fn child_uses_parent_exec_policy(parent_config: &Config, child_config: &Config) -> bool { + fn exec_policy_config_folders(config: &Config) -> Vec { + config + .config_layer_stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ false, + ) + .into_iter() + .filter_map(codex_config::ConfigLayerEntry::config_folder) + .collect() + } + + exec_policy_config_folders(parent_config) == exec_policy_config_folders(child_config) + && parent_config.config_layer_stack.requirements().exec_policy + == child_config.config_layer_stack.requirements().exec_policy +} + fn is_policy_match(rule_match: &RuleMatch) -> bool { match rule_match { RuleMatch::PrefixRuleMatch { .. } => true, @@ -170,6 +190,7 @@ pub enum ExecPolicyUpdateError { pub(crate) struct ExecPolicyManager { policy: ArcSwap, + update_lock: tokio::sync::Mutex<()>, } pub(crate) struct ExecApprovalRequest<'a> { @@ -185,6 +206,7 @@ impl ExecPolicyManager { pub(crate) fn new(policy: Arc) -> Self { Self { policy: ArcSwap::from(policy), + update_lock: tokio::sync::Mutex::new(()), } } @@ -292,11 +314,11 @@ impl ExecPolicyManager { codex_home: &Path, amendment: &ExecPolicyAmendment, ) -> Result<(), ExecPolicyUpdateError> { + let _update_guard = self.update_lock.lock().await; let policy_path = default_policy_path(codex_home); - let prefix = amendment.command.clone(); spawn_blocking({ let policy_path = policy_path.clone(); - let prefix = prefix.clone(); + let prefix = amendment.command.clone(); move || blocking_append_allow_prefix_rule(&policy_path, &prefix) }) .await @@ -306,8 +328,25 @@ impl ExecPolicyManager { source, })?; - let mut updated_policy = self.current().as_ref().clone(); - updated_policy.add_prefix_rule(&prefix, Decision::Allow)?; + let current_policy = self.current(); + let match_options = MatchOptions { + resolve_host_executables: true, + }; + let existing_evaluation = current_policy.check_multiple_with_options( + [&amendment.command], + &|_| Decision::Forbidden, + &match_options, + ); + let already_allowed = existing_evaluation.decision == Decision::Allow + && existing_evaluation.matched_rules.iter().any(|rule_match| { + is_policy_match(rule_match) && rule_match.decision() == Decision::Allow + }); + if already_allowed { + return Ok(()); + } + + let mut updated_policy = current_policy.as_ref().clone(); + updated_policy.add_prefix_rule(&amendment.command, Decision::Allow)?; self.policy.store(Arc::new(updated_policy)); Ok(()) } @@ -320,6 +359,7 @@ impl ExecPolicyManager { decision: Decision, justification: Option, ) -> Result<(), ExecPolicyUpdateError> { + let _update_guard = self.update_lock.lock().await; let policy_path = default_policy_path(codex_home); let host = host.to_string(); spawn_blocking({ diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index aaf098951..fd3fe05e1 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -1,9 +1,16 @@ use super::*; +use crate::config::Config; +use crate::config::ConfigBuilder; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; +use crate::config_loader::ConfigLayerStackOrdering; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; +use crate::config_loader::LoaderOverrides; +use crate::config_loader::RequirementSource; +use crate::config_loader::Sourced; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::RequirementsExecPolicy; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -17,6 +24,7 @@ use std::fs; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use tempfile::TempDir; use tempfile::tempdir; use toml::Value as TomlValue; @@ -73,6 +81,92 @@ fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy { FileSystemSandboxPolicy::unrestricted() } +async fn test_config() -> (TempDir, Config) { + let home = TempDir::new().expect("create temp dir"); + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .loader_overrides(LoaderOverrides { + #[cfg(target_os = "macos")] + managed_preferences_base64: Some(String::new()), + macos_managed_config_requirements_base64: Some(String::new()), + ..LoaderOverrides::default() + }) + .build() + .await + .expect("load default test config"); + (home, config) +} + +#[tokio::test] +async fn child_uses_parent_exec_policy_when_layer_stack_matches() { + let (_home, parent_config) = test_config().await; + let child_config = parent_config.clone(); + + assert!(child_uses_parent_exec_policy(&parent_config, &child_config)); +} + +#[tokio::test] +async fn child_uses_parent_exec_policy_when_non_exec_policy_layers_differ() { + let (_home, parent_config) = test_config().await; + let mut child_config = parent_config.clone(); + let mut layers: Vec<_> = child_config + .config_layer_stack + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + .into_iter() + .cloned() + .collect(); + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + TomlValue::Table(Default::default()), + )); + child_config.config_layer_stack = ConfigLayerStack::new( + layers, + child_config.config_layer_stack.requirements().clone(), + child_config.config_layer_stack.requirements_toml().clone(), + ) + .expect("config layer stack"); + + assert!(child_uses_parent_exec_policy(&parent_config, &child_config)); +} + +#[tokio::test] +async fn child_does_not_use_parent_exec_policy_when_requirements_exec_policy_differs() { + let (_home, parent_config) = test_config().await; + let mut child_config = parent_config.clone(); + let mut requirements = ConfigRequirements { + exec_policy: child_config + .config_layer_stack + .requirements() + .exec_policy + .clone(), + ..ConfigRequirements::default() + }; + let mut policy = Policy::empty(); + policy + .add_prefix_rule(&["rm".to_string()], Decision::Forbidden) + .expect("add prefix rule"); + requirements.exec_policy = Some(Sourced::new( + RequirementsExecPolicy::new(policy), + RequirementSource::Unknown, + )); + child_config.config_layer_stack = ConfigLayerStack::new( + child_config + .config_layer_stack + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + .into_iter() + .cloned() + .collect(), + requirements, + child_config.config_layer_stack.requirements_toml().clone(), + ) + .expect("config layer stack"); + + assert!(!child_uses_parent_exec_policy( + &parent_config, + &child_config + )); +} + #[tokio::test] async fn returns_empty_policy_when_no_policy_files_exist() { let temp_dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 1a3f58d0f..0bd13870c 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -44,7 +44,7 @@ pub(crate) struct SessionServices { pub(crate) user_shell: Arc, pub(crate) shell_snapshot_tx: watch::Sender>>, pub(crate) show_raw_agent_reasoning: bool, - pub(crate) exec_policy: ExecPolicyManager, + pub(crate) exec_policy: Arc, pub(crate) auth_manager: Arc, pub(crate) models_manager: Arc, pub(crate) session_telemetry: SessionTelemetry, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 58b3e30c0..f9f887523 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -610,10 +610,12 @@ impl ThreadManagerState { /*persist_extended_history*/ false, /*metrics_service_name*/ None, /*inherited_shell_snapshot*/ None, + /*inherited_exec_policy*/ None, )) .await } + #[allow(clippy::too_many_arguments)] pub(crate) async fn spawn_new_thread_with_source( &self, config: Config, @@ -622,6 +624,7 @@ impl ThreadManagerState { persist_extended_history: bool, metrics_service_name: Option, inherited_shell_snapshot: Option>, + inherited_exec_policy: Option>, ) -> CodexResult { Box::pin(self.spawn_thread_with_source( config, @@ -633,6 +636,7 @@ impl ThreadManagerState { persist_extended_history, metrics_service_name, inherited_shell_snapshot, + inherited_exec_policy, /*parent_trace*/ None, /*user_shell_override*/ None, )) @@ -646,6 +650,7 @@ impl ThreadManagerState { agent_control: AgentControl, session_source: SessionSource, inherited_shell_snapshot: Option>, + inherited_exec_policy: Option>, ) -> CodexResult { let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?; Box::pin(self.spawn_thread_with_source( @@ -658,12 +663,14 @@ impl ThreadManagerState { /*persist_extended_history*/ false, /*metrics_service_name*/ None, inherited_shell_snapshot, + inherited_exec_policy, /*parent_trace*/ None, /*user_shell_override*/ None, )) .await } + #[allow(clippy::too_many_arguments)] pub(crate) async fn fork_thread_with_source( &self, config: Config, @@ -672,6 +679,7 @@ impl ThreadManagerState { session_source: SessionSource, persist_extended_history: bool, inherited_shell_snapshot: Option>, + inherited_exec_policy: Option>, ) -> CodexResult { Box::pin(self.spawn_thread_with_source( config, @@ -683,6 +691,7 @@ impl ThreadManagerState { persist_extended_history, /*metrics_service_name*/ None, inherited_shell_snapshot, + inherited_exec_policy, /*parent_trace*/ None, /*user_shell_override*/ None, )) @@ -713,6 +722,7 @@ impl ThreadManagerState { persist_extended_history, metrics_service_name, /*inherited_shell_snapshot*/ None, + /*inherited_exec_policy*/ None, parent_trace, user_shell_override, )) @@ -731,6 +741,7 @@ impl ThreadManagerState { persist_extended_history: bool, metrics_service_name: Option, inherited_shell_snapshot: Option>, + inherited_exec_policy: Option>, parent_trace: Option, user_shell_override: Option, ) -> CodexResult { @@ -754,6 +765,7 @@ impl ThreadManagerState { persist_extended_history, metrics_service_name, inherited_shell_snapshot, + inherited_exec_policy, user_shell_override, parent_trace, }) diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 53978c176..49b9ac59d 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used, clippy::expect_used)] use anyhow::Result; +use codex_core::CodexThread; use codex_core::config::Constrained; use codex_core::config_loader::ConfigLayerStack; use codex_core::config_loader::ConfigLayerStackOrdering; @@ -28,6 +29,7 @@ use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; +use core_test_support::responses::mount_sse_once_match; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; @@ -46,9 +48,11 @@ use std::env; use std::fs; use std::path::PathBuf; use std::sync::Arc; +use std::time::Duration; use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; +use wiremock::Request; use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; @@ -681,6 +685,47 @@ async fn wait_for_completion(test: &TestCodex) { .await; } +fn body_contains(req: &Request, text: &str) -> bool { + let is_zstd = req + .headers + .get("content-encoding") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| { + value + .split(',') + .any(|entry| entry.trim().eq_ignore_ascii_case("zstd")) + }); + let bytes = if is_zstd { + zstd::stream::decode_all(std::io::Cursor::new(&req.body)).ok() + } else { + Some(req.body.clone()) + }; + bytes + .and_then(|body| String::from_utf8(body).ok()) + .is_some_and(|body| body.contains(text)) +} + +async fn wait_for_spawned_thread(test: &TestCodex) -> Result> { + let deadline = tokio::time::Instant::now() + Duration::from_secs(2); + loop { + let ids = test.thread_manager.list_thread_ids().await; + if let Some(thread_id) = ids + .iter() + .find(|id| **id != test.session_configured.session_id) + { + return test + .thread_manager + .get_thread(*thread_id) + .await + .map_err(anyhow::Error::from); + } + if tokio::time::Instant::now() >= deadline { + anyhow::bail!("timed out waiting for spawned thread"); + } + tokio::time::sleep(Duration::from_millis(10)).await; + } +} + fn scenarios() -> Vec { use AskForApproval::*; @@ -1996,6 +2041,188 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn spawned_subagent_execpolicy_amendment_propagates_to_parent_session() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let approval_policy = AskForApproval::UnlessTrusted; + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let sandbox_policy_for_config = sandbox_policy.clone(); + let mut builder = test_codex().with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + config + .features + .enable(Feature::Collab) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + const PARENT_PROMPT: &str = "spawn a child that repeats a command"; + const CHILD_PROMPT: &str = "run the same command twice"; + const SPAWN_CALL_ID: &str = "spawn-child-1"; + const CHILD_CALL_ID_1: &str = "child-touch-1"; + const PARENT_CALL_ID_2: &str = "parent-touch-2"; + + let child_file = test.cwd.path().join("subagent-allow-prefix.txt"); + let _ = fs::remove_file(&child_file); + + let spawn_args = serde_json::to_string(&json!({ + "message": CHILD_PROMPT, + }))?; + mount_sse_once_match( + &server, + |req: &Request| body_contains(req, PARENT_PROMPT), + sse(vec![ + ev_response_created("resp-parent-1"), + ev_function_call(SPAWN_CALL_ID, "spawn_agent", &spawn_args), + ev_completed("resp-parent-1"), + ]), + ) + .await; + + let child_cmd_args = serde_json::to_string(&json!({ + "command": "touch subagent-allow-prefix.txt", + "timeout_ms": 1_000, + "prefix_rule": ["touch", "subagent-allow-prefix.txt"], + }))?; + mount_sse_once_match( + &server, + |req: &Request| body_contains(req, CHILD_PROMPT) && !body_contains(req, SPAWN_CALL_ID), + sse(vec![ + ev_response_created("resp-child-1"), + ev_function_call(CHILD_CALL_ID_1, "shell_command", &child_cmd_args), + ev_completed("resp-child-1"), + ]), + ) + .await; + + mount_sse_once_match( + &server, + |req: &Request| body_contains(req, CHILD_CALL_ID_1), + sse(vec![ + ev_response_created("resp-child-2"), + ev_assistant_message("msg-child-2", "child done"), + ev_completed("resp-child-2"), + ]), + ) + .await; + + mount_sse_once_match( + &server, + |req: &Request| body_contains(req, SPAWN_CALL_ID), + sse(vec![ + ev_response_created("resp-parent-2"), + ev_assistant_message("msg-parent-2", "parent done"), + ev_completed("resp-parent-2"), + ]), + ) + .await; + + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-parent-3"), + ev_function_call(PARENT_CALL_ID_2, "shell_command", &child_cmd_args), + ev_completed("resp-parent-3"), + ]), + ) + .await; + + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-parent-4"), + ev_assistant_message("msg-parent-4", "parent rerun done"), + ev_completed("resp-parent-4"), + ]), + ) + .await; + + submit_turn( + &test, + PARENT_PROMPT, + approval_policy, + sandbox_policy.clone(), + ) + .await?; + + let child = wait_for_spawned_thread(&test).await?; + let approval_event = wait_for_event_with_timeout( + &child, + |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }, + Duration::from_secs(2), + ) + .await; + + let EventMsg::ExecApprovalRequest(approval) = approval_event else { + panic!("expected child approval before completion"); + }; + let expected_execpolicy_amendment = ExecPolicyAmendment::new(vec![ + "touch".to_string(), + "subagent-allow-prefix.txt".to_string(), + ]); + assert_eq!( + approval.proposed_execpolicy_amendment, + Some(expected_execpolicy_amendment.clone()) + ); + + child + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::ApprovedExecpolicyAmendment { + proposed_execpolicy_amendment: expected_execpolicy_amendment, + }, + }) + .await?; + + let child_event = wait_for_event_with_timeout( + &child, + |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }, + Duration::from_secs(2), + ) + .await; + match child_event { + EventMsg::TurnComplete(_) => {} + EventMsg::ExecApprovalRequest(ev) => { + panic!("unexpected second child approval request: {:?}", ev.command) + } + other => panic!("unexpected event: {other:?}"), + } + assert!( + child_file.exists(), + "expected subagent command to create file" + ); + fs::remove_file(&child_file)?; + assert!( + !child_file.exists(), + "expected child file to be removed before parent rerun" + ); + + submit_turn( + &test, + "parent reruns child command", + approval_policy, + sandbox_policy, + ) + .await?; + wait_for_completion_without_approval(&test).await; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[cfg(unix)] async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> {