Skip to content

core tests: migrate view image turns to profiles#20021

Merged
bolinfest merged 1 commit intomainfrom
pr20021
Apr 29, 2026
Merged

core tests: migrate view image turns to profiles#20021
bolinfest merged 1 commit intomainfrom
pr20021

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 28, 2026

Why

This stack is removing direct SandboxPolicy usage from test code so new tests exercise the same PermissionProfile path that runtime code now treats as canonical. view_image.rs still built Op::UserTurn requests with SandboxPolicy::DangerFullAccess and no permission profile, which kept another core test module on the legacy turn shape.

What Changed

  • Adds a small disabled_user_turn() helper for the view-image suite that derives the compatibility sandbox_policy and canonical permission_profile from PermissionProfile::Disabled.
  • Replaces repeated direct Op::UserTurn literals in codex-rs/core/tests/suite/view_image.rs with that helper.
  • Removes all SandboxPolicy references from view_image.rs, reducing the remaining codex-rs/core/tests files that mention SandboxPolicy from 18 to 17.

Verification

  • cargo check -p codex-core --tests

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

💡 Codex Review

config.network.enabled = false;

P1 Badge Preserve profile proxy enablement without cloud requirements

network_proxy_config_from_profile_network now unconditionally sets config.network.enabled = false, which makes profile-level proxy policy inert in the common path where no cloud network requirements are loaded. In that path, config assembly only keeps the proxy when network.enabled() is true, so profile settings like domain allow/deny lists or custom proxy endpoints stop being enforced even when network.enabled = true is set in the selected permissions profile, broadening runtime network access unexpectedly.


) || permission_config_syntax.is_none();

P2 Badge Keep legacy writable-root carveouts for implicit default mode

Treating permission_config_syntax == None as profile mode routes default/no-syntax configs through the profiles branch, which applies with_additional_writable_roots for extra roots instead of the legacy with_additional_legacy_workspace_writable_roots path. That drops legacy metadata carveouts (for paths like .git/.codex) on --add-dir roots in implicit default sessions, so users can get broader write access than before without opting into explicit profile syntax.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@bolinfest
Copy link
Copy Markdown
Collaborator Author

[codex] Addressed in the updated bottom commit for #19900: 13a4587. The profile network conversion now keeps the managed proxy enabled when a profile includes explicit proxy-enforced policy, while leaving it disabled for bare network access. The implicit built-in workspace profile path also now applies legacy metadata carveouts to additional writable roots, with regression coverage for add-dir roots.

@bolinfest
Copy link
Copy Markdown
Collaborator Author

[codex] Follow-up after restacking: the fixes are now in #19900 at 33482fd. I also updated the config regression tests so bare network access and explicit proxy policy are covered separately.

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 pr20021 branch 4 times, most recently from 375aad0 to c0309f4 Compare April 28, 2026 20:10
bolinfest added a commit that referenced this pull request Apr 28, 2026
## Summary
- Move TUI permission state from legacy `SandboxPolicy` values to
canonical `PermissionProfile` values across presets, app events, chat
widget state, app commands, thread routing, and cached thread session
state.
- Keep app-server compatibility boundaries explicit: embedded sessions
send `permissionProfile`, while remote sessions send only a legacy
`sandbox` projection and fall back to read-only when a custom profile
cannot be projected.
- Update status/add-dir UI summaries and snapshots to render the active
permission profile, including workspace profiles selected by the new
built-in defaults.

## Verification
- `rg '\bSandboxPolicy\b' codex-rs/tui -n` returns no matches.
- `cargo test -p codex-tui`
- `cargo check -p codex-tui --tests`
- `cargo test -p codex-tui additional_dirs`
- `just fmt`
- `just fix -p codex-tui`




































---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/20008).
* #20041
* #20040
* #20037
* #20035
* #20034
* #20033
* #20032
* #20030
* #20028
* #20027
* #20026
* #20024
* #20021
* #20018
* #20016
* #20015
* #20013
* #20011
* #20010
* __->__ #20008
@bolinfest bolinfest requested a review from viyatb-oai April 29, 2026 00:03
bolinfest added a commit that referenced this pull request Apr 29, 2026
## Summary
- Migrate direct `Op::UserTurn` construction in remote-model tests from
legacy `SandboxPolicy::DangerFullAccess` to
`PermissionProfile::Disabled` via `turn_permission_fields()`.
- Migrate the Responses API proxy header helper from an inline
workspace-write `SandboxPolicy` to
`PermissionProfile::workspace_write()`.
- Reduce `SandboxPolicy` references in `codex-rs/core/tests` from 22
files after #20015 to 20 files.

## Testing
- `cargo check -p codex-core --tests`
- `just fmt`





























---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/20016).
* #20041
* #20040
* #20037
* #20035
* #20034
* #20033
* #20032
* #20030
* #20028
* #20027
* #20026
* #20024
* #20021
* #20018
* __->__ #20016
Base automatically changed from pr20018 to main April 29, 2026 00:09
@bolinfest bolinfest merged commit 2a8ce9b into main Apr 29, 2026
14 checks passed
@bolinfest bolinfest deleted the pr20021 branch April 29, 2026 00:09
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 29, 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.

1 participant