Add exec approval app-server support

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Dylan Hurd
2026-03-22 14:35:20 -07:00
co-authored by Codex
parent c3df4c6a0a
commit 37e7b08b51
38 changed files with 735 additions and 48 deletions
@@ -65,6 +65,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the filesystem permissions to be persisted into the active named permissions profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
@@ -235,6 +235,13 @@
],
"type": "string"
},
{
"description": "User approved the command and wants to persist the filesystem access in the active named permissions profile.",
"enum": [
"acceptAndPersist"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.",
@@ -397,6 +404,17 @@
"deny"
],
"type": "string"
},
"PermissionProfilePersistence": {
"properties": {
"profileName": {
"type": "string"
}
},
"required": [
"profileName"
],
"type": "object"
}
},
"properties": {
@@ -445,6 +463,17 @@
],
"description": "Optional context for a managed-network approval prompt."
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
],
"description": "Optional named permissions profile that can persist the requested filesystem access."
},
"proposedExecpolicyAmendment": {
"description": "Optional proposed execpolicy amendment to allow similar commands without prompting.",
"items": {
@@ -17,6 +17,13 @@
],
"type": "string"
},
{
"description": "User approved the command and wants to persist the filesystem access in the active named permissions profile.",
"enum": [
"acceptAndPersist"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.",
@@ -65,6 +65,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the filesystem permissions to be persisted into the active named permissions profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
@@ -39,6 +39,17 @@
},
"type": "object"
},
"PermissionProfilePersistence": {
"properties": {
"profileName": {
"type": "string"
}
},
"required": [
"profileName"
],
"type": "object"
},
"RequestPermissionProfile": {
"additionalProperties": false,
"properties": {
@@ -73,6 +84,16 @@
"permissions": {
"$ref": "#/definitions/RequestPermissionProfile"
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
]
},
"reason": {
"type": [
"string",
@@ -76,6 +76,9 @@
"permissions": {
"$ref": "#/definitions/GrantedPermissionProfile"
},
"persistToProfile": {
"type": "boolean"
},
"scope": {
"allOf": [
{
@@ -301,6 +301,13 @@
],
"type": "string"
},
{
"description": "User approved the command and wants to persist the filesystem access in the active named permissions profile.",
"enum": [
"acceptAndPersist"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.",
@@ -411,6 +418,17 @@
],
"description": "Optional context for a managed-network approval prompt."
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
],
"description": "Optional named permissions profile that can persist the requested filesystem access."
},
"proposedExecpolicyAmendment": {
"description": "Optional proposed execpolicy amendment to allow similar commands without prompting.",
"items": {
@@ -1443,6 +1461,17 @@
}
]
},
"PermissionProfilePersistence": {
"properties": {
"profileName": {
"type": "string"
}
},
"required": [
"profileName"
],
"type": "object"
},
"PermissionsRequestApprovalParams": {
"properties": {
"itemId": {
@@ -1451,6 +1480,16 @@
"permissions": {
"$ref": "#/definitions/RequestPermissionProfile"
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
]
},
"reason": {
"type": [
"string",
@@ -1622,6 +1622,13 @@
],
"type": "string"
},
{
"description": "User approved the command and wants to persist the filesystem access in the active named permissions profile.",
"enum": [
"acceptAndPersist"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User approved the command, and wants to apply the proposed execpolicy amendment so future matching commands can run without prompting.",
@@ -1733,6 +1740,17 @@
],
"description": "Optional context for a managed-network approval prompt."
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
],
"description": "Optional named permissions profile that can persist the requested filesystem access."
},
"proposedExecpolicyAmendment": {
"description": "Optional proposed execpolicy amendment to allow similar commands without prompting.",
"items": {
@@ -3235,6 +3253,17 @@
],
"type": "string"
},
"PermissionProfilePersistence": {
"properties": {
"profileName": {
"type": "string"
}
},
"required": [
"profileName"
],
"type": "object"
},
"PermissionsRequestApprovalParams": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
@@ -3244,6 +3273,16 @@
"permissions": {
"$ref": "#/definitions/RequestPermissionProfile"
},
"permissionsProfilePersistence": {
"anyOf": [
{
"$ref": "#/definitions/PermissionProfilePersistence"
},
{
"type": "null"
}
]
},
"reason": {
"type": [
"string",
@@ -3272,6 +3311,9 @@
"permissions": {
"$ref": "#/definitions/GrantedPermissionProfile"
},
"persistToProfile": {
"type": "boolean"
},
"scope": {
"allOf": [
{
@@ -3368,6 +3410,13 @@
],
"type": "string"
},
{
"description": "User has approved this request and wants the filesystem permissions to be persisted into the active named permissions profile.",
"enum": [
"approved_persist_to_profile"
],
"type": "string"
},
{
"additionalProperties": false,
"description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.",
@@ -7,4 +7,4 @@ import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
/**
* User's decision in response to an ExecApprovalRequest.
*/
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "abort";
export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | "approved_persist_to_profile" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "abort";
@@ -4,4 +4,4 @@
import type { ExecPolicyAmendment } from "./ExecPolicyAmendment";
import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
export type CommandExecutionApprovalDecision = "accept" | "acceptForSession" | { "acceptWithExecpolicyAmendment": { execpolicy_amendment: ExecPolicyAmendment, } } | { "applyNetworkPolicyAmendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "decline" | "cancel";
export type CommandExecutionApprovalDecision = "accept" | "acceptForSession" | "acceptAndPersist" | { "acceptWithExecpolicyAmendment": { execpolicy_amendment: ExecPolicyAmendment, } } | { "applyNetworkPolicyAmendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "decline" | "cancel";
@@ -8,6 +8,7 @@ import type { CommandExecutionRequestApprovalSkillMetadata } from "./CommandExec
import type { ExecPolicyAmendment } from "./ExecPolicyAmendment";
import type { NetworkApprovalContext } from "./NetworkApprovalContext";
import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment";
import type { PermissionProfilePersistence } from "./PermissionProfilePersistence";
export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: string, itemId: string,
/**
@@ -44,6 +45,11 @@ commandActions?: Array<CommandAction> | null,
* Optional additional permissions requested for this command.
*/
additionalPermissions?: AdditionalPermissionProfile | null,
/**
* Optional named permissions profile that can persist the requested
* filesystem access.
*/
permissionsProfilePersistence?: PermissionProfilePersistence | null,
/**
* Optional skill metadata when the approval was triggered by a skill script.
*/
@@ -0,0 +1,5 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type PermissionProfilePersistence = { profileName: string, };
@@ -1,6 +1,7 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { PermissionProfilePersistence } from "./PermissionProfilePersistence";
import type { RequestPermissionProfile } from "./RequestPermissionProfile";
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, reason: string | null, permissions: RequestPermissionProfile, };
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, reason: string | null, permissions: RequestPermissionProfile, permissionsProfilePersistence?: PermissionProfilePersistence | null, };
@@ -4,4 +4,4 @@
import type { GrantedPermissionProfile } from "./GrantedPermissionProfile";
import type { PermissionGrantScope } from "./PermissionGrantScope";
export type PermissionsRequestApprovalResponse = { permissions: GrantedPermissionProfile, scope: PermissionGrantScope, };
export type PermissionsRequestApprovalResponse = { permissions: GrantedPermissionProfile, scope: PermissionGrantScope, persistToProfile?: boolean, };
@@ -198,6 +198,7 @@ export type { OverriddenMetadata } from "./OverriddenMetadata";
export type { PatchApplyStatus } from "./PatchApplyStatus";
export type { PatchChangeKind } from "./PatchChangeKind";
export type { PermissionGrantScope } from "./PermissionGrantScope";
export type { PermissionProfilePersistence } from "./PermissionProfilePersistence";
export type { PermissionsRequestApprovalParams } from "./PermissionsRequestApprovalParams";
export type { PermissionsRequestApprovalResponse } from "./PermissionsRequestApprovalResponse";
export type { PlanDeltaNotification } from "./PlanDeltaNotification";
@@ -1681,6 +1681,7 @@ mod tests {
}),
macos: None,
}),
permissions_profile_persistence: None,
skill_metadata: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
@@ -1706,6 +1707,7 @@ mod tests {
cwd: None,
command_actions: None,
additional_permissions: None,
permissions_profile_persistence: None,
skill_metadata: Some(v2::CommandExecutionRequestApprovalSkillMetadata {
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
}),
@@ -969,6 +969,9 @@ pub enum CommandExecutionApprovalDecision {
/// User approved the command and future prompts in the same session-scoped
/// approval cache should run without prompting.
AcceptForSession,
/// User approved the command and wants to persist the filesystem access in
/// the active named permissions profile.
AcceptAndPersist,
/// User approved the command, and wants to apply the proposed execpolicy amendment so future
/// matching commands can run without prompting.
AcceptWithExecpolicyAmendment {
@@ -988,6 +991,7 @@ impl From<CoreReviewDecision> for CommandExecutionApprovalDecision {
fn from(value: CoreReviewDecision) -> Self {
match value {
CoreReviewDecision::Approved => Self::Accept,
CoreReviewDecision::ApprovedPersistToProfile => Self::AcceptAndPersist,
CoreReviewDecision::ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} => Self::AcceptWithExecpolicyAmendment {
@@ -5145,6 +5149,11 @@ pub struct CommandExecutionRequestApprovalParams {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional = nullable)]
pub additional_permissions: Option<AdditionalPermissionProfile>,
/// Optional named permissions profile that can persist the requested
/// filesystem access.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional = nullable)]
pub permissions_profile_persistence: Option<PermissionProfilePersistence>,
/// Optional skill metadata when the approval was triggered by a skill script.
#[experimental("item/commandExecution/requestApproval.skillMetadata")]
#[serde(default, skip_serializing_if = "Option::is_none")]
@@ -1921,6 +1921,7 @@ impl CodexClient {
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
available_decisions,
permissions_profile_persistence: _,
} = params;
println!(
+4 -4
View File
@@ -931,15 +931,15 @@ When an upstream HTTP status is available (for example, from the Responses API o
Certain actions (shell commands or modifying files) may require explicit user approval depending on the user's config. When `turn/start` is used, the app-server drives an approval flow by sending a server-initiated JSON-RPC request to the client. The client must respond to tell Codex whether to proceed. UIs should present these requests inline with the active turn so users can review the proposed command or diff before choosing.
- Requests include `threadId` and `turnId`—use them to scope UI state to the active conversation.
- Respond with a single `{ "decision": ... }` payload. Command approvals support `accept`, `acceptForSession`, `acceptWithExecpolicyAmendment`, `applyNetworkPolicyAmendment`, `decline`, or `cancel`. The server resumes or declines the work and ends the item with `item/completed`.
- Respond with a single `{ "decision": ... }` payload. Command approvals support `accept`, `acceptForSession`, `acceptAndPersist`, `acceptWithExecpolicyAmendment`, `applyNetworkPolicyAmendment`, `decline`, or `cancel`. The server resumes or declines the work and ends the item with `item/completed`.
### Command execution approvals
Order of messages:
1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action.
2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access; any filesystem paths in that payload are absolute on the wire, and network access is represented as `additionalPermissions.network.enabled`. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted.
3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`.
2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access; any filesystem paths in that payload are absolute on the wire, and network access is represented as `additionalPermissions.network.enabled`. For eligible filesystem-only requests, `permissionsProfilePersistence` names the editable profile that can absorb an always-allow decision. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. Optional persistence hints may also be included via `proposedExecpolicyAmendment` and `proposedNetworkPolicyAmendments`. Clients can prefer `availableDecisions` when present to render the exact set of choices the server wants to expose, while still falling back to the older heuristics if it is omitted.
3. Client response — for example `{ "decision": "accept" }`, `{ "decision": "acceptForSession" }`, `{ "decision": "acceptAndPersist" }`, `{ "decision": { "acceptWithExecpolicyAmendment": { "execpolicy_amendment": [...] } } }`, `{ "decision": { "applyNetworkPolicyAmendment": { "network_policy_amendment": { "host": "example.com", "action": "allow" } } } }`, `{ "decision": "decline" }`, or `{ "decision": "cancel" }`.
4. `serverRequest/resolved` — `{ threadId, requestId }` confirms the pending request has been resolved or cleared, including lifecycle cleanup on turn start/complete/interrupt.
5. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result.
@@ -975,7 +975,7 @@ Order of messages:
### Permission requests
The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the standalone tool's narrower permission shape, so it can request network access and additional filesystem access but does not include the broader `macos` branch used by command-execution `additionalPermissions`.
The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the standalone tool's narrower permission shape, so it can request network access and additional filesystem access but does not include the broader `macos` branch used by command-execution `additionalPermissions`. For eligible filesystem-only requests, `permissionsProfilePersistence` identifies the editable named profile that can absorb an always-allow approval. Clients return the granted subset in `permissions`, the grant `scope`, and may set `persistToProfile: true` to persist that granted filesystem subset into the active profile.
```json
{
@@ -610,6 +610,7 @@ pub(crate) async fn apply_bespoke_event_handling(
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
additional_permissions,
permissions_profile_persistence,
skill_metadata,
parsed_cmd,
..
@@ -680,8 +681,13 @@ pub(crate) async fn apply_bespoke_event_handling(
.map(V2NetworkPolicyAmendment::from)
.collect()
});
let additional_permissions =
additional_permissions.map(V2AdditionalPermissionProfile::from);
let additional_permissions_for_request = additional_permissions
.clone()
.map(V2AdditionalPermissionProfile::from);
let permissions_profile_persistence_for_request =
permissions_profile_persistence
.clone()
.map(codex_app_server_protocol::PermissionProfilePersistence::from);
let skill_metadata =
skill_metadata.map(CommandExecutionRequestApprovalSkillMetadata::from);
@@ -695,7 +701,9 @@ pub(crate) async fn apply_bespoke_event_handling(
command,
cwd,
command_actions,
additional_permissions,
additional_permissions: additional_permissions_for_request,
permissions_profile_persistence:
permissions_profile_persistence_for_request,
skill_metadata,
proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2,
proposed_network_policy_amendments: proposed_network_policy_amendments_v2,
@@ -706,6 +714,11 @@ pub(crate) async fn apply_bespoke_event_handling(
params,
))
.await;
let additional_permissions = additional_permissions.and_then(|permissions| {
serde_json::to_value(permissions)
.ok()
.and_then(|value| serde_json::from_value(value).ok())
});
tokio::spawn(async move {
on_command_execution_request_approval_response(
event_turn_id,
@@ -713,6 +726,8 @@ pub(crate) async fn apply_bespoke_event_handling(
approval_id,
call_id,
completion_item,
additional_permissions,
permissions_profile_persistence,
pending_request_id,
rx,
conversation,
@@ -2295,6 +2310,7 @@ async fn on_exec_approval_response(
id: call_id,
turn_id: Some(turn_id),
decision: response.decision,
persist_permissions: None,
})
.await
{
@@ -2655,6 +2671,8 @@ async fn on_command_execution_request_approval_response(
approval_id: Option<String>,
item_id: String,
completion_item: Option<CommandExecutionCompletionItem>,
additional_permissions: Option<codex_protocol::models::PermissionProfile>,
permissions_profile_persistence: Option<CorePermissionProfilePersistence>,
pending_request_id: RequestId,
receiver: oneshot::Receiver<ClientRequestResult>,
conversation: Arc<CodexThread>,
@@ -2682,6 +2700,9 @@ async fn on_command_execution_request_approval_response(
CommandExecutionApprovalDecision::AcceptForSession => {
(ReviewDecision::ApprovedForSession, None)
}
CommandExecutionApprovalDecision::AcceptAndPersist => {
(ReviewDecision::ApprovedPersistToProfile, None)
}
CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
execpolicy_amendment,
} => (
@@ -2760,11 +2781,23 @@ async fn on_command_execution_request_approval_response(
.await;
}
let persist_permissions = if matches!(decision, ReviewDecision::ApprovedPersistToProfile) {
permissions_profile_persistence.and_then(|target| {
additional_permissions.map(|permissions| PersistPermissionProfileAction {
profile_name: target.profile_name,
permissions,
})
})
} else {
None
};
if let Err(err) = conversation
.submit(Op::ExecApproval {
id: approval_id.unwrap_or_else(|| item_id.clone()),
turn_id: Some(event_turn_id),
decision,
persist_permissions,
})
.await
{
+2
View File
@@ -1124,6 +1124,7 @@ mod tests {
macos: None,
},
),
permissions_profile_persistence: None,
skill_metadata: Some(CommandExecutionRequestApprovalSkillMetadata {
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
}),
@@ -1191,6 +1192,7 @@ mod tests {
macos: None,
},
),
permissions_profile_persistence: None,
skill_metadata: Some(CommandExecutionRequestApprovalSkillMetadata {
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
}),
+4
View File
@@ -1619,6 +1619,7 @@ impl App {
available_decisions: ev.effective_available_decisions(),
network_approval_context: ev.network_approval_context.clone(),
additional_permissions: ev.additional_permissions.clone(),
permissions_profile_persistence: ev.permissions_profile_persistence.clone(),
}))
}
EventMsg::ApplyPatchApprovalRequest(ev) => Some(ThreadInteractiveRequest::Approval(
@@ -4714,6 +4715,7 @@ mod tests {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -6361,6 +6363,7 @@ guardian_approval = true
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -6450,6 +6453,7 @@ guardian_approval = true
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -455,6 +455,7 @@ mod tests {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -464,6 +465,7 @@ mod tests {
id: "approval-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: codex_protocol::protocol::ReviewDecision::Approved,
persist_permissions: None,
});
let snapshot = store.snapshot();
@@ -599,6 +601,7 @@ mod tests {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -689,6 +692,7 @@ mod tests {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: Vec::new(),
},
),
@@ -700,6 +704,7 @@ mod tests {
id: "call-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: codex_protocol::protocol::ReviewDecision::Approved,
persist_permissions: None,
});
assert_eq!(store.has_pending_thread_approvals(), false);
@@ -57,6 +57,7 @@ pub(crate) enum ApprovalRequest {
available_decisions: Vec<ReviewDecision>,
network_approval_context: Option<NetworkApprovalContext>,
additional_permissions: Option<PermissionProfile>,
permissions_profile_persistence: Option<PermissionProfilePersistence>,
},
Permissions {
thread_id: ThreadId,
@@ -154,12 +155,14 @@ impl ApprovalOverlay {
available_decisions,
network_approval_context,
additional_permissions,
permissions_profile_persistence,
..
} => (
exec_options(
available_decisions,
network_approval_context.as_ref(),
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
),
network_approval_context.as_ref().map_or_else(
|| "Would you like to run the following command?".to_string(),
@@ -225,8 +228,23 @@ impl ApprovalOverlay {
};
if let Some(request) = self.current_request.as_ref() {
match (request, &option.decision) {
(ApprovalRequest::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => {
self.handle_exec_decision(id, command, decision.clone());
(
ApprovalRequest::Exec {
id,
command,
additional_permissions,
permissions_profile_persistence,
..
},
ApprovalDecision::Review(decision),
) => {
self.handle_exec_decision(
id,
command,
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
decision.clone(),
);
}
(
ApprovalRequest::Permissions {
@@ -263,7 +281,14 @@ impl ApprovalOverlay {
self.advance_queue();
}
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
fn handle_exec_decision(
&self,
id: &str,
command: &[String],
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
decision: ReviewDecision,
) {
let Some(request) = self.current_request.as_ref() else {
return;
};
@@ -276,12 +301,18 @@ impl ApprovalOverlay {
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
}
let thread_id = request.thread_id();
let persist_permissions = persist_permissions_for_exec_decision(
&decision,
additional_permissions,
permissions_profile_persistence,
);
self.app_event_tx.send(AppEvent::SubmitThreadOp {
thread_id,
op: Op::ExecApproval {
id: id.to_string(),
turn_id: None,
decision,
persist_permissions,
},
});
}
@@ -458,8 +489,20 @@ impl BottomPaneView for ApprovalOverlay {
&& let Some(request) = self.current_request.as_ref()
{
match request {
ApprovalRequest::Exec { id, command, .. } => {
self.handle_exec_decision(id, command, ReviewDecision::Abort);
ApprovalRequest::Exec {
id,
command,
additional_permissions,
permissions_profile_persistence,
..
} => {
self.handle_exec_decision(
id,
command,
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
ReviewDecision::Abort,
);
}
ApprovalRequest::Permissions {
call_id,
@@ -688,6 +731,7 @@ fn exec_options(
available_decisions: &[ReviewDecision],
network_approval_context: Option<&NetworkApprovalContext>,
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Vec<ApprovalOption> {
available_decisions
.iter()
@@ -736,6 +780,14 @@ fn exec_options(
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
}),
ReviewDecision::ApprovedPersistToProfile => {
permissions_profile_persistence.map(|_| ApprovalOption {
label: "Yes, always allow these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedPersistToProfile),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
})
}
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => {
@@ -1031,6 +1083,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
}
}
@@ -1049,6 +1102,7 @@ mod tests {
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
},
permissions_profile_persistence: None,
}
}
@@ -1096,6 +1150,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1124,6 +1179,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1157,6 +1213,7 @@ mod tests {
],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1214,6 +1271,7 @@ mod tests {
protocol: NetworkApprovalProtocol::Https,
}),
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1240,6 +1298,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1281,6 +1340,7 @@ mod tests {
],
Some(&network_context),
None,
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1305,6 +1365,7 @@ mod tests {
],
None,
None,
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1331,6 +1392,7 @@ mod tests {
&[ReviewDecision::Approved, ReviewDecision::Abort],
None,
Some(&additional_permissions),
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1345,7 +1407,7 @@ mod tests {
#[test]
fn permissions_options_use_expected_labels() {
let labels: Vec<String> = permissions_options()
let labels: Vec<String> = permissions_options(None)
.into_iter()
.map(|option| option.label)
.collect();
@@ -1457,6 +1519,7 @@ mod tests {
}),
..Default::default()
}),
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1504,6 +1567,7 @@ mod tests {
}),
..Default::default()
}),
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1539,6 +1603,7 @@ mod tests {
protocol: NetworkApprovalProtocol::Https,
}),
additional_permissions: None,
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
+1
View File
@@ -1296,6 +1296,7 @@ mod tests {
],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
}
}
+1
View File
@@ -3361,6 +3361,7 @@ impl ChatWidget {
available_decisions,
network_approval_context: ev.network_approval_context,
additional_permissions: ev.additional_permissions,
permissions_profile_persistence: ev.permissions_profile_persistence,
};
self.bottom_pane
.push_approval_request(request, &self.config.features);
+8
View File
@@ -3314,6 +3314,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -3365,6 +3366,7 @@ async fn exec_approval_uses_approval_id_when_present() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
}),
});
@@ -3407,6 +3409,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -3463,6 +3466,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9046,6 +9050,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9108,6 +9113,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9157,6 +9163,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9525,6 +9532,7 @@ async fn status_widget_and_approval_modal_snapshot() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
+13
View File
@@ -813,6 +813,19 @@ pub fn new_approval_decision_cell(
],
)
}
ApprovedPersistToProfile => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"✔ ".green(),
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" and saved those permissions".into(),
],
)
}
ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} => {
+82
View File
@@ -172,6 +172,9 @@ fn command_execution_decision_to_review_decision(
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptForSession => {
codex_protocol::protocol::ReviewDecision::ApprovedForSession
}
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptAndPersist => {
codex_protocol::protocol::ReviewDecision::ApprovedPersistToProfile
}
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
execpolicy_amendment,
} => codex_protocol::protocol::ReviewDecision::ApprovedExecpolicyAmendment {
@@ -208,12 +211,16 @@ fn default_exec_approval_decisions(
&[codex_protocol::approvals::NetworkPolicyAmendment],
>,
additional_permissions: Option<&codex_protocol::models::PermissionProfile>,
permissions_profile_persistence: Option<
&codex_protocol::request_permissions::PermissionProfilePersistence,
>,
) -> Vec<codex_protocol::protocol::ReviewDecision> {
ExecApprovalRequestEvent::default_available_decisions(
network_approval_context,
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
additional_permissions,
permissions_profile_persistence,
)
}
@@ -1632,6 +1639,14 @@ impl App {
.map(codex_app_server_protocol::NetworkPolicyAmendment::into_core)
.collect::<Vec<_>>()
});
let permissions_profile_persistence = params
.permissions_profile_persistence
.clone()
.map(|target| {
codex_protocol::request_permissions::PermissionProfilePersistence {
profile_name: target.profile_name,
}
});
Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec {
thread_id,
thread_label,
@@ -1656,10 +1671,12 @@ impl App {
proposed_execpolicy_amendment.as_ref(),
proposed_network_policy_amendments.as_deref(),
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
)
}),
network_approval_context,
additional_permissions,
permissions_profile_persistence,
}))
}
ServerRequest::FileChangeRequestApproval { params, .. } => Some(
@@ -1713,6 +1730,14 @@ impl App {
serde_json::to_value(&params.permissions).ok()?,
)
.ok()?,
permissions_profile_persistence: params
.permissions_profile_persistence
.clone()
.map(|target| {
codex_protocol::request_permissions::PermissionProfilePersistence {
profile_name: target.profile_name,
}
}),
}),
),
_ => None,
@@ -5165,6 +5190,7 @@ mod tests {
use crate::multi_agents::AgentPickerThreadEntry;
use assert_matches::assert_matches;
use codex_app_server_protocol::AdditionalFileSystemPermissions;
use codex_app_server_protocol::AdditionalNetworkPermissions;
use codex_app_server_protocol::AdditionalPermissionProfile;
use codex_app_server_protocol::AgentMessageDeltaNotification;
@@ -5174,6 +5200,7 @@ mod tests {
use codex_app_server_protocol::NetworkApprovalProtocol as AppServerNetworkApprovalProtocol;
use codex_app_server_protocol::NetworkPolicyAmendment as AppServerNetworkPolicyAmendment;
use codex_app_server_protocol::NetworkPolicyRuleAction as AppServerNetworkPolicyRuleAction;
use codex_app_server_protocol::PermissionProfilePersistence as AppServerPermissionProfilePersistence;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
@@ -5208,6 +5235,7 @@ mod tests {
use codex_protocol::protocol::McpAuthStatus;
use codex_protocol::protocol::NetworkApprovalContext;
use codex_protocol::protocol::NetworkApprovalProtocol;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SandboxPolicy;
@@ -5216,6 +5244,7 @@ mod tests {
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::user_input::TextElement;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use crossterm::event::KeyModifiers;
use insta::assert_snapshot;
use pretty_assertions::assert_eq;
@@ -7059,6 +7088,58 @@ guardian_approval = true
);
}
#[tokio::test]
async fn inactive_thread_exec_approval_includes_persist_decision_in_fallbacks() {
let app = make_test_app().await;
let thread_id = ThreadId::new();
let mut request = exec_approval_request(thread_id, "turn-approval", "call-approval", None);
let ServerRequest::CommandExecutionRequestApproval { params, .. } = &mut request else {
panic!("expected exec approval request");
};
params.additional_permissions = Some(AdditionalPermissionProfile {
network: None,
file_system: Some(AdditionalFileSystemPermissions {
read: None,
write: Some(vec![
AbsolutePathBuf::try_from(PathBuf::from("/tmp/persist-me"))
.expect("absolute path"),
]),
}),
macos: None,
});
params.permissions_profile_persistence = Some(AppServerPermissionProfilePersistence {
profile_name: "default".to_string(),
});
let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Exec {
available_decisions,
permissions_profile_persistence,
..
})) = app
.interactive_request_for_thread_request(thread_id, &request)
.await
else {
panic!("expected exec approval request");
};
assert_eq!(
permissions_profile_persistence,
Some(
codex_protocol::request_permissions::PermissionProfilePersistence {
profile_name: "default".to_string(),
}
)
);
assert_eq!(
available_decisions,
vec![
ReviewDecision::Approved,
ReviewDecision::ApprovedPersistToProfile,
ReviewDecision::Abort,
]
);
}
#[tokio::test]
async fn inactive_thread_approval_badge_clears_after_turn_completion_notification() -> Result<()>
{
@@ -7762,6 +7843,7 @@ guardian_approval = true
cwd: Some(PathBuf::from("/tmp/project")),
command_actions: None,
additional_permissions: None,
permissions_profile_persistence: None,
skill_metadata: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
@@ -149,22 +149,27 @@ impl PendingAppServerRequests {
})
})
.transpose()?,
AppCommandView::RequestPermissionsResponse { id, response } => self
AppCommandView::RequestPermissionsResponse {
id,
response,
persist_permissions,
} => self
.permissions_approvals
.remove(id)
.map(|request_id| {
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
result: serde_json::to_value(PermissionsRequestApprovalResponse {
permissions: serde_json::from_value::<GrantedPermissionProfile>(
serde_json::to_value(&response.permissions).map_err(|err| {
format!("failed to encode granted permissions: {err}")
})?,
)
.map_err(|err| {
format!("failed to decode granted permissions for app-server: {err}")
})?,
permissions: GrantedPermissionProfile {
network: response.permissions.network.clone().map(Into::into),
file_system: response
.permissions
.file_system
.clone()
.map(Into::into),
},
scope: response.scope.into(),
persist_to_profile: persist_permissions.is_some(),
})
.map_err(|err| {
format!("failed to serialize permissions approval response: {err}")
@@ -267,6 +272,10 @@ fn file_change_decision(decision: &ReviewDecision) -> Result<FileChangeApprovalD
ReviewDecision::ApprovedForSession => Ok(FileChangeApprovalDecision::AcceptForSession),
ReviewDecision::Denied => Ok(FileChangeApprovalDecision::Decline),
ReviewDecision::Abort => Ok(FileChangeApprovalDecision::Cancel),
ReviewDecision::ApprovedPersistToProfile => Err(
"permission profile persistence is not a valid file change approval decision"
.to_string(),
),
ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
Err("execpolicy amendment is not a valid file change approval decision".to_string())
}
@@ -297,6 +306,7 @@ mod tests {
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::mcp::RequestId as McpRequestId;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::PersistPermissionProfileAction;
use codex_protocol::protocol::ReviewDecision;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -318,6 +328,7 @@ mod tests {
cwd: None,
command_actions: None,
additional_permissions: None,
permissions_profile_persistence: None,
skill_metadata: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
@@ -332,6 +343,7 @@ mod tests {
id: "approval-1".to_string(),
turn_id: None,
decision: ReviewDecision::Approved,
persist_permissions: None,
})
.expect("resolution should serialize")
.expect("request should be pending");
@@ -356,6 +368,7 @@ mod tests {
"network": { "enabled": null }
}))
.expect("valid permissions"),
permissions_profile_persistence: None,
},
}),
None
@@ -383,6 +396,7 @@ mod tests {
.expect("valid permissions"),
scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
},
persist_permissions: None,
})
.expect("permissions response should serialize")
.expect("permissions request should be pending");
@@ -396,6 +410,7 @@ mod tests {
}))
.expect("valid permissions"),
scope: PermissionGrantScope::Session,
persist_to_profile: false,
}
);
@@ -430,6 +445,72 @@ mod tests {
);
}
#[test]
fn resolves_persisted_permissions_through_app_server_request_id() {
let mut pending = PendingAppServerRequests::default();
assert_eq!(
pending.note_server_request(&ServerRequest::PermissionsRequestApproval {
request_id: AppServerRequestId::Integer(9),
params: PermissionsRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "perm-2".to_string(),
reason: None,
permissions: serde_json::from_value(json!({
"fileSystem": {
"write": ["/tmp"]
}
}))
.expect("valid permissions"),
permissions_profile_persistence: None,
},
}),
None
);
let permissions = pending
.take_resolution(&Op::RequestPermissionsResponse {
id: "perm-2".to_string(),
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: serde_json::from_value(json!({
"file_system": {
"write": ["/tmp"]
}
}))
.expect("valid permissions"),
scope: codex_protocol::request_permissions::PermissionGrantScope::Turn,
},
persist_permissions: Some(PersistPermissionProfileAction {
profile_name: "default".to_string(),
permissions: serde_json::from_value(json!({
"file_system": {
"write": ["/tmp"]
}
}))
.expect("valid permissions"),
}),
})
.expect("permissions response should serialize")
.expect("permissions request should be pending");
assert_eq!(permissions.request_id, AppServerRequestId::Integer(9));
assert_eq!(
serde_json::from_value::<PermissionsRequestApprovalResponse>(permissions.result)
.expect("permissions response should decode"),
PermissionsRequestApprovalResponse {
permissions: serde_json::from_value(json!({
"fileSystem": {
"write": ["/tmp"]
}
}))
.expect("valid permissions"),
scope: PermissionGrantScope::Turn,
persist_to_profile: true,
}
);
}
#[test]
fn correlates_mcp_elicitation_server_request_with_resolution() {
let mut pending = PendingAppServerRequests::default();
@@ -627,6 +627,7 @@ mod tests {
cwd: Some(PathBuf::from("/tmp")),
command_actions: None,
additional_permissions: None,
permissions_profile_persistence: None,
skill_metadata: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
@@ -761,6 +762,7 @@ mod tests {
id: "approval-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: ReviewDecision::Approved,
persist_permissions: None,
});
let snapshot = store.snapshot();
@@ -912,6 +914,7 @@ mod tests {
id: "call-1".to_string(),
turn_id: Some("turn-1".to_string()),
decision: ReviewDecision::Approved,
persist_permissions: None,
});
assert_eq!(store.has_pending_thread_approvals(), false);
+22 -4
View File
@@ -14,6 +14,7 @@ use codex_protocol::protocol::ConversationAudioParams;
use codex_protocol::protocol::ConversationStartParams;
use codex_protocol::protocol::ConversationTextParams;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::PersistPermissionProfileAction;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::ReviewRequest;
use codex_protocol::protocol::SandboxPolicy;
@@ -68,6 +69,7 @@ pub(crate) enum AppCommandView<'a> {
id: &'a str,
turn_id: &'a Option<String>,
decision: &'a ReviewDecision,
persist_permissions: &'a Option<PersistPermissionProfileAction>,
},
PatchApproval {
id: &'a str,
@@ -87,6 +89,7 @@ pub(crate) enum AppCommandView<'a> {
RequestPermissionsResponse {
id: &'a str,
response: &'a RequestPermissionsResponse,
persist_permissions: &'a Option<PersistPermissionProfileAction>,
},
ReloadUserConfig,
ListSkills {
@@ -203,11 +206,13 @@ impl AppCommand {
id: String,
turn_id: Option<String>,
decision: ReviewDecision,
persist_permissions: Option<PersistPermissionProfileAction>,
) -> Self {
Self(Op::ExecApproval {
id,
turn_id,
decision,
persist_permissions,
})
}
@@ -238,8 +243,13 @@ impl AppCommand {
pub(crate) fn request_permissions_response(
id: String,
response: RequestPermissionsResponse,
persist_permissions: Option<PersistPermissionProfileAction>,
) -> Self {
Self(Op::RequestPermissionsResponse { id, response })
Self(Op::RequestPermissionsResponse {
id,
response,
persist_permissions,
})
}
pub(crate) fn reload_user_config() -> Self {
@@ -353,10 +363,12 @@ impl AppCommand {
id,
turn_id,
decision,
persist_permissions,
} => AppCommandView::ExecApproval {
id,
turn_id,
decision,
persist_permissions,
},
Op::PatchApproval { id, decision } => AppCommandView::PatchApproval { id, decision },
Op::ResolveElicitation {
@@ -375,9 +387,15 @@ impl AppCommand {
Op::UserInputAnswer { id, response } => {
AppCommandView::UserInputAnswer { id, response }
}
Op::RequestPermissionsResponse { id, response } => {
AppCommandView::RequestPermissionsResponse { id, response }
}
Op::RequestPermissionsResponse {
id,
response,
persist_permissions,
} => AppCommandView::RequestPermissionsResponse {
id,
response,
persist_permissions,
},
Op::ReloadUserConfig => AppCommandView::ReloadUserConfig,
Op::ListSkills { cwds, force_reload } => AppCommandView::ListSkills {
cwds,
@@ -5,6 +5,7 @@ use codex_protocol::ThreadId;
use codex_protocol::approvals::ElicitationAction;
use codex_protocol::mcp::RequestId as McpRequestId;
use codex_protocol::protocol::ConversationAudioParams;
use codex_protocol::protocol::PersistPermissionProfileAction;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::ReviewRequest;
use codex_protocol::request_permissions::RequestPermissionsResponse;
@@ -79,10 +80,17 @@ impl AppEventSender {
));
}
pub(crate) fn exec_approval(&self, thread_id: ThreadId, id: String, decision: ReviewDecision) {
pub(crate) fn exec_approval(
&self,
thread_id: ThreadId,
id: String,
decision: ReviewDecision,
persist_permissions: Option<PersistPermissionProfileAction>,
) {
self.send(AppEvent::SubmitThreadOp {
thread_id,
op: AppCommand::exec_approval(id, /*turn_id*/ None, decision).into_core(),
op: AppCommand::exec_approval(id, /*turn_id*/ None, decision, persist_permissions)
.into_core(),
});
}
@@ -91,10 +99,12 @@ impl AppEventSender {
thread_id: ThreadId,
id: String,
response: RequestPermissionsResponse,
persist_permissions: Option<PersistPermissionProfileAction>,
) {
self.send(AppEvent::SubmitThreadOp {
thread_id,
op: AppCommand::request_permissions_response(id, response).into_core(),
op: AppCommand::request_permissions_response(id, response, persist_permissions)
.into_core(),
});
}
@@ -29,8 +29,10 @@ use codex_protocol::protocol::NetworkApprovalContext;
use codex_protocol::protocol::NetworkPolicyRuleAction;
#[cfg(test)]
use codex_protocol::protocol::Op;
use codex_protocol::protocol::PersistPermissionProfileAction;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::PermissionProfilePersistence;
use codex_protocol::request_permissions::RequestPermissionProfile;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
@@ -56,6 +58,7 @@ pub(crate) enum ApprovalRequest {
available_decisions: Vec<ReviewDecision>,
network_approval_context: Option<NetworkApprovalContext>,
additional_permissions: Option<PermissionProfile>,
permissions_profile_persistence: Option<PermissionProfilePersistence>,
},
Permissions {
thread_id: ThreadId,
@@ -63,6 +66,7 @@ pub(crate) enum ApprovalRequest {
call_id: String,
reason: Option<String>,
permissions: RequestPermissionProfile,
permissions_profile_persistence: Option<PermissionProfilePersistence>,
},
ApplyPatch {
thread_id: ThreadId,
@@ -152,12 +156,14 @@ impl ApprovalOverlay {
available_decisions,
network_approval_context,
additional_permissions,
permissions_profile_persistence,
..
} => (
exec_options(
available_decisions,
network_approval_context.as_ref(),
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
),
network_approval_context.as_ref().map_or_else(
|| "Would you like to run the following command?".to_string(),
@@ -169,8 +175,11 @@ impl ApprovalOverlay {
},
),
),
ApprovalRequest::Permissions { .. } => (
permissions_options(),
ApprovalRequest::Permissions {
permissions_profile_persistence,
..
} => (
permissions_options(permissions_profile_persistence.as_ref()),
"Would you like to grant these permissions?".to_string(),
),
ApprovalRequest::ApplyPatch { .. } => (
@@ -220,17 +229,38 @@ impl ApprovalOverlay {
};
if let Some(request) = self.current_request.as_ref() {
match (request, &option.decision) {
(ApprovalRequest::Exec { id, command, .. }, ApprovalDecision::Review(decision)) => {
self.handle_exec_decision(id, command, decision.clone());
(
ApprovalRequest::Exec {
id,
command,
additional_permissions,
permissions_profile_persistence,
..
},
ApprovalDecision::Review(decision),
) => {
self.handle_exec_decision(
id,
command,
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
decision.clone(),
);
}
(
ApprovalRequest::Permissions {
call_id,
permissions,
permissions_profile_persistence,
..
},
ApprovalDecision::Review(decision),
) => self.handle_permissions_decision(call_id, permissions, decision.clone()),
) => self.handle_permissions_decision(
call_id,
permissions,
permissions_profile_persistence.as_ref(),
decision.clone(),
),
(ApprovalRequest::ApplyPatch { id, .. }, ApprovalDecision::Review(decision)) => {
self.handle_patch_decision(id, decision.clone());
}
@@ -252,7 +282,14 @@ impl ApprovalOverlay {
self.advance_queue();
}
fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
fn handle_exec_decision(
&self,
id: &str,
command: &[String],
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
decision: ReviewDecision,
) {
let Some(request) = self.current_request.as_ref() else {
return;
};
@@ -265,21 +302,29 @@ impl ApprovalOverlay {
self.app_event_tx.send(AppEvent::InsertHistoryCell(cell));
}
let thread_id = request.thread_id();
let persist_permissions = persist_permissions_for_exec_decision(
&decision,
additional_permissions,
permissions_profile_persistence,
);
self.app_event_tx
.exec_approval(thread_id, id.to_string(), decision);
.exec_approval(thread_id, id.to_string(), decision, persist_permissions);
}
fn handle_permissions_decision(
&self,
call_id: &str,
permissions: &RequestPermissionProfile,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
decision: ReviewDecision,
) {
let Some(request) = self.current_request.as_ref() else {
return;
};
let granted_permissions = match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => permissions.clone(),
ReviewDecision::Approved
| ReviewDecision::ApprovedForSession
| ReviewDecision::ApprovedPersistToProfile => permissions.clone(),
ReviewDecision::Denied | ReviewDecision::Abort => Default::default(),
ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => Default::default(),
@@ -302,6 +347,11 @@ impl ApprovalOverlay {
)));
}
let thread_id = request.thread_id();
let persist_permissions = persist_permissions_for_permissions_decision(
&decision,
permissions_profile_persistence,
&granted_permissions,
);
self.app_event_tx.request_permissions_response(
thread_id,
call_id.to_string(),
@@ -309,6 +359,7 @@ impl ApprovalOverlay {
permissions: granted_permissions,
scope,
},
persist_permissions,
);
}
@@ -423,15 +474,33 @@ impl BottomPaneView for ApprovalOverlay {
&& let Some(request) = self.current_request.as_ref()
{
match request {
ApprovalRequest::Exec { id, command, .. } => {
self.handle_exec_decision(id, command, ReviewDecision::Abort);
ApprovalRequest::Exec {
id,
command,
additional_permissions,
permissions_profile_persistence,
..
} => {
self.handle_exec_decision(
id,
command,
additional_permissions.as_ref(),
permissions_profile_persistence.as_ref(),
ReviewDecision::Abort,
);
}
ApprovalRequest::Permissions {
call_id,
permissions,
permissions_profile_persistence,
..
} => {
self.handle_permissions_decision(call_id, permissions, ReviewDecision::Abort);
self.handle_permissions_decision(
call_id,
permissions,
permissions_profile_persistence.as_ref(),
ReviewDecision::Abort,
);
}
ApprovalRequest::ApplyPatch { id, .. } => {
self.handle_patch_decision(id, ReviewDecision::Abort);
@@ -647,6 +716,7 @@ fn exec_options(
available_decisions: &[ReviewDecision],
network_approval_context: Option<&NetworkApprovalContext>,
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Vec<ApprovalOption> {
available_decisions
.iter()
@@ -695,6 +765,14 @@ fn exec_options(
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
}),
ReviewDecision::ApprovedPersistToProfile => {
permissions_profile_persistence.map(|_| ApprovalOption {
label: "Yes, always allow these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedPersistToProfile),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
})
}
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => {
@@ -841,8 +919,10 @@ fn patch_options() -> Vec<ApprovalOption> {
]
}
fn permissions_options() -> Vec<ApprovalOption> {
vec![
fn permissions_options(
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Vec<ApprovalOption> {
let mut options = vec![
ApprovalOption {
label: "Yes, grant these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::Approved),
@@ -861,7 +941,50 @@ fn permissions_options() -> Vec<ApprovalOption> {
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))],
},
]
];
if permissions_profile_persistence.is_some() {
options.insert(
1,
ApprovalOption {
label: "Yes, always allow these permissions".to_string(),
decision: ApprovalDecision::Review(ReviewDecision::ApprovedPersistToProfile),
display_shortcut: None,
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))],
},
);
}
options
}
fn persist_permissions_for_exec_decision(
decision: &ReviewDecision,
additional_permissions: Option<&PermissionProfile>,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
) -> Option<PersistPermissionProfileAction> {
if !matches!(decision, ReviewDecision::ApprovedPersistToProfile) {
return None;
}
let permissions = additional_permissions?.clone();
let profile_name = permissions_profile_persistence?.profile_name.clone();
Some(PersistPermissionProfileAction {
profile_name,
permissions,
})
}
fn persist_permissions_for_permissions_decision(
decision: &ReviewDecision,
permissions_profile_persistence: Option<&PermissionProfilePersistence>,
granted_permissions: &RequestPermissionProfile,
) -> Option<PersistPermissionProfileAction> {
if !matches!(decision, ReviewDecision::ApprovedPersistToProfile) {
return None;
}
let profile_name = permissions_profile_persistence?.profile_name.clone();
Some(PersistPermissionProfileAction {
profile_name,
permissions: granted_permissions.clone().into(),
})
}
fn elicitation_options() -> Vec<ApprovalOption> {
@@ -945,6 +1068,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
}
}
@@ -963,6 +1087,7 @@ mod tests {
write: Some(vec![absolute_path("/tmp/out.txt")]),
}),
},
permissions_profile_persistence: None,
}
}
@@ -1010,6 +1135,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1038,6 +1164,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1071,6 +1198,7 @@ mod tests {
],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1128,6 +1256,7 @@ mod tests {
protocol: NetworkApprovalProtocol::Https,
}),
additional_permissions: None,
permissions_profile_persistence: None,
},
tx,
Features::with_defaults(),
@@ -1154,6 +1283,7 @@ mod tests {
available_decisions: vec![ReviewDecision::Approved, ReviewDecision::Abort],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1195,6 +1325,7 @@ mod tests {
],
Some(&network_context),
None,
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1219,6 +1350,7 @@ mod tests {
],
None,
None,
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1245,6 +1377,7 @@ mod tests {
&[ReviewDecision::Approved, ReviewDecision::Abort],
None,
Some(&additional_permissions),
None,
);
let labels: Vec<String> = options.into_iter().map(|option| option.label).collect();
@@ -1259,7 +1392,7 @@ mod tests {
#[test]
fn permissions_options_use_expected_labels() {
let labels: Vec<String> = permissions_options()
let labels: Vec<String> = permissions_options(None)
.into_iter()
.map(|option| option.label)
.collect();
@@ -1322,6 +1455,7 @@ mod tests {
}),
..Default::default()
}),
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1370,6 +1504,7 @@ mod tests {
}),
..Default::default()
}),
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1417,6 +1552,7 @@ mod tests {
}),
..Default::default()
}),
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1452,6 +1588,7 @@ mod tests {
protocol: NetworkApprovalProtocol::Https,
}),
additional_permissions: None,
permissions_profile_persistence: None,
};
let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults());
@@ -1286,6 +1286,7 @@ mod tests {
],
network_approval_context: None,
additional_permissions: None,
permissions_profile_persistence: None,
}
}
+15
View File
@@ -1228,6 +1228,11 @@ fn exec_approval_request_from_params(
.network_approval_context
.and_then(convert_via_json),
additional_permissions: params.additional_permissions.and_then(convert_via_json),
permissions_profile_persistence: params.permissions_profile_persistence.map(|target| {
codex_protocol::request_permissions::PermissionProfilePersistence {
profile_name: target.profile_name,
}
}),
turn_id: params.turn_id,
approval_id: params.approval_id,
proposed_execpolicy_amendment: params
@@ -1256,6 +1261,9 @@ fn exec_approval_request_from_params(
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptForSession => {
codex_protocol::protocol::ReviewDecision::ApprovedForSession
}
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptAndPersist => {
codex_protocol::protocol::ReviewDecision::ApprovedPersistToProfile
}
codex_app_server_protocol::CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
execpolicy_amendment,
} => codex_protocol::protocol::ReviewDecision::ApprovedExecpolicyAmendment {
@@ -1391,6 +1399,11 @@ fn request_permissions_from_params(
serde_json::to_value(params.permissions).unwrap_or(serde_json::Value::Null),
)
.unwrap_or_default(),
permissions_profile_persistence: params.permissions_profile_persistence.map(|target| {
codex_protocol::request_permissions::PermissionProfilePersistence {
profile_name: target.profile_name,
}
}),
}
}
@@ -3906,6 +3919,7 @@ impl ChatWidget {
available_decisions,
network_approval_context: ev.network_approval_context,
additional_permissions: ev.additional_permissions,
permissions_profile_persistence: ev.permissions_profile_persistence,
};
self.bottom_pane
.push_approval_request(request, &self.config.features);
@@ -3990,6 +4004,7 @@ impl ChatWidget {
call_id: ev.call_id,
reason: ev.reason,
permissions: ev.permissions,
permissions_profile_persistence: ev.permissions_profile_persistence,
};
self.bottom_pane
.push_approval_request(request, &self.config.features);
@@ -3324,6 +3324,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -3375,6 +3376,7 @@ async fn exec_approval_uses_approval_id_when_present() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
}),
});
@@ -3417,6 +3419,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -3473,6 +3476,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9578,6 +9582,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9640,6 +9645,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -9689,6 +9695,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -10073,6 +10080,7 @@ async fn status_widget_and_approval_modal_snapshot() {
additional_permissions: None,
skill_metadata: None,
available_decisions: None,
permissions_profile_persistence: None,
parsed_cmd: vec![],
};
chat.handle_codex_event(Event {
@@ -819,6 +819,19 @@ pub fn new_approval_decision_cell(
],
)
}
ApprovedPersistToProfile => {
let snippet = Span::from(exec_snippet(&command)).dim();
(
"✔ ".green(),
vec![
actor.subject().into(),
"approved".bold(),
" codex to run ".into(),
snippet,
" and saved those permissions".into(),
],
)
}
ApprovedExecpolicyAmendment {
proposed_execpolicy_amendment,
} => {