Skip to content

core tests: migrate plan item turns to profiles#20026

Open
bolinfest wants to merge 1 commit intopr20024from
pr20026
Open

core tests: migrate plan item turns to profiles#20026
bolinfest wants to merge 1 commit intopr20024from
pr20026

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 28, 2026

Why

The core item tests still had a cluster of plan-mode Op::UserTurn literals that used SandboxPolicy::DangerFullAccess and omitted permission_profile. These tests are validating emitted item lifecycle events, so keeping them on the legacy sandbox-only turn shape adds noise to the broader permissions migration without testing legacy behavior.

What Changed

  • Adds a local disabled_plan_turn() helper that preserves the existing std::env::current_dir() turn cwd behavior.
  • Uses turn_permission_fields(PermissionProfile::Disabled, cwd) to populate both the compatibility sandbox_policy and canonical permission_profile fields.
  • Replaces the plan-mode hand-built turns in codex-rs/core/tests/suite/items.rs, removing all SandboxPolicy references from that file and reducing remaining codex-rs/core/tests SandboxPolicy files from 16 to 15.

Verification

  • cargo check -p codex-core --tests

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest requested a review from a team as a code owner April 28, 2026 16:54
@bolinfest bolinfest changed the base branch from main to pr20024 April 28, 2026 16:54
bolinfest added a commit that referenced this pull request Apr 28, 2026
## Why

The migration away from `SandboxPolicy` needs new configs to start from
permissions profiles instead of deriving profiles from legacy sandbox
modes. Existing users can have empty `config.toml` files, and we should
not rewrite user-owned config files that may live in shared
repositories.

This PR introduces built-in profile names so an empty config can resolve
to a canonical `PermissionProfile`, while explicit named `[permissions]`
profiles still behave predictably.

## What changed

- Adds built-in `default_permissions` profile names:
  - `:read-only` maps to `PermissionProfile::read_only()`.
- `:workspace` maps to the workspace-write profile, including
project-root metadata carveouts.
- `:danger-no-sandbox` maps to `PermissionProfile::Disabled`, preserving
the distinction between no sandbox and a broad managed sandbox.
- Reserves the `:` prefix for built-in profiles so user-defined
`[permissions]` profiles cannot collide with future built-ins.
- Allows `default_permissions` to reference a built-in profile without
requiring a `[permissions]` table.
- Makes an otherwise empty config choose a built-in profile by
trust/platform context: trusted or untrusted project roots use
`:workspace` when the platform supports that sandbox, while roots
without a trust decision use `:read-only`.
- Keeps legacy `sandbox_mode` configs on the legacy path, and still
rejects user-defined `[permissions]` profiles that omit
`default_permissions` so we do not silently guess among custom profiles.
- Preserves compatibility behavior for implicit defaults: bare
`network.enabled = true` allows runtime network without starting the
managed proxy, explicit profile proxy policy still starts the proxy, and
implicit workspace/add-dir roots keep legacy metadata carveouts.

## Verification

- `cargo test -p codex-core builtin --lib`
- `cargo test -p codex-core profile_network_proxy_config`
- `cargo test -p codex-core
implicit_builtin_workspace_profile_preserves_add_dir_metadata_carveouts`
- `cargo test -p codex-core
permissions_profiles_network_enabled_allows_runtime_network_without_proxy`
- `cargo test -p codex-core
permissions_profiles_proxy_policy_starts_managed_network_proxy`

## Documentation

Public Codex config docs should mention these built-in names when the
`[permissions]` config format is ready to document as stable.









---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19900).
* #20041
* #20040
* #20037
* #20035
* #20034
* #20033
* #20032
* #20030
* #20028
* #20027
* #20026
* #20024
* #20021
* #20018
* #20016
* #20015
* #20013
* #20011
* #20010
* #20008
* __->__ #19900
@bolinfest bolinfest force-pushed the pr20024 branch 2 times, most recently from 7487c90 to a7c7b8f Compare April 28, 2026 20:01
@bolinfest bolinfest force-pushed the pr20024 branch 2 times, most recently from 11faa1d to 95f1811 Compare April 28, 2026 20:10
@bolinfest bolinfest force-pushed the pr20026 branch 2 times, most recently from a25260e to 92d44cf Compare April 28, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant