Conversation
This was referenced May 14, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5691ca3924
ℹ️ 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".
| config: &Config, | ||
| session_configured_event: &SessionConfiguredEvent, | ||
| ) -> Vec<(&'static str, String)> { | ||
| let permission_profile = config.permissions.permission_profile(); |
Contributor
There was a problem hiding this comment.
For normal workspace-write configs, core/src/config/mod.rs adds the Codex memories directory to the stored permission profile as an internal writable root, and this now summarizes that fully materialized profile by enumerating all writable roots. In that scenario the exec startup banner will list $CODEX_HOME/memories alongside the user's workspace roots, which is not an effective workspace root and differs from the TUI path that summarizes effective_workspace_roots() instead; please summarize the effective workspace roots directly or filter internal roots here.
Useful? React with 👍 / 👎.
bolinfest
added a commit
that referenced
this pull request
May 14, 2026
#22624) ## Why This is a small precursor to the larger permissions-migration work. Both the comparison stack in [#22401](#22401) / [#22402](#22402) and the alternate stack in [#22610](#22610) / [#22611](#22611) / [#22612](#22612) are easier to review if the terminology is already settled underneath them. Because `:project_roots` and `:danger-no-sandbox` have not shipped as stable user-facing surface area, carrying them forward as aliases would just add more migration logic to the later stacks. This PR removes that ambiguity now so the follow-on work can rely on one spelling for each built-in concept. ## What Changed - renamed the config-facing special filesystem key from `:project_roots` to `:workspace_roots` - dropped unpublished `:project_roots` parsing support in `core/src/config/permissions.rs`, so new config only recognizes `:workspace_roots` - renamed the built-in full-access permission profile id from `:danger-no-sandbox` to `:danger-full-access` - dropped unpublished `:danger-no-sandbox` support entirely, including the old active-profile canonicalization path, and added explicit rejection coverage for the legacy id - introduced shared built-in permission-profile id constants in `codex-rs/protocol/src/models.rs` - updated `core`, `app-server`, and `tui` call sites that special-case built-in profiles to use the shared constants and canonical ids - updated tests and the Linux sandbox README to use `:workspace_roots` / `:danger-full-access` ## Verification I focused verification on the three places this rename can regress: config parsing, active-profile identity surfaced back out of `core`, and user/server call sites that special-case built-in profiles. Targeted checks: - `config::tests::default_permissions_can_select_builtin_profile_without_permissions_table` - `config::tests::default_permissions_read_only_applies_additional_writable_roots_as_modifications` - `config::tests::default_permissions_can_select_builtin_full_access_profile` - `config::tests::legacy_danger_no_sandbox_is_rejected` - `workspace_root` filtered `codex-core` tests - `request_processors::thread_processor::thread_processor_tests::thread_processor_behavior_tests::requested_permissions_trust_project_uses_permission_profile_intent` - `suite::v2::turn_start::turn_start_rejects_invalid_permission_selection_before_starting_turn` - `status::tests::status_snapshot_shows_auto_review_permissions` - `status::tests::status_permissions_full_disk_managed_with_network_is_danger_full_access` - `app_server_session::tests::embedded_turn_permissions_use_active_profile_selection`
bolinfest
force-pushed
the
pr22612
branch
2 times, most recently
from
5a5197b to
5cba401
Compare
bolinfest
force-pushed
the
pr22611
branch
2 times, most recently
from
247bbda to
842662a
Compare
bolinfest
force-pushed
the
pr22612
branch
2 times, most recently
from
c10ca6e to
e1395d8
Compare
bolinfest
force-pushed
the
pr22611
branch
2 times, most recently
from
1a97dd8 to
cbe9413
Compare
bolinfest
force-pushed
the
pr22612
branch
2 times, most recently
from
1925bcd to
1b90490
Compare
bolinfest
force-pushed
the
pr22611
branch
2 times, most recently
from
d525c4b to
1d60544
Compare
bolinfest
force-pushed
the
pr22611
branch
2 times, most recently
from
bc34cfa to
49e6f61
Compare
bolinfest
force-pushed
the
pr22612
branch
2 times, most recently
from
b4e18af to
e87d7c6
Compare
bolinfest
force-pushed
the
pr22611
branch
2 times, most recently
from
98c9b99 to
3d2cc25
Compare
bolinfest
force-pushed
the
pr22612
branch
2 times, most recently
from
25fb4be to
b2d2a83
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR builds on #22611.
After
runtimeWorkspaceRootsmoved onto thread state, the user-facing summaries were still inconsistent about which roots they showed. In particular,/statusand the exec startup summary could under-report extra workspace roots from--add-diror from profile-definedworkspace_roots, which made the new model look incorrect even when the permissions themselves were right.What Changed
Config::effective_workspace_roots()Verification
Targeted coverage for this follow-up lives in:
codex-rs/tui/src/status/tests.rscodex-rs/exec/src/event_processor_with_human_output_tests.rsThe added regressions verify that:
cwdonly