feat: add permission profile list api#23412
Conversation
Co-authored-by: Codex noreply@openai.com
![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: 022c70895f
ℹ️ 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".
| id: BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS.to_string(), | ||
| description: None, |
There was a problem hiding this comment.
When managed requirements restrict permissions (for example allowed_sandbox_modes = ["read-only"]), ConfigBuilder already falls back and clears the active profile for disallowed selections, but this catalog still advertises the unrestricted built-in profiles unconditionally. In those managed environments a client can present :danger-full-access as an available choice from permissionProfile/list, only for thread/start/overrides to be constrained back to read-only, so the list should be derived from the effective constraints or omit profiles that cannot actually be selected.
Useful? React with 👍 / 👎.
Co-authored-by: Codex noreply@openai.com
viyatb-oai
changed the title
…ents" This reverts commit 05c815c.
viyatb-oai
changed the title
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
| #[ts(export_to = "v2/")] | ||
| pub struct PermissionProfileListParams { | ||
| /// Opaque pagination cursor returned by a previous call. | ||
| #[ts(optional = nullable)] |
There was a problem hiding this comment.
I'm debating whether adding support for a cursor is helpful or just adds more complexity that our clients are unlikely to need?
There was a problem hiding this comment.
I’m keeping cursor support here. It matches the default shape for new app-server list APIs and avoids revisiting the contract later if the profile catalog grows beyond the small built-in set.
There was a problem hiding this comment.
sorry codex comment - having a cursor would be great if an org pushed a lot of permission profiles.
| #[ts(export_to = "v2/")] | ||
| pub struct PermissionProfileSummary { | ||
| /// Available permission profile identifier. | ||
| pub id: String, |
There was a problem hiding this comment.
I'm debating whether we should optionally provide label: Option<String> in addition to description where the label would be the value to show instead of the id in a UI element without a lot of space. This could be used to simply the raw names of the built-in profiles in the UI?
There was a problem hiding this comment.
I’m leaving label out of this PR. I’d like this list surface to stay semantic rather than presentation-specific until we have a clear label source and contract for custom profiles too, not just the built-ins.
There was a problem hiding this comment.
keeping this for a follow up pr
Co-authored-by: Codex noreply@openai.com
2 participants
Why
Clients need a typed permission-profile catalog instead of reconstructing that state from config internals.
What changed
permissionProfile/listto the app-server v2 protocol with cursor pagination and optionalcwd.[permissions.<id>]profiles from the effective config for the request context.descriptionmetadata for display purposes.