mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
tui: expose auto-review permission grant choices
This commit is contained in:
@@ -210,6 +210,7 @@ impl PendingAppServerRequests {
|
||||
),
|
||||
scope: response.scope.into(),
|
||||
strict_auto_review: response.strict_auto_review.then_some(true),
|
||||
approvals_reviewer: response.approvals_reviewer.map(Into::into),
|
||||
})
|
||||
.map_err(|err| {
|
||||
format!("failed to serialize permissions approval response: {err}")
|
||||
@@ -529,6 +530,7 @@ mod tests {
|
||||
},
|
||||
scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
|
||||
strict_auto_review: false,
|
||||
approvals_reviewer: None,
|
||||
},
|
||||
})
|
||||
.expect("permissions response should serialize")
|
||||
@@ -564,6 +566,7 @@ mod tests {
|
||||
},
|
||||
scope: PermissionGrantScope::Session,
|
||||
strict_auto_review: None,
|
||||
approvals_reviewer: None,
|
||||
}
|
||||
);
|
||||
|
||||
|
||||
@@ -493,13 +493,6 @@ impl App {
|
||||
};
|
||||
if write_response.status == WriteStatus::OkOverridden {
|
||||
let message = overridden_write_message(&write_response);
|
||||
tracing::warn!(
|
||||
message,
|
||||
"feature flag config write was overridden by effective config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Experimental feature changes were saved but not applied: {message}"
|
||||
));
|
||||
if let Some(effective_config) = self
|
||||
.read_effective_config_after_overridden_write(
|
||||
app_server,
|
||||
@@ -507,10 +500,55 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
let guardian_disabled_as_requested = feature_updates_to_apply
|
||||
.iter()
|
||||
.any(|(feature, enabled)| *feature == Feature::GuardianApproval && !*enabled)
|
||||
&& !feature_enabled_from_effective_config(
|
||||
&effective_config,
|
||||
Feature::GuardianApproval,
|
||||
);
|
||||
if !guardian_disabled_as_requested {
|
||||
tracing::warn!(
|
||||
message,
|
||||
"feature flag config write was overridden by effective config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Experimental feature changes were saved but not applied: {message}"
|
||||
));
|
||||
}
|
||||
self.sync_feature_state_from_effective_config(
|
||||
&effective_config,
|
||||
&feature_updates_to_apply,
|
||||
);
|
||||
let guardian_disabled = feature_updates_to_apply
|
||||
.iter()
|
||||
.any(|(feature, enabled)| *feature == Feature::GuardianApproval && !*enabled)
|
||||
&& !self.config.features.enabled(Feature::GuardianApproval);
|
||||
if guardian_disabled {
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
let op = AppCommand::override_turn_context(
|
||||
/*cwd*/ None,
|
||||
/*approval_policy*/ None,
|
||||
Some(self.config.approvals_reviewer),
|
||||
/*permission_profile*/ None,
|
||||
/*active_permission_profile*/ None,
|
||||
/*windows_sandbox_level*/ None,
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
);
|
||||
let replay_state_op = ThreadEventStore::op_can_change_pending_replay_state(&op)
|
||||
.then(|| op.clone());
|
||||
let submitted = self.chat_widget.submit_op(op);
|
||||
if submitted && let Some(op) = replay_state_op.as_ref() {
|
||||
self.note_active_thread_outbound_op(op).await;
|
||||
self.refresh_pending_thread_approvals().await;
|
||||
}
|
||||
}
|
||||
self.sync_auto_review_runtime_state_from_effective_config(
|
||||
&effective_config,
|
||||
&feature_updates_to_apply,
|
||||
@@ -519,6 +557,12 @@ impl App {
|
||||
if windows_sandbox_changed {
|
||||
self.propagate_windows_sandbox_turn_context();
|
||||
}
|
||||
if guardian_disabled_as_requested && let Some(label) = permissions_history_label {
|
||||
self.chat_widget.add_info_message(
|
||||
format!("Permissions updated to {label}"),
|
||||
/*hint*/ None,
|
||||
);
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -52,6 +52,7 @@ use codex_app_server_protocol::NetworkPolicyRuleAction;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_features::Features;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -396,24 +397,35 @@ impl ApprovalOverlay {
|
||||
};
|
||||
let granted_permissions = match decision {
|
||||
PermissionsDecision::GrantForTurn
|
||||
| PermissionsDecision::GrantForTurnWithStrictAutoReview
|
||||
| PermissionsDecision::GrantForSession => permissions.clone(),
|
||||
| PermissionsDecision::GrantForTurnWithAutoReview
|
||||
| PermissionsDecision::GrantForSession
|
||||
| PermissionsDecision::GrantForSessionWithAutoReview => permissions.clone(),
|
||||
PermissionsDecision::Deny => Default::default(),
|
||||
};
|
||||
let scope = if matches!(decision, PermissionsDecision::GrantForSession) {
|
||||
let scope = if matches!(
|
||||
decision,
|
||||
PermissionsDecision::GrantForSession
|
||||
| PermissionsDecision::GrantForSessionWithAutoReview
|
||||
) {
|
||||
PermissionGrantScope::Session
|
||||
} else {
|
||||
PermissionGrantScope::Turn
|
||||
};
|
||||
let strict_auto_review = matches!(
|
||||
let approvals_reviewer = matches!(
|
||||
decision,
|
||||
PermissionsDecision::GrantForTurnWithStrictAutoReview
|
||||
);
|
||||
PermissionsDecision::GrantForTurnWithAutoReview
|
||||
| PermissionsDecision::GrantForSessionWithAutoReview
|
||||
)
|
||||
.then_some(ApprovalsReviewer::AutoReview);
|
||||
if request.thread_label().is_none() {
|
||||
let message = if granted_permissions.is_empty() {
|
||||
"You did not grant additional permissions"
|
||||
} else if strict_auto_review {
|
||||
"You granted additional permissions with strict auto review"
|
||||
} else if approvals_reviewer == Some(ApprovalsReviewer::AutoReview)
|
||||
&& matches!(scope, PermissionGrantScope::Session)
|
||||
{
|
||||
"You granted additional permissions for this session with auto review for this turn"
|
||||
} else if approvals_reviewer == Some(ApprovalsReviewer::AutoReview) {
|
||||
"You granted additional permissions with auto review for this turn"
|
||||
} else if matches!(scope, PermissionGrantScope::Session) {
|
||||
"You granted additional permissions for this session"
|
||||
} else {
|
||||
@@ -430,7 +442,8 @@ impl ApprovalOverlay {
|
||||
codex_protocol::request_permissions::RequestPermissionsResponse {
|
||||
permissions: granted_permissions,
|
||||
scope,
|
||||
strict_auto_review,
|
||||
strict_auto_review: false,
|
||||
approvals_reviewer,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -810,8 +823,9 @@ enum ApprovalDecision {
|
||||
#[derive(Clone, Copy)]
|
||||
enum PermissionsDecision {
|
||||
GrantForTurn,
|
||||
GrantForTurnWithStrictAutoReview,
|
||||
GrantForTurnWithAutoReview,
|
||||
GrantForSession,
|
||||
GrantForSessionWithAutoReview,
|
||||
Deny,
|
||||
}
|
||||
|
||||
@@ -1061,9 +1075,9 @@ fn permissions_options(keymap: &ApprovalKeymap) -> Vec<ApprovalOption> {
|
||||
shortcuts: keymap.approve.clone(),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, grant for this turn with strict auto review".to_string(),
|
||||
label: "Yes, grant and use auto review for this turn".to_string(),
|
||||
decision: ApprovalDecision::Permissions(
|
||||
PermissionsDecision::GrantForTurnWithStrictAutoReview,
|
||||
PermissionsDecision::GrantForTurnWithAutoReview,
|
||||
),
|
||||
shortcuts: vec![key_hint::plain(KeyCode::Char('r'))],
|
||||
},
|
||||
@@ -1072,6 +1086,13 @@ fn permissions_options(keymap: &ApprovalKeymap) -> Vec<ApprovalOption> {
|
||||
decision: ApprovalDecision::Permissions(PermissionsDecision::GrantForSession),
|
||||
shortcuts: keymap.approve_for_session.clone(),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, grant for this session and use auto review this turn".to_string(),
|
||||
decision: ApprovalDecision::Permissions(
|
||||
PermissionsDecision::GrantForSessionWithAutoReview,
|
||||
),
|
||||
shortcuts: Vec::new(),
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "No, continue without permissions".to_string(),
|
||||
decision: ApprovalDecision::Permissions(PermissionsDecision::Deny),
|
||||
@@ -1805,8 +1826,9 @@ mod tests {
|
||||
labels,
|
||||
vec![
|
||||
"Yes, grant these permissions for this turn".to_string(),
|
||||
"Yes, grant for this turn with strict auto review".to_string(),
|
||||
"Yes, grant and use auto review for this turn".to_string(),
|
||||
"Yes, grant these permissions for this session".to_string(),
|
||||
"Yes, grant for this session and use auto review this turn".to_string(),
|
||||
"No, continue without permissions".to_string(),
|
||||
]
|
||||
);
|
||||
@@ -1884,6 +1906,7 @@ mod tests {
|
||||
} = ev
|
||||
{
|
||||
assert_eq!(response.scope, PermissionGrantScope::Session);
|
||||
assert_eq!(response.approvals_reviewer, None);
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
@@ -1921,6 +1944,7 @@ mod tests {
|
||||
assert!(response.permissions.is_empty());
|
||||
assert_eq!(response.scope, PermissionGrantScope::Turn);
|
||||
assert!(!response.strict_auto_review);
|
||||
assert_eq!(response.approvals_reviewer, None);
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
@@ -1932,7 +1956,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_strict_auto_review_shortcut_submits_turn_scope_with_strict_review() {
|
||||
fn permissions_auto_review_shortcut_submits_turn_scope_with_reviewer_override() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = make_overlay(make_permissions_request(), tx, Features::with_defaults());
|
||||
@@ -1947,14 +1971,52 @@ mod tests {
|
||||
} = ev
|
||||
{
|
||||
assert_eq!(response.scope, PermissionGrantScope::Turn);
|
||||
assert!(response.strict_auto_review);
|
||||
assert!(!response.strict_auto_review);
|
||||
assert_eq!(
|
||||
response.approvals_reviewer,
|
||||
Some(ApprovalsReviewer::AutoReview)
|
||||
);
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
saw_op,
|
||||
"expected permission approval decision to emit a strict auto review response"
|
||||
"expected permission approval decision to emit an auto-review response"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_session_auto_review_option_combines_session_scope_with_reviewer_override() {
|
||||
let (tx, mut rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx);
|
||||
let mut view = make_overlay(make_permissions_request(), tx, Features::with_defaults());
|
||||
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
let mut saw_op = false;
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::SubmitThreadOp {
|
||||
op: Op::RequestPermissionsResponse { response, .. },
|
||||
..
|
||||
} = ev
|
||||
{
|
||||
assert_eq!(response.scope, PermissionGrantScope::Session);
|
||||
assert!(!response.strict_auto_review);
|
||||
assert_eq!(
|
||||
response.approvals_reviewer,
|
||||
Some(ApprovalsReviewer::AutoReview)
|
||||
);
|
||||
saw_op = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
saw_op,
|
||||
"expected session permission approval to emit an auto-review response"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+3
-3
@@ -2,7 +2,6 @@
|
||||
source: tui/src/bottom_pane/approval_overlay.rs
|
||||
expression: "normalize_snapshot_paths(render_overlay_lines(&view, 120))"
|
||||
---
|
||||
|
||||
Would you like to grant these permissions?
|
||||
|
||||
Reason: need workspace access
|
||||
@@ -10,8 +9,9 @@ expression: "normalize_snapshot_paths(render_overlay_lines(&view, 120))"
|
||||
Permission rule: network; read `/tmp/readme.txt`; write `/tmp/out.txt`
|
||||
|
||||
› 1. Yes, grant these permissions for this turn (y)
|
||||
2. Yes, grant for this turn with strict auto review (r)
|
||||
2. Yes, grant and use auto review for this turn (r)
|
||||
3. Yes, grant these permissions for this session (a)
|
||||
4. No, continue without permissions (d)
|
||||
4. Yes, grant for this session and use auto review this turn
|
||||
5. No, continue without permissions (d)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
Reference in New Issue
Block a user