mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
feat: include available decisions in command approval requests
This commit is contained in:
@@ -2590,9 +2590,13 @@ impl Session {
|
||||
|
||||
/// Emit an exec approval request event and await the user's decision.
|
||||
///
|
||||
/// The request is keyed by `call_id` + `approval_id` so matching responses are delivered
|
||||
/// to the correct in-flight turn. If the task is aborted, this returns the
|
||||
/// default `ReviewDecision` (`Denied`).
|
||||
/// The request is keyed by `call_id` + `approval_id` so matching responses
|
||||
/// are delivered to the correct in-flight turn. If the task is aborted,
|
||||
/// this returns the default `ReviewDecision` (`Denied`).
|
||||
///
|
||||
/// Note that if `available_decisions` is `None`, then the other fields will
|
||||
/// be used to derive the available decisions via
|
||||
/// [ExecApprovalRequestEvent::default_available_decisions].
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn request_command_approval(
|
||||
&self,
|
||||
@@ -2605,6 +2609,7 @@ impl Session {
|
||||
network_approval_context: Option<NetworkApprovalContext>,
|
||||
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
available_decisions: Option<Vec<ReviewDecision>>,
|
||||
) -> ReviewDecision {
|
||||
// command-level approvals use `call_id`.
|
||||
// `approval_id` is only present for subcommand callbacks (execve intercept)
|
||||
@@ -2638,6 +2643,14 @@ impl Session {
|
||||
},
|
||||
]
|
||||
});
|
||||
let available_decisions = available_decisions.unwrap_or_else(|| {
|
||||
ExecApprovalRequestEvent::default_available_decisions(
|
||||
network_approval_context.as_ref(),
|
||||
proposed_execpolicy_amendment.as_ref(),
|
||||
proposed_network_policy_amendments.as_deref(),
|
||||
additional_permissions.as_ref(),
|
||||
)
|
||||
});
|
||||
let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
|
||||
call_id,
|
||||
approval_id,
|
||||
@@ -2649,6 +2662,7 @@ impl Session {
|
||||
proposed_execpolicy_amendment,
|
||||
proposed_network_policy_amendments,
|
||||
additional_permissions,
|
||||
available_decisions: Some(available_decisions),
|
||||
parsed_cmd,
|
||||
});
|
||||
self.send_event(turn_context, event).await;
|
||||
|
||||
@@ -322,6 +322,7 @@ async fn handle_exec_approval(
|
||||
network_approval_context,
|
||||
proposed_execpolicy_amendment,
|
||||
additional_permissions,
|
||||
available_decisions,
|
||||
..
|
||||
} = event;
|
||||
// Race approval with cancellation and timeout to avoid hangs.
|
||||
@@ -335,6 +336,7 @@ async fn handle_exec_approval(
|
||||
network_approval_context,
|
||||
proposed_execpolicy_amendment,
|
||||
additional_permissions,
|
||||
available_decisions,
|
||||
);
|
||||
let decision = await_approval_with_cancel(
|
||||
approval_fut,
|
||||
|
||||
@@ -331,6 +331,7 @@ impl NetworkApprovalService {
|
||||
protocol,
|
||||
};
|
||||
|
||||
let available_decisions = None;
|
||||
let approval_decision = session
|
||||
.request_command_approval(
|
||||
turn_context.as_ref(),
|
||||
@@ -342,6 +343,7 @@ impl NetworkApprovalService {
|
||||
Some(network_approval_context.clone()),
|
||||
None,
|
||||
None,
|
||||
available_decisions,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -150,6 +150,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
let call_id = ctx.call_id.to_string();
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, "shell", keys, move || async move {
|
||||
let available_decisions = None;
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
@@ -163,6 +164,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
req.additional_permissions.clone(),
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
})
|
||||
|
||||
@@ -189,6 +189,7 @@ impl CoreShellActionProvider {
|
||||
workdir: &AbsolutePathBuf,
|
||||
stopwatch: &Stopwatch,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
decision_source: &DecisionSource,
|
||||
) -> anyhow::Result<ReviewDecision> {
|
||||
let command = join_program_and_argv(program, argv);
|
||||
let workdir = workdir.to_path_buf();
|
||||
@@ -198,6 +199,20 @@ impl CoreShellActionProvider {
|
||||
let approval_id = Some(Uuid::new_v4().to_string());
|
||||
Ok(stopwatch
|
||||
.pause_for(async move {
|
||||
let available_decisions = vec![
|
||||
Some(ReviewDecision::Approved),
|
||||
// Currently, ApprovedForSession is only honored for skills,
|
||||
// so only offer it for skill script approvals.
|
||||
if matches!(decision_source, DecisionSource::SkillScript { .. }) {
|
||||
Some(ReviewDecision::ApprovedForSession)
|
||||
} else {
|
||||
None
|
||||
},
|
||||
Some(ReviewDecision::Abort),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.collect();
|
||||
session
|
||||
.request_command_approval(
|
||||
&turn,
|
||||
@@ -209,6 +224,7 @@ impl CoreShellActionProvider {
|
||||
None,
|
||||
None,
|
||||
additional_permissions,
|
||||
Some(available_decisions),
|
||||
)
|
||||
.await
|
||||
})
|
||||
@@ -273,6 +289,7 @@ impl CoreShellActionProvider {
|
||||
workdir,
|
||||
&self.stopwatch,
|
||||
additional_permissions,
|
||||
&decision_source,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
|
||||
@@ -111,6 +111,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
.or_else(|| req.justification.clone());
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, "unified_exec", keys, || async move {
|
||||
let available_decisions = None;
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
@@ -124,6 +125,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
.proposed_execpolicy_amendment()
|
||||
.cloned(),
|
||||
req.additional_permissions.clone(),
|
||||
available_decisions,
|
||||
)
|
||||
.await
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user