Skip to content

core: fix apply_patch request permissions test#21060

Merged
bolinfest merged 1 commit into
mainfrom
pr21060
May 4, 2026
Merged

core: fix apply_patch request permissions test#21060
bolinfest merged 1 commit into
mainfrom
pr21060

Conversation

@bolinfest

@bolinfest bolinfest commented May 4, 2026

Copy link
Copy Markdown
Collaborator

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

@bolinfest bolinfest requested a review from a team as a code owner May 4, 2026 18:52
@bolinfest bolinfest changed the title core: fix request permissions apply_patch test core: fix apply_patch request permissions test May 4, 2026
@bolinfest bolinfest merged commit 229b40a into main May 4, 2026
37 of 38 checks passed
@bolinfest bolinfest deleted the pr21060 branch May 4, 2026 19:49
@github-actions github-actions Bot locked and limited conversation to collaborators May 4, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants