mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core: fix apply_patch request permissions test (#21060)
## Why The Bazel test coverage change exposed `approved_folder_write_request_permissions_unblocks_later_apply_patch`, and `rust-ci-full.yml` showed the same test failing on `main` on macOS. There were two separate classes of problems here. ### Clean CI failure The test emits an `apply_patch` tool call, but its config did not enable the `apply_patch` tool, so the mocked response completed without an `apply-patch-call` output. After enabling the tool, the same path also needs the aggregate `codex-core` test binary to dispatch `--codex-run-as-fs-helper`; sandboxed `apply_patch` uses that helper under macOS Seatbelt. The test now also canonicalizes the temporary patch target before building the patch payload so the path matches normalized grants on macOS, where `/var` paths often normalize to `/private/var`. ### Local/enterprise config isolation The core test harness now builds its default test config with managed config disabled, so host-managed enterprise config cannot alter these tests. The request-permissions turns in this test also explicitly use the user reviewer path, keeping the assertions focused on `request_permissions` behavior rather than reviewer defaults from the host. ## What Changed - Enable `apply_patch` in `approved_folder_write_request_permissions_unblocks_later_apply_patch`. - Teach the core integration test binary to dispatch `CODEX_FS_HELPER_ARG1`, matching the existing apply-patch and linux-sandbox dispatch paths. - Canonicalize the tempdir-backed patch target before creating the patch. - Ignore managed config in default core test configs and explicitly pin this test to `ApprovalsReviewer::User`. ## Verification Run outside the Codex app sandbox because these macOS tests intentionally spawn Seatbelt: - `cargo test -p codex-core approved_folder_write_request_permissions_unblocks_later_apply_patch` - `cargo test -p codex-core approved_folder_write_request_permissions_unblocks_later_exec_without_sandbox_args`
This commit is contained in:
committed by
GitHub
Unverified
parent
8126af3879
commit
229b40aa21
@@ -10,6 +10,7 @@ use tempfile::TempDir;
|
||||
|
||||
use codex_config::CloudRequirementsLoader;
|
||||
use codex_config::ConfigRequirementsToml;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::NetworkRequirementsToml;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::config::Config;
|
||||
@@ -181,6 +182,7 @@ pub async fn load_default_config_for_test_with_cloud_requirements(
|
||||
cloud_requirements: CloudRequirementsLoader,
|
||||
) -> Config {
|
||||
ConfigBuilder::default()
|
||||
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(default_test_overrides())
|
||||
.cloud_requirements(cloud_requirements)
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Aggregates all former standalone integration tests as modules.
|
||||
use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1;
|
||||
use codex_exec_server::CODEX_FS_HELPER_ARG1;
|
||||
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use codex_test_binary_support::TestBinaryDispatchGuard;
|
||||
use codex_test_binary_support::TestBinaryDispatchMode;
|
||||
@@ -16,6 +17,9 @@ pub static CODEX_ALIASES_TEMP_DIR: Option<TestBinaryDispatchGuard> = {
|
||||
if argv1 == Some(CODEX_CORE_APPLY_PATCH_ARG1) {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
if argv1 == Some(CODEX_FS_HELPER_ARG1) {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
if exe_name == CODEX_LINUX_SANDBOX_ARG0 {
|
||||
return TestBinaryDispatchMode::DispatchArg0Only;
|
||||
}
|
||||
|
||||
@@ -264,7 +264,7 @@ async fn approved_folder_write_request_permissions_unblocks_later_exec_without_s
|
||||
"write outside the workspace",
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
/*approvals_reviewer*/ None,
|
||||
Some(ApprovalsReviewer::User),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -340,6 +340,7 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
let permission_profile_for_config = permission_profile.clone();
|
||||
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config
|
||||
.permissions
|
||||
@@ -367,7 +368,10 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
} else {
|
||||
"patched-via-request-permissions"
|
||||
};
|
||||
let requested_file = requested_dir.path().join(requested_file_name);
|
||||
let requested_file = requested_dir
|
||||
.path()
|
||||
.canonicalize()?
|
||||
.join(requested_file_name);
|
||||
let requested_permissions = requested_directory_write_permissions(requested_dir.path());
|
||||
let normalized_requested_permissions =
|
||||
normalized_directory_write_permissions(requested_dir.path())?;
|
||||
@@ -422,7 +426,7 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
"patch outside the workspace",
|
||||
approval_policy,
|
||||
permission_profile,
|
||||
strict_auto_review.then_some(ApprovalsReviewer::User),
|
||||
Some(ApprovalsReviewer::User),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -463,8 +467,7 @@ async fn apply_patch_after_request_permissions(strict_auto_review: bool) -> Resu
|
||||
EventMsg::TurnComplete(_) => {}
|
||||
EventMsg::ApplyPatchApprovalRequest(approval) => {
|
||||
panic!(
|
||||
"unexpected apply_patch approval request after granted permissions: {:?}",
|
||||
approval.call_id
|
||||
"unexpected apply_patch approval request after granted permissions: {approval:?}",
|
||||
)
|
||||
}
|
||||
other => panic!("unexpected event: {other:?}"),
|
||||
|
||||
Reference in New Issue
Block a user