Conversation
bolinfest
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a850df1e1e
ℹ️ 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".
| /// Optional active permissions profile for this command. | ||
| /// | ||
| /// Defaults to the user's configured permissions when omitted. Cannot be | ||
| /// combined with `sandboxPolicy`. | ||
| #[experimental("command/exec.permissionProfile")] | ||
| #[ts(optional = nullable)] | ||
| pub permission_profile: Option<PermissionProfile>, | ||
| pub permission_profile: Option<ActivePermissionProfile>, |
There was a problem hiding this comment.
After changing permissionProfile to ActivePermissionProfile, the app-server docs still show clients sending a full managed profile object (type, fileSystem, network) in codex-rs/app-server/README.md:929-936. That old example now fails because this field expects an active profile object with an id, so clients following the documented command/exec example will send invalid requests; update the README example/notes along with this API shape change.
Useful? React with 👍 / 👎.
bolinfest
force-pushed
the
pr22924
branch
2 times, most recently
from
78674c0 to
07b7e3c
Compare
2 participants
Why
The app server API should expose permission profile identity, not the lower-level runtime permission model.
PermissionProfileis the compiled sandbox/network representation that the server uses internally; exposing it through app-server-protocol forces clients to understand details that should remain implementation-level.The API boundary should prefer
ActivePermissionProfile: a stable profile id, plus future parent-profile metadata, that clients can pass back when they want to select the same active permissions. This also avoids schema generation collisions between the app-server v2 API type space and the core protocol model.Incidentally, while PR makes a number of changes to
command/exec, note that we are hoping to deprecate this API in favor ofprocess/spawn, so we don't need to be too finicky about these changes.What Changed
PermissionProfilefrom the app-server-protocol API surface, including generated schema and TypeScript exports.CommandExecParams.permissionProfiletoActivePermissionProfile.ConfigManagerfor the command cwd, matching turn override selection semantics.PermissionProfileshims.