Stabilize guardian approval coverage (#14103)
## Summary - align the guardian permission test with the actual sandbox policy it widens and use a slightly larger Windows-only timeout budget - expose the additional-permissions normalization helper to the guardian test module - replace the guardian popup snapshot assertion with targeted string assertions ## Why this fixes the flake This group was carrying two separate sources of drift. The guardian core test widened derived sandbox policies without updating the source sandbox policy, and it used a Windows command/timeout combination that was too tight on slower runners. Separately, the TUI test was snapshotting the full popup even though unrelated feature text changes were the only thing moving. The new assertions keep coverage on the guardian entry itself while removing unrelated snapshot churn.
This commit is contained in:
Ahmed Ibrahim
GitHub
parent
ad57505ef5
commit
e03e9b63ea
5 changed files with 33 additions and 65 deletions
|
|
@ -16,6 +16,8 @@ use codex_execpolicy::RuleMatch;
|
|||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use core_test_support::codex_linux_sandbox_exe_or_skip;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
|
|
@ -70,15 +72,17 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
|||
.features
|
||||
.enable(Feature::RequestPermissions)
|
||||
.expect("test setup should allow enabling request permissions");
|
||||
turn_context_raw
|
||||
.sandbox_policy
|
||||
.set(SandboxPolicy::DangerFullAccess)
|
||||
.expect("test setup should allow updating sandbox policy");
|
||||
// This test is about request-permissions validation, not managed sandbox
|
||||
// policy enforcement. Widen the derived sandbox policies directly so the
|
||||
// command runs without depending on a platform sandbox binary.
|
||||
turn_context_raw.file_system_sandbox_policy =
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::from(
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
);
|
||||
FileSystemSandboxPolicy::from(turn_context_raw.sandbox_policy.get());
|
||||
turn_context_raw.network_sandbox_policy =
|
||||
codex_protocol::permissions::NetworkSandboxPolicy::from(&SandboxPolicy::DangerFullAccess);
|
||||
NetworkSandboxPolicy::from(turn_context_raw.sandbox_policy.get());
|
||||
let mut config = (*turn_context_raw.config).clone();
|
||||
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
|
||||
let config = Arc::new(config);
|
||||
|
|
@ -92,11 +96,14 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
|||
turn_context_raw.provider = config.model_provider.clone();
|
||||
let session = Arc::new(session);
|
||||
let turn_context = Arc::new(turn_context_raw);
|
||||
let expiration_ms: u64 = if cfg!(windows) { 2_500 } else { 1_000 };
|
||||
|
||||
let params = ExecParams {
|
||||
command: if cfg!(windows) {
|
||||
vec![
|
||||
"cmd.exe".to_string(),
|
||||
"/Q".to_string(),
|
||||
"/D".to_string(),
|
||||
"/C".to_string(),
|
||||
"echo hi".to_string(),
|
||||
]
|
||||
|
|
@ -108,7 +115,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
|||
]
|
||||
},
|
||||
cwd: turn_context.cwd.clone(),
|
||||
expiration: 1000.into(),
|
||||
expiration: expiration_ms.into(),
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ fn resolve_workdir_base_path(
|
|||
|
||||
/// Validates feature/policy constraints for `with_additional_permissions` and
|
||||
/// normalizes any path-based permissions. Errors if the request is invalid.
|
||||
pub(super) fn normalize_and_validate_additional_permissions(
|
||||
pub(crate) fn normalize_and_validate_additional_permissions(
|
||||
request_permission_enabled: bool,
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
|
|
|
|||
|
|
@ -1,19 +0,0 @@
|
|||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: popup
|
||||
---
|
||||
Experimental features
|
||||
Toggle experimental features. Changes are saved to config.toml.
|
||||
|
||||
› [ ] JavaScript REPL Enable a persistent Node-backed JavaScript REPL for interactive website debugging
|
||||
and other inline JavaScript execution capabilities. Requires Node >= v22.22.0
|
||||
installed.
|
||||
[ ] Multi-agents Ask Codex to spawn multiple agents to parallelize the work and win in efficiency.
|
||||
[ ] Apps Use a connected ChatGPT App using "$". Install Apps via /apps command. Restart
|
||||
Codex after enabling.
|
||||
[ ] Automatic approval review Dispatch `on-request` approval prompts (for e.g. sandbox escapes or blocked network
|
||||
access) to a carefully-prompted security reviewer subagent rather than blocking the
|
||||
agent on your input.
|
||||
[ ] Prevent sleep while running Keep your computer awake while Codex is running a thread.
|
||||
|
||||
Press space to select or enter to save for next conversation
|
||||
|
|
@ -1,20 +0,0 @@
|
|||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: popup
|
||||
---
|
||||
Experimental features
|
||||
Toggle experimental features. Changes are saved to config.toml.
|
||||
|
||||
› [ ] JavaScript REPL Enable a persistent Node-backed JavaScript REPL for interactive website debugging
|
||||
and other inline JavaScript execution capabilities. Requires Node >= v22.22.0
|
||||
installed.
|
||||
[ ] Bubblewrap sandbox Try the new linux sandbox based on bubblewrap.
|
||||
[ ] Multi-agents Ask Codex to spawn multiple agents to parallelize the work and win in efficiency.
|
||||
[ ] Apps Use a connected ChatGPT App using "$". Install Apps via /apps command. Restart
|
||||
Codex after enabling.
|
||||
[ ] Automatic approval review Dispatch `on-request` approval prompts (for e.g. sandbox escapes or blocked network
|
||||
access) to a carefully-prompted security reviewer subagent rather than blocking the
|
||||
agent on your input.
|
||||
[ ] Prevent sleep while running Keep your computer awake while Codex is running a thread.
|
||||
|
||||
Press space to select or enter to save for next conversation
|
||||
|
|
@ -47,7 +47,6 @@ use codex_protocol::items::PlanItem;
|
|||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
|
|
@ -99,7 +98,6 @@ use codex_protocol::protocol::UndoCompletedEvent;
|
|||
use codex_protocol::protocol::UndoStartedEvent;
|
||||
use codex_protocol::protocol::ViewImageToolCallEvent;
|
||||
use codex_protocol::protocol::WarningEvent;
|
||||
use codex_protocol::request_permissions::RequestPermissionsEvent;
|
||||
use codex_protocol::request_user_input::RequestUserInputEvent;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestion;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestionOption;
|
||||
|
|
@ -2748,20 +2746,6 @@ async fn handle_request_user_input_sets_pending_notification() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn handle_request_permissions_opens_approval_modal() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await;
|
||||
|
||||
chat.handle_request_permissions_now(RequestPermissionsEvent {
|
||||
call_id: "call-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
reason: Some("need workspace access".to_string()),
|
||||
permissions: PermissionProfile::default(),
|
||||
});
|
||||
|
||||
assert!(chat.bottom_pane.has_active_view());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plan_reasoning_scope_popup_mentions_selected_reasoning() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await;
|
||||
|
|
@ -7161,14 +7145,30 @@ async fn experimental_popup_shows_js_repl_node_requirement() {
|
|||
#[tokio::test]
|
||||
async fn experimental_popup_includes_guardian_approval() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
let guardian_stage = FEATURES
|
||||
.iter()
|
||||
.find(|spec| spec.id == Feature::GuardianApproval)
|
||||
.map(|spec| spec.stage)
|
||||
.expect("expected guardian approval feature metadata");
|
||||
let guardian_name = guardian_stage
|
||||
.experimental_menu_name()
|
||||
.expect("expected guardian approval experimental menu name");
|
||||
let guardian_description = guardian_stage
|
||||
.experimental_menu_description()
|
||||
.expect("expected guardian approval experimental description");
|
||||
|
||||
chat.open_experimental_popup();
|
||||
|
||||
let popup = render_bottom_popup(&chat, 120);
|
||||
#[cfg(target_os = "linux")]
|
||||
assert_snapshot!("experimental_popup_includes_guardian_approval_linux", popup);
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
assert_snapshot!("experimental_popup_includes_guardian_approval", popup);
|
||||
let normalized_popup = popup.split_whitespace().collect::<Vec<_>>().join(" ");
|
||||
assert!(
|
||||
popup.contains(guardian_name),
|
||||
"expected guardian approvals entry in experimental popup, got:\n{popup}"
|
||||
);
|
||||
assert!(
|
||||
normalized_popup.contains(guardian_description),
|
||||
"expected guardian approvals description in experimental popup, got:\n{popup}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue