diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 1a58a0054f..f21b2b5de6 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -43,7 +43,7 @@ use uuid::Uuid; // Keep ambiguous attribution bounded before adding every candidate to model-visible context. const MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_CANDIDATES: usize = 16; // Leave room in the 10K-token action item for network metadata and JSON formatting. -const MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_TOKENS: u64 = 8_000; +const MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_TOKENS: u64 = 6_000; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum NetworkApprovalMode { @@ -605,22 +605,30 @@ impl NetworkApprovalService { } } let use_guardian = routes_approval_to_guardian(&turn_context); - let possible_triggers = possible_trigger_calls - .iter() - .map(|call| call.trigger.clone()) - .collect::>(); - if use_guardian { - let candidates_fit = possible_triggers.len() - <= MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_CANDIDATES - && serde_json::to_vec_pretty(&possible_triggers).is_ok_and(|encoded| { - approx_tokens_from_byte_count(encoded.len()) - <= MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_TOKENS - }); - if !candidates_fit { - pending.set_decision(PendingApprovalDecision::Deny).await; - self.pending_host_approvals.lock().await.remove(&key); - return NetworkDecision::deny(REASON_NOT_ALLOWED); - } + if use_guardian + && possible_trigger_calls.len() > MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_CANDIDATES + { + pending.set_decision(PendingApprovalDecision::Deny).await; + self.pending_host_approvals.lock().await.remove(&key); + return NetworkDecision::deny(REASON_NOT_ALLOWED); + } + let possible_triggers = if use_guardian { + possible_trigger_calls + .iter() + .map(|call| call.trigger.clone()) + .collect::>() + } else { + Vec::new() + }; + if use_guardian + && !serde_json::to_vec_pretty(&possible_triggers).is_ok_and(|encoded| { + approx_tokens_from_byte_count(encoded.len()) + <= MAX_GUARDIAN_NETWORK_ACCESS_TRIGGER_TOKENS + }) + { + pending.set_decision(PendingApprovalDecision::Deny).await; + self.pending_host_approvals.lock().await.remove(&key); + return NetworkDecision::deny(REASON_NOT_ALLOWED); } let guardian_review_id = use_guardian.then(new_guardian_review_id); let approval_decision = if let Some(review_id) = guardian_review_id.clone() { diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 0385ecb2b4..e6f57fb6e4 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -349,34 +349,10 @@ async fn active_call_preserves_triggering_command_context() { } #[tokio::test] -async fn multiple_active_calls_in_the_same_environment_return_all_candidates() { +async fn active_call_attribution_returns_all_matching_environment_candidates() { let service = NetworkApprovalService::default(); register_call_with_default_shell_trigger(&service, "registration-1").await; register_call_with_default_shell_trigger(&service, "registration-2").await; - - match service - .resolve_active_call_attribution(ActiveNetworkApprovalCallScope::Environment("local")) - .await - { - ActiveNetworkApprovalAttribution::Ambiguous(calls) => { - assert_eq!( - calls - .iter() - .map(|call| call.registration_id.as_str()) - .collect::>(), - vec!["registration-1", "registration-2"] - ); - } - ActiveNetworkApprovalAttribution::None | ActiveNetworkApprovalAttribution::Single(_) => { - panic!("multiple active calls should return every candidate") - } - } -} - -#[tokio::test] -async fn active_call_attribution_is_filtered_by_environment() { - let service = NetworkApprovalService::default(); - register_call_with_default_shell_trigger(&service, "registration-local").await; service .register_call( "registration-remote".to_string(), @@ -398,14 +374,20 @@ async fn active_call_attribution_is_filtered_by_environment() { .await; match service - .resolve_active_call_attribution(ActiveNetworkApprovalCallScope::Environment("remote")) + .resolve_active_call_attribution(ActiveNetworkApprovalCallScope::Environment("local")) .await { - ActiveNetworkApprovalAttribution::Single(call) => { - assert_eq!(call.registration_id, "registration-remote"); + ActiveNetworkApprovalAttribution::Ambiguous(calls) => { + assert_eq!( + calls + .iter() + .map(|call| call.registration_id.as_str()) + .collect::>(), + vec!["registration-1", "registration-2"] + ); } - ActiveNetworkApprovalAttribution::None | ActiveNetworkApprovalAttribution::Ambiguous(_) => { - panic!("one matching environment call should resolve exactly") + ActiveNetworkApprovalAttribution::None | ActiveNetworkApprovalAttribution::Single(_) => { + panic!("multiple matching calls should return every candidate") } } }