Skip to content

test: reduce core sandbox policy test setup#23036

Merged
bolinfest merged 1 commit into
mainfrom
pr23036
May 17, 2026
Merged

test: reduce core sandbox policy test setup#23036
bolinfest merged 1 commit into
mainfrom
pr23036

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented May 16, 2026

Why

SandboxPolicy is a legacy compatibility shape, but several core tests still used it for ordinary turn setup even when the runtime path now carries PermissionProfile. With the first cleanup PR merged, this follow-up trims more core test scaffolding so remaining SandboxPolicy matches are easier to classify as production compatibility, legacy-boundary coverage, or explicit conversion tests.

What Changed

  • Updated apply-patch handler and runtime tests to pass PermissionProfile directly.
  • Changed sandboxing test helpers to build permission profiles without first creating SandboxPolicy values.
  • Converted request-permissions integration turns to pass PermissionProfile through the test helper, leaving legacy sandbox projection at the Op::UserTurn boundary.
  • Converted unified exec integration helpers and direct turn submissions to use PermissionProfile values instead of SandboxPolicy setup.
  • Removed now-unused SandboxPolicy imports from the touched core tests.

Test Plan

  • just fmt
  • cargo test -p codex-core --lib tools::sandboxing::tests
  • cargo test -p codex-core --lib tools::runtimes::apply_patch::tests
  • cargo test -p codex-core --lib tools::handlers::apply_patch::tests
  • cargo test -p codex-core --lib unified_exec::process_manager::tests
  • cargo test -p codex-core --test all request_permissions::
  • cargo test -p codex-core --test all unified_exec::
  • just fix -p codex-core

@bolinfest bolinfest requested a review from a team as a code owner May 16, 2026 15:07
@bolinfest bolinfest changed the base branch from main to pr23030 May 16, 2026 15:07
bolinfest added a commit that referenced this pull request May 16, 2026
## Why

`SandboxPolicy` is now a legacy compatibility shape, but several tests
still built a `SandboxPolicy` only to immediately convert it into
`PermissionProfile` for APIs that already accept canonical runtime
permissions. Those detours make it harder to audit where legacy sandbox
policy is still required, because boundary-only usages are mixed
together with ordinary test setup.

## What Changed

- Updated tests in `codex-core`, `codex-exec`, `codex-analytics`, and
`codex-config` to construct `PermissionProfile` values directly when the
code under test takes a permission profile.
- Changed exec-policy, request-permissions, session, and sandbox test
helpers to pass `PermissionProfile` through instead of converting from
`SandboxPolicy` internally.
- Left `SandboxPolicy` in place where tests are explicitly exercising
legacy compatibility or request/response boundaries.

## Test Plan

- `cargo test -p codex-analytics -p codex-config`
- `cargo test -p codex-core --lib safety::tests`
- `cargo test -p codex-core --lib exec_policy::tests::`
- `cargo test -p codex-core --lib exec::tests`
- `cargo test -p codex-core --lib guardian_review_session_config`
- `cargo test -p codex-core --lib tools::network_approval::tests`
- `cargo test -p codex-core --lib
tools::runtimes::shell::unix_escalation::tests`
- `cargo test -p codex-core --lib managed_network`
- `cargo test -p codex-core --test all request_permissions::`
- `cargo test -p codex-exec sandbox`


---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23030).
* #23036
* __->__ #23030
Base automatically changed from pr23030 to main May 16, 2026 19:12
@bolinfest bolinfest merged commit 0a83353 into main May 17, 2026
60 of 62 checks passed
@bolinfest bolinfest deleted the pr23036 branch May 17, 2026 15:39
@github-actions github-actions Bot locked and limited conversation to collaborators May 17, 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.

2 participants