mirror of
https://github.com/openai/codex.git
synced 2026-10-19 08:16:48 +00:00
Use server-authoritative permissions in the TUI (#49472)
## Why Client permission definitions and constraints can differ from those on the connected app server. Accepted permission changes also need to survive task transitions before their settings notifications arrive. ## What changed - Discover permission profiles and availability from the connected server and apply selections through `thread/settings/update`, including custom profiles. - Adopt the server's effective permission settings instead of reapplying local definitions or constraints. - Retain accepted selections and effective approval settings across new tasks, forks, side conversations, directory changes, reconnects, and safety retries without blocking navigation on a settings notification. - Recognize the current built-in permission preset when the server supplies an unnamed profile. ## Testing Add regression coverage for conflicting client/server profile definitions, rejected and unchanged selections, consecutive requests with omitted approval settings, and permission carryover before confirmation. Extend discovery and menu tests to verify server-provided availability and unnamed-profile highlighting. GitOrigin-RevId: bcd767b58dc92dc4f945189078e51e6e5fce6d24
This commit is contained in:
@@ -59,6 +59,8 @@ pub(super) struct AgentsOverviewState {
|
||||
Arc<std::sync::Mutex<super::agents_overview_view::AgentsOverviewViewState>>,
|
||||
/// Explicit permission-profile choices for new-session carryover, retained across navigation.
|
||||
pub(super) selected_permission_profiles: HashMap<ThreadId, String>,
|
||||
/// Accepted menu requests; unchanged settings may never produce a notification.
|
||||
pub(super) requested_permission_profiles: HashMap<ThreadId, PermissionProfileSelection>,
|
||||
/// Keep new tasks subscribed and reusable until a first turn makes them resumable.
|
||||
pub(super) blank_sessions: HashMap<ThreadId, crate::app_server_session::AppServerStartedThread>,
|
||||
pub(super) input_states: HashMap<ThreadId, ThreadInputState>,
|
||||
@@ -956,6 +958,15 @@ impl App {
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.is_some_and(|active| !active.id.starts_with(':'))
|
||||
&& self.chat_widget.thread_id().is_none_or(|thread_id| {
|
||||
self.agents_overview
|
||||
.selected_permission_profiles
|
||||
.get(&thread_id)
|
||||
!= profile
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.map(|active| &active.id)
|
||||
})
|
||||
&& (!profile.matches_config(&config)
|
||||
|| config.permissions.profile_workspace_roots()
|
||||
!= self.config.permissions.profile_workspace_roots())
|
||||
|
||||
@@ -121,6 +121,13 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
};
|
||||
let selected_profile = self.chat_widget.thread_id().and_then(|thread_id| {
|
||||
if let Some(selection) = self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id)
|
||||
{
|
||||
return Some(selection.clone());
|
||||
}
|
||||
let source = self.chat_widget.config_ref();
|
||||
let active = source.permissions.active_permission_profile()?;
|
||||
(self
|
||||
|
||||
@@ -119,6 +119,9 @@ impl App {
|
||||
self.agents_overview.threads.insert(thread_id, Some(thread));
|
||||
}
|
||||
ServerNotification::ThreadArchived(_) | ServerNotification::ThreadDeleted(_) => {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&thread_id);
|
||||
self.agents_overview
|
||||
.selected_permission_profiles
|
||||
.remove(&thread_id);
|
||||
@@ -132,6 +135,9 @@ impl App {
|
||||
self.agents_overview.removed_threads.remove(&thread_id);
|
||||
}
|
||||
ServerNotification::ThreadClosed(_) => {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&thread_id);
|
||||
self.agents_overview.activity.remove(&thread_id);
|
||||
if let Some(usage) = self.agents_overview.usage.get_mut(&thread_id) {
|
||||
usage.tokens = None;
|
||||
@@ -160,6 +166,10 @@ impl App {
|
||||
}
|
||||
ServerNotification::ThreadSettingsUpdated(settings) => {
|
||||
if !self.pending_server_profiles.contains_key(&thread_id)
|
||||
&& !self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
&& self
|
||||
.agents_overview
|
||||
.selected_permission_profiles
|
||||
|
||||
@@ -113,6 +113,7 @@ impl App {
|
||||
.await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(super) async fn rebuild_config_for_permission_profile(
|
||||
&self,
|
||||
profile_id: &str,
|
||||
@@ -136,6 +137,8 @@ impl App {
|
||||
.await
|
||||
}
|
||||
|
||||
// Local setup for startup fixtures; connected selections always use the server below.
|
||||
#[cfg(test)]
|
||||
pub(super) async fn apply_permission_profile_selection(
|
||||
&mut self,
|
||||
selection: PermissionProfileSelection,
|
||||
@@ -266,25 +269,11 @@ impl App {
|
||||
pub(super) async fn select_permission_profile(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
selection: PermissionProfileSelection,
|
||||
mut selection: PermissionProfileSelection,
|
||||
) {
|
||||
if self.reject_pending_permission_change() {
|
||||
return;
|
||||
}
|
||||
if (self.chat_widget.thread_id().is_none() && selection.profile_id.starts_with(':'))
|
||||
|| (app_server.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Embedded
|
||||
&& self
|
||||
.config
|
||||
.custom_permission_profiles
|
||||
.iter()
|
||||
.any(|profile| profile.id == selection.profile_id))
|
||||
{
|
||||
if self.apply_permission_profile_selection(selection).await {
|
||||
self.chat_widget.submit_initial_user_message_if_pending();
|
||||
}
|
||||
return;
|
||||
}
|
||||
let Some(thread_id) = self.chat_widget.thread_id() else {
|
||||
self.chat_widget
|
||||
.retain_input_after_failed_permission_selection();
|
||||
@@ -293,21 +282,16 @@ impl App {
|
||||
);
|
||||
return;
|
||||
};
|
||||
if !selection.profile_id.starts_with(':')
|
||||
&& self.chat_widget.is_user_turn_pending_or_running()
|
||||
{
|
||||
self.chat_widget
|
||||
.retain_input_after_failed_permission_selection();
|
||||
self.chat_widget.add_error_message(
|
||||
"Wait for the current turn to finish before changing permissions.".into(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
let config = self.chat_widget.config_ref();
|
||||
if config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.is_some_and(|profile| profile.id == selection.profile_id)
|
||||
if !self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
&& selection.profile_id.starts_with(':')
|
||||
&& config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.is_some_and(|profile| profile.id == selection.profile_id)
|
||||
&& selection
|
||||
.approval_policy
|
||||
.is_none_or(|policy| config.permissions.approval_policy.value() == policy.to_core())
|
||||
@@ -330,10 +314,25 @@ impl App {
|
||||
};
|
||||
match app_server.thread_settings_update(params).await {
|
||||
Ok(true) => {
|
||||
// Omission preserves the active value on update, but uses defaults on start.
|
||||
// Retain effective intent across consecutive requests before confirmation.
|
||||
let previous = self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id);
|
||||
selection.approval_policy = selection
|
||||
.approval_policy
|
||||
.or_else(|| previous.and_then(|profile| profile.approval_policy))
|
||||
.or(Some(config.permissions.approval_policy.value().into()));
|
||||
selection.approvals_reviewer = selection
|
||||
.approvals_reviewer
|
||||
.or_else(|| previous.and_then(|profile| profile.approvals_reviewer))
|
||||
.or(Some(config.approvals_reviewer));
|
||||
self.agents_overview
|
||||
.selected_permission_profiles
|
||||
.insert(thread_id, selection.profile_id.clone());
|
||||
self.pending_server_profiles
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.insert(thread_id, selection.clone());
|
||||
self.chat_widget.add_info_message(
|
||||
format!(
|
||||
@@ -386,18 +385,35 @@ impl App {
|
||||
true
|
||||
}
|
||||
|
||||
pub(super) fn confirmed_server_profile(
|
||||
pub(super) fn selected_server_profile(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> Option<PermissionProfileSelection> {
|
||||
if self.chat_widget.thread_id() != Some(thread_id) {
|
||||
return None;
|
||||
}
|
||||
if let Some(selection) = self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id)
|
||||
{
|
||||
return Some(selection.clone());
|
||||
}
|
||||
if self.app_server_target.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Remote
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let config = self.chat_widget.config_ref();
|
||||
let active = config.permissions.active_permission_profile()?;
|
||||
if active.id.starts_with(':')
|
||||
|| (self.app_server_target.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Embedded
|
||||
|| (self
|
||||
.agents_overview
|
||||
.selected_permission_profiles
|
||||
.get(&thread_id)
|
||||
!= Some(&active.id)
|
||||
&& self.app_server_target.thread_params_mode()
|
||||
== crate::app_server_session::ThreadParamsMode::Embedded
|
||||
&& self
|
||||
.config
|
||||
.custom_permission_profiles
|
||||
@@ -414,6 +430,44 @@ impl App {
|
||||
})
|
||||
}
|
||||
|
||||
/// Adopt authoritative widget settings before deriving overrides for subsequent turns.
|
||||
pub(super) fn adopt_server_permissions(&mut self) {
|
||||
self.chat_widget.set_permission_network(/*network*/ None);
|
||||
self.config.permissions = self.chat_widget.config_ref().permissions.clone();
|
||||
self.config.approvals_reviewer = self.chat_widget.config_ref().approvals_reviewer;
|
||||
self.runtime_approval_policy_override = Some(RuntimeApprovalPolicyOverride::Explicit(
|
||||
self.config.permissions.approval_policy.value().into(),
|
||||
));
|
||||
self.runtime_approvals_reviewer_override = if self
|
||||
.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.is_some_and(|profile| profile.id.starts_with(':'))
|
||||
{
|
||||
Some(self.config.approvals_reviewer)
|
||||
} else {
|
||||
self.runtime_approvals_reviewer_override
|
||||
.filter(|reviewer| *reviewer == self.config.approvals_reviewer)
|
||||
};
|
||||
self.runtime_permission_profile_override =
|
||||
Some(RuntimePermissionProfileOverride::from_config(&self.config));
|
||||
}
|
||||
|
||||
pub(super) fn adopt_inherited_server_selection(&mut self) {
|
||||
if let Some(thread_id) = self.chat_widget.thread_id()
|
||||
&& let Some(active) = self
|
||||
.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
{
|
||||
self.agents_overview
|
||||
.selected_permission_profiles
|
||||
.insert(thread_id, active.id);
|
||||
}
|
||||
self.adopt_server_permissions();
|
||||
}
|
||||
|
||||
pub(super) async fn refresh_in_memory_config_from_disk(&mut self) -> Result<()> {
|
||||
let mut config = self
|
||||
.rebuild_config_for_cwd(self.chat_widget.config_ref().cwd.to_path_buf())
|
||||
|
||||
@@ -533,7 +533,7 @@ impl App {
|
||||
fork_config.model = Some(self.chat_widget.current_model().to_string());
|
||||
fork_config.model_reasoning_effort =
|
||||
self.chat_widget.current_reasoning_effort();
|
||||
let selected_profile = self.confirmed_server_profile(thread_id);
|
||||
let selected_profile = self.selected_server_profile(thread_id);
|
||||
match app_server.fork_thread_at(
|
||||
&self.local_settings,
|
||||
fork_config,
|
||||
@@ -574,6 +574,9 @@ impl App {
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
if selected_profile.is_some() {
|
||||
self.adopt_inherited_server_selection();
|
||||
}
|
||||
// Keep local input without replacing the fork's running state.
|
||||
self.chat_widget.restore_reconnected_input(retained_input, &[]);
|
||||
if let Some(err) = name_error {
|
||||
|
||||
@@ -327,8 +327,13 @@ impl App {
|
||||
self.pending_app_server_requests.clear();
|
||||
// The displayed task was resumed above. Keep offscreen selections pending until those
|
||||
// tasks can be resumed from the server too; their old confirmations cannot arrive.
|
||||
let pending_displayed_profile =
|
||||
displayed.and_then(|id| self.pending_server_profiles.remove(&id));
|
||||
let pending_displayed_profile = displayed.and_then(|id| {
|
||||
self.pending_server_profiles.remove(&id).or_else(|| {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&id)
|
||||
})
|
||||
});
|
||||
if let Some(selection) = &pending_displayed_profile {
|
||||
self.runtime_approval_policy_override = None;
|
||||
if selection.approvals_reviewer.is_some() {
|
||||
|
||||
@@ -60,6 +60,8 @@ impl App {
|
||||
items,
|
||||
cwd,
|
||||
active_permission_profile,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
model: turn_model,
|
||||
effort,
|
||||
collaboration_mode,
|
||||
@@ -180,7 +182,7 @@ impl App {
|
||||
let retry_display = ChatWidget::user_message_display_from_inputs(items);
|
||||
|
||||
self.config = retry_config.clone();
|
||||
let selected_profile = self.confirmed_server_profile(thread_id);
|
||||
let selected_profile = self.selected_server_profile(thread_id);
|
||||
let started = app_server
|
||||
.fork_thread_at(
|
||||
&self.local_settings,
|
||||
@@ -200,6 +202,9 @@ impl App {
|
||||
}
|
||||
};
|
||||
let retry_thread_id = started.session.thread_id;
|
||||
*approval_policy = started.session.approval_policy;
|
||||
*approvals_reviewer = Some(started.session.approvals_reviewer);
|
||||
*active_permission_profile = started.session.active_permission_profile.clone();
|
||||
|
||||
self.detach_current_thread_for_navigation(app_server, Some(retry_thread_id))
|
||||
.await;
|
||||
@@ -216,6 +221,9 @@ impl App {
|
||||
return;
|
||||
}
|
||||
|
||||
if selected_profile.is_some() {
|
||||
self.adopt_inherited_server_selection();
|
||||
}
|
||||
let failure_input_state = input_state.clone();
|
||||
self.chat_widget.restore_thread_input_state(
|
||||
input_state,
|
||||
|
||||
@@ -757,6 +757,20 @@ impl App {
|
||||
self.app_event_tx
|
||||
.send(AppEvent::ResetTranscriptForThreadSwitch);
|
||||
self.replay_thread_snapshot(snapshot, resume_restored_queue);
|
||||
if let Some(thread_id) = self.chat_widget.thread_id()
|
||||
&& let Some(active) = self
|
||||
.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
&& self
|
||||
.agents_overview
|
||||
.selected_permission_profiles
|
||||
.get(&thread_id)
|
||||
== Some(&active.id)
|
||||
{
|
||||
self.adopt_server_permissions();
|
||||
}
|
||||
if external_writer {
|
||||
self.chat_widget.show_external_writer_thread();
|
||||
}
|
||||
|
||||
@@ -722,8 +722,14 @@ impl App {
|
||||
.await;
|
||||
|
||||
let fork_config = self.side_fork_config();
|
||||
let selected_profile = self.selected_server_profile(parent_thread_id);
|
||||
match app_server
|
||||
.fork_side_thread(&self.local_settings, fork_config, parent_thread_id)
|
||||
.fork_side_thread(
|
||||
&self.local_settings,
|
||||
fork_config,
|
||||
parent_thread_id,
|
||||
selected_profile.as_ref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(forked) => {
|
||||
@@ -779,6 +785,9 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
if self.active_thread_id == Some(child_thread_id) {
|
||||
if selected_profile.is_some() {
|
||||
self.adopt_inherited_server_selection();
|
||||
}
|
||||
if let Some(user_message) = user_message.take() {
|
||||
let _ = self
|
||||
.chat_widget
|
||||
|
||||
@@ -2862,6 +2862,20 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<()
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
|
||||
app.runtime_permission_profile_override = Some(RuntimePermissionProfileOverride::from_config(
|
||||
app.chat_widget.config_ref(),
|
||||
));
|
||||
app.select_permission_profile(
|
||||
&mut app_server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: ":read-only".into(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
display_label: "Read Only".into(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let control = Box::pin(app.handle_start_side(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
@@ -2903,6 +2917,53 @@ async fn handle_start_side_seeds_navigation_before_thread_started() -> Result<()
|
||||
}
|
||||
|
||||
assert!(saw_thread_started);
|
||||
assert_eq!(
|
||||
app.config.permissions.permission_profile(),
|
||||
&PermissionProfile::read_only()
|
||||
);
|
||||
assert_eq!(
|
||||
app.runtime_permission_profile_override,
|
||||
Some(RuntimePermissionProfileOverride::from_config(
|
||||
app.chat_widget.config_ref()
|
||||
))
|
||||
);
|
||||
app.select_agent_thread(&mut tui, &mut app_server, parent_thread_id)
|
||||
.await?;
|
||||
app.select_permission_profile(
|
||||
&mut app_server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: ":workspace".into(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
display_label: "Workspace".into(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
app.select_agent_thread(&mut tui, &mut app_server, side_thread_id)
|
||||
.await?;
|
||||
let settings = next_thread_settings_updated(&mut app_server, parent_thread_id).await;
|
||||
app.enqueue_thread_notification(
|
||||
parent_thread_id,
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)
|
||||
.await?;
|
||||
app.select_agent_thread(&mut tui, &mut app_server, parent_thread_id)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
app.config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.unwrap()
|
||||
.id,
|
||||
":workspace"
|
||||
);
|
||||
assert_eq!(
|
||||
app.runtime_permission_profile_override,
|
||||
Some(RuntimePermissionProfileOverride::from_config(
|
||||
app.chat_widget.config_ref()
|
||||
))
|
||||
);
|
||||
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -3112,23 +3173,6 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() ->
|
||||
@"• Permission selection requested: server-only"
|
||||
);
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.select_permission_profile(&mut server, selection.clone())
|
||||
.await;
|
||||
insta::assert_snapshot!(
|
||||
next_history_message(&mut events),
|
||||
@"■ Wait for permissions to update before changing permissions."
|
||||
);
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
AppEvent::ForkCurrentSession { name: None },
|
||||
)
|
||||
.await?;
|
||||
let _ = next_history_message(&mut events);
|
||||
insta::assert_snapshot!(
|
||||
next_history_message(&mut events),
|
||||
@"■ Wait for permissions to update before forking."
|
||||
);
|
||||
app.chat_widget
|
||||
.restore_user_message_to_composer("use the selected profile".into());
|
||||
app.chat_widget
|
||||
@@ -3171,6 +3215,7 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() ->
|
||||
);
|
||||
let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:8765")?;
|
||||
app.app_server_target = crate::AppServerTarget::Remote { endpoint };
|
||||
assert!(app.selected_server_profile(thread_id).is_none());
|
||||
while events.try_recv().is_ok() {}
|
||||
app.change_working_directory(&mut tui, &mut server, home.path().abs())
|
||||
.await;
|
||||
@@ -3206,20 +3251,24 @@ async fn server_only_profile_selection_keeps_turns_on_the_selected_profile() ->
|
||||
.restore_user_message_to_composer("queued follow-up".into());
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
let selection = app
|
||||
.confirmed_server_profile(thread_id)
|
||||
.expect("server profile");
|
||||
let selection = PermissionProfileSelection {
|
||||
profile_id: "server-only".into(),
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
display_label: "server-only".into(),
|
||||
};
|
||||
app.select_permission_profile(&mut server, selection).await;
|
||||
insta::assert_snapshot!(
|
||||
next_history_message(&mut events),
|
||||
@"■ Wait for the current turn to finish before changing permissions."
|
||||
@"• Permission selection requested: server-only"
|
||||
);
|
||||
app.handle_event(&mut tui, &mut server, AppEvent::SettingsSelectionSettled)
|
||||
.await?;
|
||||
app.handle_thread_event_now(ThreadBufferedEvent::Notification(Box::new(
|
||||
turn_completed_notification(thread_id, "second", TurnStatus::Completed),
|
||||
)));
|
||||
assert!(app.chat_widget.has_queued_follow_up_messages());
|
||||
assert!(!app.chat_widget.has_queued_follow_up_messages());
|
||||
let _ = next_user_turn_op(&mut ops);
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -9232,7 +9281,10 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() {
|
||||
.await
|
||||
.expect("pending permission selection should block the cyber model");
|
||||
assert_eq!(app.chat_widget.current_model(), previous_model);
|
||||
app.pending_server_profiles.remove(&thread_id);
|
||||
let previous_selection = app.pending_server_profiles.remove(&thread_id).unwrap();
|
||||
app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.insert(thread_id, previous_selection);
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
@@ -9242,6 +9294,11 @@ async fn selecting_cyber_model_defaults_active_thread_to_auto_review() {
|
||||
.expect("model selection should succeed");
|
||||
|
||||
assert!(app.pending_server_profiles.contains_key(&thread_id));
|
||||
assert!(
|
||||
!app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
);
|
||||
let notification = next_thread_settings_updated(&mut app_server, thread_id).await;
|
||||
assert_eq!(
|
||||
notification.thread_settings.approval_policy,
|
||||
|
||||
@@ -206,7 +206,7 @@ async fn reconnect_restores_history_permissions_and_resumes_unsent_input() -> Re
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
if pending_profile {
|
||||
app.runtime_approvals_reviewer_override = Some(ApprovalsReviewer::User);
|
||||
app.pending_server_profiles.insert(
|
||||
app.agents_overview.requested_permission_profiles.insert(
|
||||
id,
|
||||
PermissionProfileSelection {
|
||||
profile_id: "server-only".into(),
|
||||
@@ -303,6 +303,7 @@ async fn reconnect_restores_history_permissions_and_resumes_unsent_input() -> Re
|
||||
);
|
||||
}
|
||||
assert!(app.pending_server_profiles.is_empty());
|
||||
assert!(app.agents_overview.requested_permission_profiles.is_empty());
|
||||
if pending_profile {
|
||||
assert_eq!(
|
||||
app.resume_permission_overrides(&app.config),
|
||||
|
||||
@@ -23,7 +23,12 @@ async fn confirm_permission_selection(
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)
|
||||
.await?;
|
||||
if !app.pending_server_profiles.contains_key(&thread_id) {
|
||||
if !app.pending_server_profiles.contains_key(&thread_id)
|
||||
&& !app
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
@@ -659,7 +664,7 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result<
|
||||
let home = tempdir()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n",
|
||||
"approval_policy = \"never\"\napprovals_reviewer = \"auto_review\"\ndefault_permissions = \":workspace\"\n[permissions.server-only]\nextends = \":read-only\"\n",
|
||||
)?;
|
||||
let server_config = ConfigBuilder::default()
|
||||
.codex_home(home.path().into())
|
||||
@@ -729,16 +734,53 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result<
|
||||
"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}]
|
||||
}))?]).await?;
|
||||
}
|
||||
app.select_permission_profile(&mut server, selection.clone())
|
||||
.await;
|
||||
// Omitted custom-profile fields must preserve the preceding accepted choice,
|
||||
// even before either settings notification reaches the TUI.
|
||||
app.select_permission_profile(
|
||||
&mut server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: ":read-only".into(),
|
||||
..selection.clone()
|
||||
},
|
||||
)
|
||||
.await;
|
||||
// Drain without applying it: confirmation is still pending in the TUI, but
|
||||
// the server cannot coalesce both updates into one latest snapshot.
|
||||
let _ = next_thread_settings_updated(&mut server, thread_id).await;
|
||||
app.select_permission_profile(
|
||||
&mut server,
|
||||
PermissionProfileSelection {
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
..selection.clone()
|
||||
},
|
||||
)
|
||||
.await;
|
||||
let view = app.agents_overview_view(Vec::new(), /*selected_thread_id*/ None);
|
||||
app.chat_widget.show_bottom_pane_view(Box::new(view));
|
||||
app.new_agents_overview_session(&mut tui, &mut server, /*cwd*/ None)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(thread_id));
|
||||
insta::assert_snapshot!(
|
||||
"command_center_pending_server_permissions",
|
||||
crate::chatwidget::tests::helpers::render_bottom_popup(&app.chat_widget, /*width*/ 80)
|
||||
assert_ne!(app.chat_widget.thread_id(), Some(thread_id));
|
||||
assert_eq!(
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.active_permission_profile(),
|
||||
Some(ActivePermissionProfile {
|
||||
id: "server-only".into(),
|
||||
extends: Some(":read-only".into())
|
||||
}),
|
||||
);
|
||||
assert_eq!(
|
||||
(
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.approval_policy
|
||||
.value(),
|
||||
app.chat_widget.config_ref().approvals_reviewer,
|
||||
),
|
||||
(AskForApproval::OnRequest.to_core(), ApprovalsReviewer::User)
|
||||
);
|
||||
let settings = next_thread_settings_updated(&mut server, thread_id).await;
|
||||
app.enqueue_thread_notification(
|
||||
@@ -786,6 +828,7 @@ async fn command_center_new_preserves_only_selected_server_profiles() -> Result<
|
||||
.remove(&app.chat_widget.thread_id().unwrap());
|
||||
app.select_permission_profile(&mut server, selection.clone())
|
||||
.await;
|
||||
assert!(!app.reject_pending_permission_change());
|
||||
}
|
||||
}
|
||||
server.shutdown().await?;
|
||||
@@ -1133,6 +1176,17 @@ async fn command_center_new_checkout_and_worktree_preserve_source_and_default_br
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true);
|
||||
Box::pin(app.handle_event(&mut tui, &mut server, AppEvent::OpenPermissionsPopup)).await?;
|
||||
tokio::time::timeout(Duration::from_secs(3), async {
|
||||
while let Some(event) = events.recv().await {
|
||||
let loaded = matches!(&event, AppEvent::PermissionProfilesLoaded { .. });
|
||||
Box::pin(app.handle_event(&mut tui, &mut server, event)).await?;
|
||||
if loaded {
|
||||
return Ok::<_, color_eyre::Report>(());
|
||||
}
|
||||
}
|
||||
color_eyre::eyre::bail!("permissions menu event stream closed")
|
||||
})
|
||||
.await??;
|
||||
app.chat_widget.handle_key_event(KeyCode::Up.into());
|
||||
app.chat_widget.handle_key_event(KeyCode::Enter.into());
|
||||
while let Ok(event) = events.try_recv() {
|
||||
|
||||
@@ -11,7 +11,6 @@ async fn builtin_permission_selection_adopts_server_settings() -> Result<()> {
|
||||
let thread_id = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await?;
|
||||
let other = server.start_thread(&app.config).await?.session.thread_id;
|
||||
// A previously selected custom profile may have left a profile-specific proxy cached.
|
||||
let home = tempdir()?;
|
||||
std::fs::write(
|
||||
@@ -65,17 +64,12 @@ proxy_url = "http://127.0.0.1:43128"
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
before,
|
||||
);
|
||||
assert!(app.pending_server_profiles.contains_key(&thread_id));
|
||||
assert!(
|
||||
app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
);
|
||||
insta::assert_snapshot!(next_history_message(&mut events).replace(profile_id, "<PROFILE>"), @"• Permission selection requested: <PROFILE>");
|
||||
while events.try_recv().is_ok() {}
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.handle_event(&mut tui, &mut server, AppEvent::NewSession { name: None })
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(thread_id));
|
||||
insta::assert_snapshot!(next_history_message(&mut events), @"■ Wait for permissions to update before switching tasks.");
|
||||
app.select_agents_overview_thread(&mut tui, &mut server, other)
|
||||
.await?;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(thread_id));
|
||||
let settings = next_thread_settings_updated(&mut server, thread_id).await;
|
||||
app.enqueue_thread_notification(
|
||||
thread_id,
|
||||
@@ -139,3 +133,251 @@ async fn matching_builtin_permission_selection_submits_pending_prompt() -> Resul
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn custom_permission_selection_uses_server_definition_and_preserves_state_on_rejection()
|
||||
-> Result<()> {
|
||||
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
|
||||
app.config.ephemeral = false;
|
||||
let home = tempdir()?;
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":workspace\"\n",
|
||||
)?;
|
||||
let server_config = ConfigBuilder::default()
|
||||
.codex_home(home.path().into())
|
||||
.build()
|
||||
.await?;
|
||||
let mut server = crate::start_embedded_app_server_for_picker(&server_config).await?;
|
||||
let started = server.start_thread(&app.config).await?;
|
||||
let thread_id = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await?;
|
||||
let client_home = tempdir()?;
|
||||
std::fs::write(
|
||||
client_home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":danger-full-access\"\n",
|
||||
)?;
|
||||
let client_config = ConfigBuilder::default()
|
||||
.codex_home(client_home.path().into())
|
||||
.build()
|
||||
.await?;
|
||||
app.config.codex_home = client_config.codex_home;
|
||||
app.config.custom_permission_profiles = client_config.custom_permission_profiles;
|
||||
let selection = PermissionProfileSelection {
|
||||
profile_id: "shared".into(),
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
approvals_reviewer: None,
|
||||
display_label: "shared".into(),
|
||||
};
|
||||
app.chat_widget.handle_server_notification(
|
||||
turn_started_notification(thread_id, "turn-1"),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
assert!(app.chat_widget.is_user_turn_pending_or_running());
|
||||
let before = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref());
|
||||
app.select_permission_profile(&mut server, selection.clone())
|
||||
.await;
|
||||
assert_eq!(
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
before
|
||||
);
|
||||
let settings = next_thread_settings_updated(&mut server, thread_id).await;
|
||||
let expected = PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&settings.thread_settings.sandbox_policy.to_core(),
|
||||
settings.thread_settings.cwd.as_path(),
|
||||
);
|
||||
assert!(matches!(
|
||||
settings.thread_settings.sandbox_policy,
|
||||
codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { .. }
|
||||
));
|
||||
app.enqueue_thread_notification(
|
||||
thread_id,
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
(
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.permission_profile(),
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.approval_policy
|
||||
.value()
|
||||
),
|
||||
(&expected, AskForApproval::Never.to_core())
|
||||
);
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
"default_permissions = \":workspace\"\n[permissions.shared]\nextends = \":read-only\"\n",
|
||||
)?;
|
||||
app.select_permission_profile(
|
||||
&mut server,
|
||||
PermissionProfileSelection {
|
||||
approval_policy: None,
|
||||
..selection.clone()
|
||||
},
|
||||
)
|
||||
.await;
|
||||
let settings = next_thread_settings_updated(&mut server, thread_id).await;
|
||||
assert!(matches!(
|
||||
settings.thread_settings.sandbox_policy,
|
||||
codex_app_server_protocol::SandboxPolicy::ReadOnly { .. }
|
||||
));
|
||||
app.enqueue_thread_notification(
|
||||
thread_id,
|
||||
ServerNotification::ThreadSettingsUpdated(settings),
|
||||
)
|
||||
.await?;
|
||||
app.chat_widget.handle_server_notification(
|
||||
turn_completed_notification(thread_id, "turn-1", TurnStatus::Completed),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
// Unchanged re-selection is accepted without requiring a notification.
|
||||
app.select_permission_profile(&mut server, selection.clone())
|
||||
.await;
|
||||
assert!(!app.reject_pending_permission_change());
|
||||
let confirmed = RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref());
|
||||
let confirmed_config = app.config.clone();
|
||||
app.select_permission_profile(
|
||||
&mut server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: "missing-on-server".into(),
|
||||
..selection
|
||||
},
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
confirmed
|
||||
);
|
||||
assert!(!app.pending_server_profiles.contains_key(&thread_id));
|
||||
assert_eq!(app.config, confirmed_config);
|
||||
assert_eq!(
|
||||
app.agents_overview
|
||||
.selected_permission_profiles
|
||||
.get(&thread_id)
|
||||
.map(String::as_str),
|
||||
Some("shared")
|
||||
);
|
||||
// Materialize the task so /fork exercises persisted history with the confirmed profile.
|
||||
server.thread_inject_items(thread_id, vec![serde_json::from_value(serde_json::json!({
|
||||
"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}]
|
||||
}))?]).await?;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
while events.try_recv().is_ok() {}
|
||||
app.change_working_directory(&mut tui, &mut server, client_home.path().abs())
|
||||
.await;
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(thread_id));
|
||||
insta::assert_snapshot!(next_history_message(&mut events), @"■ Changing directories with an unconfirmed named profile is not supported.");
|
||||
let side = server
|
||||
.fork_side_thread(
|
||||
&app.local_settings,
|
||||
app.config.clone(),
|
||||
thread_id,
|
||||
app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id),
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
(
|
||||
side.session.permission_profile,
|
||||
side.session.approval_policy
|
||||
),
|
||||
(PermissionProfile::read_only(), AskForApproval::Never)
|
||||
);
|
||||
app.cli_kv_overrides.push((
|
||||
"permissions.shared.extends".into(),
|
||||
TomlValue::String(":danger-full-access".into()),
|
||||
));
|
||||
// Both the accepted request and its confirmed descendant remain server-owned.
|
||||
for _ in 0..2 {
|
||||
let previous_thread_id = app.chat_widget.thread_id();
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
AppEvent::ForkCurrentSession { name: None },
|
||||
)
|
||||
.await?;
|
||||
assert_ne!(app.chat_widget.thread_id(), previous_thread_id);
|
||||
assert_eq!(
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
confirmed
|
||||
);
|
||||
}
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn accepted_permissions_survive_directory_change_before_notification() -> Result<()> {
|
||||
for preserve_history in [false, true] {
|
||||
let (mut app, _events, _ops) = make_test_app_with_channels().await;
|
||||
app.config.ephemeral = false;
|
||||
app.config
|
||||
.permissions
|
||||
.set_permission_profile(PermissionProfile::Disabled)?;
|
||||
let destination = app.config.codex_home.join("destination");
|
||||
std::fs::create_dir(&destination)?;
|
||||
crate::legacy_core::config::set_project_trust_level(
|
||||
app.config.codex_home.as_path(),
|
||||
&destination,
|
||||
codex_protocol::config_types::TrustLevel::Trusted,
|
||||
)
|
||||
.map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?;
|
||||
let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
let started = server.start_thread(&app.config).await?;
|
||||
let source = started.session.thread_id;
|
||||
app.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await?;
|
||||
app.runtime_permission_profile_override =
|
||||
Some(RuntimePermissionProfileOverride::from_config(&app.config));
|
||||
if preserve_history {
|
||||
server.thread_inject_items(source, vec![serde_json::from_value(serde_json::json!({
|
||||
"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "saved history"}]
|
||||
}))?]).await?;
|
||||
}
|
||||
app.select_permission_profile(
|
||||
&mut server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: ":read-only".into(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
display_label: "Read Only".into(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&source),
|
||||
"selection must be unconfirmed"
|
||||
);
|
||||
// Destination constraints belong to the client; the server's result is authoritative.
|
||||
let requirements = app.config.codex_home.join("client-requirements.toml");
|
||||
std::fs::write(&requirements, "allowed_approval_policies = [\"never\"]")?;
|
||||
app.loader_overrides.system_requirements_path = Some(requirements.to_path_buf());
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.change_working_directory(&mut tui, &mut server, destination.clone())
|
||||
.await;
|
||||
assert_ne!(app.chat_widget.thread_id(), Some(source));
|
||||
assert_eq!(
|
||||
(&app.config.cwd, app.config.permissions.permission_profile()),
|
||||
(&destination, &PermissionProfile::read_only())
|
||||
);
|
||||
assert_eq!(
|
||||
app.config.permissions.approval_policy.value(),
|
||||
AskForApproval::OnRequest.to_core()
|
||||
);
|
||||
assert_eq!(
|
||||
app.runtime_permission_profile_override,
|
||||
Some(RuntimePermissionProfileOverride::from_config(&app.config))
|
||||
);
|
||||
server.shutdown().await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -105,7 +105,11 @@ async fn permission_shortcut_confirms_without_persisting() -> Result<()> {
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
before
|
||||
);
|
||||
assert!(app.pending_server_profiles.contains_key(&thread_id));
|
||||
assert!(
|
||||
app.agents_overview
|
||||
.requested_permission_profiles
|
||||
.contains_key(&thread_id)
|
||||
);
|
||||
insta::assert_snapshot!(
|
||||
next_history_message(&mut events),
|
||||
@"• Permission selection requested: Read Only"
|
||||
|
||||
@@ -40,6 +40,7 @@ const SAFETY_RETRY_THREAD_NAME: &str = "Safety retry source";
|
||||
enum SafetyRetryScenario {
|
||||
Once,
|
||||
RetryTwice,
|
||||
PendingPermissions,
|
||||
InterruptedPrevious,
|
||||
UnsupportedPermissions,
|
||||
}
|
||||
@@ -569,6 +570,16 @@ goals = true
|
||||
.enable(Feature::Goals)
|
||||
.expect("test config should allow goals");
|
||||
|
||||
if scenario == SafetyRetryScenario::PendingPermissions {
|
||||
app.config
|
||||
.permissions
|
||||
.set_permission_profile_from_session_snapshot(
|
||||
PermissionProfileSnapshot::from_session_snapshot(
|
||||
PermissionProfile::Disabled,
|
||||
Some(ActivePermissionProfile::new(":danger-full-access")),
|
||||
),
|
||||
)?;
|
||||
}
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?;
|
||||
let mut started = app_server.start_thread(&app.config).await?;
|
||||
@@ -783,6 +794,22 @@ goals = true
|
||||
}
|
||||
}
|
||||
|
||||
if scenario == SafetyRetryScenario::PendingPermissions {
|
||||
app.runtime_permission_profile_override = Some(
|
||||
RuntimePermissionProfileOverride::from_config(app.chat_widget.config_ref()),
|
||||
);
|
||||
app.select_permission_profile(
|
||||
&mut app_server,
|
||||
PermissionProfileSelection {
|
||||
profile_id: ":read-only".into(),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::User),
|
||||
display_label: "Read Only".into(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
Box::pin(app.retry_safety_buffered_turn(
|
||||
&mut tui,
|
||||
&mut app_server,
|
||||
@@ -923,6 +950,15 @@ goals = true
|
||||
let retry_thread_id = app.chat_widget.thread_id().expect("retry thread id");
|
||||
// Capture the completed retry before processing its automatic goal continuation.
|
||||
wait_for_turn_completed(&mut app, &mut app_server, retry_thread_id).await;
|
||||
if scenario == SafetyRetryScenario::PendingPermissions {
|
||||
assert_eq!(
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.permission_profile(),
|
||||
&PermissionProfile::read_only()
|
||||
);
|
||||
}
|
||||
let mut replayed_history = String::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
@@ -1165,3 +1201,14 @@ async fn safety_retry_rejects_unsupported_permissions_before_interrupting() -> R
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn safety_retry_preserves_unconfirmed_permissions_on_its_first_turn() -> Result<()> {
|
||||
run_safety_retry(
|
||||
/*previous_prompt*/ None,
|
||||
/*failing_draft*/ None,
|
||||
/*committed_steer*/ None,
|
||||
SafetyRetryScenario::PendingPermissions,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -725,7 +725,12 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> {
|
||||
let mut side_config = app.config.clone();
|
||||
side_config.ephemeral = true;
|
||||
let side = server
|
||||
.fork_side_thread(&app.local_settings, side_config.clone(), thread_id)
|
||||
.fork_side_thread(
|
||||
&app.local_settings,
|
||||
side_config.clone(),
|
||||
thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
let side_id = side.session.thread_id;
|
||||
app.side_threads
|
||||
@@ -735,7 +740,12 @@ async fn delete_current_thread_navigates_only_after_success() -> Result<()> {
|
||||
side_config.cwd = side_config.cwd.join("failure");
|
||||
assert!(
|
||||
server
|
||||
.fork_side_thread(&app.local_settings, side_config, thread_id)
|
||||
.fork_side_thread(
|
||||
&app.local_settings,
|
||||
side_config,
|
||||
thread_id,
|
||||
/*selected_profile*/ None
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
@@ -1013,7 +1023,12 @@ async fn archive_current_thread_returns_shared_servers_to_agents() -> Result<()>
|
||||
let mut side_config = app.config.clone();
|
||||
side_config.ephemeral = true;
|
||||
let side = server
|
||||
.fork_side_thread(&app.local_settings, side_config, thread_id)
|
||||
.fork_side_thread(
|
||||
&app.local_settings,
|
||||
side_config,
|
||||
thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
let side_id = side.session.thread_id;
|
||||
app.side_threads
|
||||
@@ -2822,6 +2837,7 @@ async fn paginated_workflows_never_request_full_thread_history() -> Result<()> {
|
||||
&crate::local_settings::LocalSettings::from(&side_config),
|
||||
side_config,
|
||||
paginated_thread_id,
|
||||
/*selected_profile*/ None,
|
||||
))
|
||||
.await?;
|
||||
|
||||
|
||||
-11
@@ -1,11 +0,0 @@
|
||||
---
|
||||
source: tui/src/app/tests/background_task_defaults_tests.rs
|
||||
expression: "crate::chatwidget::tests::helpers::render_bottom_popup(&app.chat_widget, 80)"
|
||||
---
|
||||
Unable to complete action
|
||||
Wait for permissions to update before starting a session.
|
||||
|
||||
|
||||
› 1. Return to command center
|
||||
|
||||
enter select · esc back
|
||||
@@ -827,7 +827,12 @@ impl App {
|
||||
}
|
||||
if should_start_turn {
|
||||
let config = self.chat_widget.config_ref();
|
||||
let selected_profile = self.pending_server_profiles.get(&thread_id);
|
||||
let selected_profile =
|
||||
self.pending_server_profiles.get(&thread_id).or_else(|| {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id)
|
||||
});
|
||||
let selected_active = selected_profile
|
||||
.map(|profile| ActivePermissionProfile::new(profile.profile_id.clone()));
|
||||
let (turn_approval_policy, turn_approvals_reviewer) =
|
||||
@@ -1196,6 +1201,11 @@ impl App {
|
||||
if self
|
||||
.pending_server_profiles
|
||||
.get(&thread_id)
|
||||
.or_else(|| {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id)
|
||||
})
|
||||
.is_some_and(|selected| {
|
||||
notification
|
||||
.thread_settings
|
||||
@@ -1210,7 +1220,11 @@ impl App {
|
||||
})
|
||||
})
|
||||
{
|
||||
confirmed_profile = self.pending_server_profiles.remove(&thread_id);
|
||||
confirmed_profile = self.pending_server_profiles.remove(&thread_id).or_else(|| {
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&thread_id)
|
||||
});
|
||||
}
|
||||
}
|
||||
let inferred_session = if let ServerNotification::ThreadStarted(started) = ¬ification
|
||||
@@ -1298,41 +1312,9 @@ impl App {
|
||||
.on_thread_settings_updated(settings.clone());
|
||||
notification = None;
|
||||
}
|
||||
if let Some(selected) = confirmed_profile {
|
||||
if confirmed_profile.is_some() {
|
||||
if self.chat_widget.thread_id() == Some(thread_id) {
|
||||
if selected.approvals_reviewer.is_some() {
|
||||
self.runtime_approvals_reviewer_override = None;
|
||||
}
|
||||
self.runtime_permission_profile_override = None;
|
||||
}
|
||||
if self.chat_widget.thread_id() == Some(thread_id)
|
||||
&& let Some(profile) = self
|
||||
.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
&& profile.id.starts_with(':')
|
||||
{
|
||||
let config = self.chat_widget.config_ref();
|
||||
let network = config
|
||||
.network_proxy_spec_for_active_permission_profile(
|
||||
&profile,
|
||||
config.permissions.permission_profile(),
|
||||
)
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(%err, "failed to refresh local permission network settings");
|
||||
None
|
||||
});
|
||||
self.chat_widget.set_permission_network(network);
|
||||
self.config.permissions = self.chat_widget.config_ref().permissions.clone();
|
||||
self.config.approvals_reviewer = self.chat_widget.config_ref().approvals_reviewer;
|
||||
self.runtime_approval_policy_override =
|
||||
Some(RuntimeApprovalPolicyOverride::Explicit(
|
||||
self.config.permissions.approval_policy.value().into(),
|
||||
));
|
||||
self.runtime_approvals_reviewer_override = Some(self.config.approvals_reviewer);
|
||||
self.runtime_permission_profile_override =
|
||||
Some(RuntimePermissionProfileOverride::from_config(&self.config));
|
||||
self.adopt_server_permissions();
|
||||
}
|
||||
self.app_event_tx.send(AppEvent::SettingsSelectionSettled);
|
||||
}
|
||||
@@ -1574,6 +1556,9 @@ impl App {
|
||||
|
||||
let thread_id = session.thread_id;
|
||||
self.pending_server_profiles.remove(&thread_id);
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&thread_id);
|
||||
if self.primary_thread_id != Some(thread_id) {
|
||||
self.recap.reset_for_new_thread(Instant::now());
|
||||
}
|
||||
|
||||
@@ -218,19 +218,29 @@ impl App {
|
||||
if !thread_settings_update_has_changes(¶ms) {
|
||||
return false;
|
||||
}
|
||||
if ThreadId::from_string(¶ms.thread_id)
|
||||
.ok()
|
||||
.is_some_and(|thread_id| self.pending_server_profiles.contains_key(&thread_id))
|
||||
&& (params.cwd.is_some()
|
||||
|| params.approval_policy.is_some()
|
||||
|| params.approvals_reviewer.is_some()
|
||||
|| params.sandbox_policy.is_some()
|
||||
|| params.permissions.is_some())
|
||||
let thread_id = ThreadId::from_string(¶ms.thread_id).ok();
|
||||
let changes_permissions = params.cwd.is_some()
|
||||
|| params.approval_policy.is_some()
|
||||
|| params.approvals_reviewer.is_some()
|
||||
|| params.sandbox_policy.is_some()
|
||||
|| params.permissions.is_some();
|
||||
if changes_permissions
|
||||
&& thread_id.is_some_and(|id| self.pending_server_profiles.contains_key(&id))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
match app_server.thread_settings_update(params).await {
|
||||
Ok(settings_updated) => settings_updated,
|
||||
Ok(settings_updated) => {
|
||||
if settings_updated
|
||||
&& changes_permissions
|
||||
&& let Some(thread_id) = thread_id
|
||||
{
|
||||
self.agents_overview
|
||||
.requested_permission_profiles
|
||||
.remove(&thread_id);
|
||||
}
|
||||
settings_updated
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!("failed to update app-server thread settings from TUI: {err}");
|
||||
self.chat_widget
|
||||
|
||||
@@ -22,6 +22,7 @@ pub(super) struct ManagedWorktreeAttach {
|
||||
keymap: RuntimeKeymap,
|
||||
cwd: AbsolutePathBuf,
|
||||
name_error: Option<String>,
|
||||
selected_profile: Option<PermissionProfileSelection>,
|
||||
}
|
||||
|
||||
/// A /cd request awaiting a fresh event-loop iteration.
|
||||
@@ -177,6 +178,19 @@ impl App {
|
||||
"Changing directories with a named profile is not supported.",
|
||||
);
|
||||
}
|
||||
let selected_profile = self
|
||||
.agents_overview
|
||||
.requested_permission_profiles
|
||||
.get(&thread_id)
|
||||
.cloned();
|
||||
if selected_profile
|
||||
.as_ref()
|
||||
.is_some_and(|profile| !profile.profile_id.starts_with(':'))
|
||||
{
|
||||
return self.working_directory_error(
|
||||
"Changing directories with an unconfirmed named profile is not supported.",
|
||||
);
|
||||
}
|
||||
let cells = &self.transcript_cells;
|
||||
if cells.iter().any(|cell| cell.as_any().is::<LoadingCell>()) {
|
||||
return self.working_directory_error("MCP inventory is still loading.");
|
||||
@@ -330,6 +344,7 @@ impl App {
|
||||
let profile = self
|
||||
.runtime_permission_profile_override
|
||||
.as_ref()
|
||||
.filter(|_| selected_profile.is_none())
|
||||
.filter(|profile| {
|
||||
scope == RuntimePolicyOverrideScope::All
|
||||
|| profile.turn_override == RuntimePermissionProfileTurnOverride::LegacySandbox
|
||||
@@ -356,12 +371,15 @@ impl App {
|
||||
{
|
||||
return self.working_directory_error("Permission profile cannot be preserved by /cd.");
|
||||
}
|
||||
if let Err(error) = self.apply_runtime_policy_overrides(&mut config, scope) {
|
||||
if selected_profile.is_none()
|
||||
&& let Err(error) = self.apply_runtime_policy_overrides(&mut config, scope)
|
||||
{
|
||||
return self.working_directory_error(format!("{error:#}"));
|
||||
}
|
||||
let actual = config.permissions.approval_policy.value();
|
||||
let approval = self
|
||||
.runtime_approval_policy_override
|
||||
.filter(|_| selected_profile.is_none())
|
||||
.filter(|value| {
|
||||
scope == RuntimePolicyOverrideScope::All
|
||||
|| matches!(value, RuntimeApprovalPolicyOverride::Explicit(_))
|
||||
@@ -436,7 +454,7 @@ impl App {
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ None,
|
||||
DeferUntilNextTurn,
|
||||
/*selected_profile*/ None,
|
||||
selected_profile.as_ref(),
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
@@ -446,7 +464,7 @@ impl App {
|
||||
&config,
|
||||
/*session_start_source*/ None,
|
||||
/*remote_cwd_override*/ None,
|
||||
/*selected_profile*/ None,
|
||||
selected_profile.as_ref(),
|
||||
)
|
||||
.await
|
||||
};
|
||||
@@ -455,12 +473,27 @@ impl App {
|
||||
Err(e) => return self.working_directory_error(format!("Failed to change: {e}")),
|
||||
};
|
||||
let session = &transitioned.session;
|
||||
let permissions_match = if let Some(selected) = &selected_profile {
|
||||
session
|
||||
.active_permission_profile
|
||||
.as_ref()
|
||||
.is_some_and(|active| active.id == selected.profile_id)
|
||||
&& selected
|
||||
.approval_policy
|
||||
.is_none_or(|policy| session.approval_policy == policy)
|
||||
&& selected
|
||||
.approvals_reviewer
|
||||
.is_none_or(|reviewer| session.approvals_reviewer == reviewer)
|
||||
} else {
|
||||
session.approval_policy.to_core() == config.permissions.approval_policy.value()
|
||||
&& session.approvals_reviewer == config.approvals_reviewer
|
||||
&& session.active_permission_profile
|
||||
== config.permissions.active_permission_profile()
|
||||
};
|
||||
if session.thread_id == thread_id
|
||||
|| crate::session_resume::cwds_differ(session.cwd.as_path(), cwd.as_path())
|
||||
|| session.runtime_workspace_roots != config.workspace_roots
|
||||
|| session.approval_policy.to_core() != config.permissions.approval_policy.value()
|
||||
|| session.approvals_reviewer != config.approvals_reviewer
|
||||
|| session.active_permission_profile != config.permissions.active_permission_profile()
|
||||
|| !permissions_match
|
||||
{
|
||||
if session.thread_id != thread_id {
|
||||
let _ = app_server.thread_unsubscribe(session.thread_id).await;
|
||||
@@ -520,6 +553,7 @@ impl App {
|
||||
keymap,
|
||||
cwd,
|
||||
name_error,
|
||||
selected_profile,
|
||||
};
|
||||
if managed_worktree.is_some() {
|
||||
// Let the large synchronous ChatWidget constructor run on a fresh event-loop stack.
|
||||
@@ -544,6 +578,7 @@ impl App {
|
||||
keymap,
|
||||
cwd,
|
||||
name_error,
|
||||
selected_profile,
|
||||
} = attach;
|
||||
self.local_settings = local_settings;
|
||||
self.refresh_server_version_overview_notice(CODEX_CLI_VERSION);
|
||||
@@ -560,6 +595,9 @@ impl App {
|
||||
if let Err(error) = attach_widget(self, tui, started, lineage, message).await {
|
||||
return self.working_directory_error(format!("Could not restore session: {error}"));
|
||||
}
|
||||
if selected_profile.is_some() {
|
||||
self.adopt_inherited_server_selection();
|
||||
}
|
||||
if matches!(
|
||||
self.runtime_approval_policy_override,
|
||||
Some(RuntimeApprovalPolicyOverride::Restored(_))
|
||||
|
||||
@@ -793,6 +793,13 @@ impl AppServerSession {
|
||||
if history_support == ThreadHistorySupport::LegacyOnly {
|
||||
self.history_support = ThreadHistorySupport::LegacyOnly;
|
||||
}
|
||||
// An explicit server selection must not reuse the client's local display policy.
|
||||
let selected_permissions = selected_profile.map(|_| {
|
||||
PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&response.sandbox.to_core(),
|
||||
response.cwd.as_path(),
|
||||
)
|
||||
});
|
||||
let mut started = started_thread_from_start_response(
|
||||
response,
|
||||
local_settings,
|
||||
@@ -800,6 +807,9 @@ impl AppServerSession {
|
||||
self.thread_params_mode(),
|
||||
)
|
||||
.await?;
|
||||
if let Some(permissions) = selected_permissions {
|
||||
started.session.permission_profile = permissions;
|
||||
}
|
||||
started.task_tools_available = task_tools_available;
|
||||
if task_tools_available {
|
||||
self.remember_task_tool_thread(started.session.thread_id);
|
||||
@@ -879,6 +889,7 @@ impl AppServerSession {
|
||||
local_settings: &LocalSettings,
|
||||
config: Config,
|
||||
thread_id: ThreadId,
|
||||
selected_profile: Option<&PermissionProfileSelection>,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
self.fork_thread_at_with_presentation(
|
||||
local_settings,
|
||||
@@ -888,7 +899,7 @@ impl AppServerSession {
|
||||
/*before_turn_id*/ None,
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
ForkPresentation::SideConversation,
|
||||
/*selected_profile*/ None,
|
||||
selected_profile,
|
||||
ForkPermissionMode::InheritSaved,
|
||||
ForkConfigSource::Session,
|
||||
)
|
||||
@@ -957,6 +968,7 @@ impl AppServerSession {
|
||||
}
|
||||
if self.thread_params_mode() == ThreadParamsMode::Remote
|
||||
&& permission_mode == ForkPermissionMode::InheritSaved
|
||||
&& selected_profile.is_none()
|
||||
{
|
||||
params.approval_policy = None;
|
||||
params.approvals_reviewer = None;
|
||||
@@ -1022,6 +1034,13 @@ impl AppServerSession {
|
||||
"preserving the created fork after bounded history hydration failed"
|
||||
);
|
||||
}
|
||||
// Explicit selections use the server's effective policy, including on local daemons.
|
||||
let selected_permissions = selected_profile.map(|_| {
|
||||
PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&response.sandbox.to_core(),
|
||||
response.cwd.as_path(),
|
||||
)
|
||||
});
|
||||
let mut started = started_thread_from_fork_response(
|
||||
response,
|
||||
local_settings,
|
||||
@@ -1029,6 +1048,9 @@ impl AppServerSession {
|
||||
self.thread_params_mode(),
|
||||
)
|
||||
.await?;
|
||||
if let Some(permissions) = selected_permissions {
|
||||
started.session.permission_profile = permissions;
|
||||
}
|
||||
started.session.fork_parent_title = fork_parent.and_then(|thread| thread.name);
|
||||
if self.task_tools_available(thread_id) {
|
||||
started.task_tools_available = true;
|
||||
@@ -3660,6 +3682,7 @@ mod tests {
|
||||
&LocalSettings::from(&ephemeral_config),
|
||||
ephemeral_config,
|
||||
source_thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -3786,6 +3809,7 @@ mod tests {
|
||||
&LocalSettings::from(&side_config),
|
||||
side_config,
|
||||
source_thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -3917,7 +3941,12 @@ mod tests {
|
||||
.fork_thread(&LocalSettings::from(&config), config.clone(), thread_id)
|
||||
.await?;
|
||||
let side = app_server
|
||||
.fork_side_thread(&LocalSettings::from(&config), config, thread_id)
|
||||
.fork_side_thread(
|
||||
&LocalSettings::from(&config),
|
||||
config,
|
||||
thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(regular.turns.len(), 1);
|
||||
|
||||
@@ -130,13 +130,7 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override
|
||||
),
|
||||
);
|
||||
}
|
||||
// A locally remembered profile may have been removed since selection.
|
||||
let stale_selection = crate::app_event::PermissionProfileSelection {
|
||||
profile_id: "removed-profile".into(),
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
display_label: "removed-profile".into(),
|
||||
};
|
||||
// With no new selection, remote forks retain the saved profile.
|
||||
let forked = server
|
||||
.fork_thread_at(
|
||||
&local_settings,
|
||||
@@ -145,7 +139,7 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override
|
||||
/*last_turn_id*/ None,
|
||||
/*before_turn_id*/ None,
|
||||
ForkGoalContinuation::StartIfIdle,
|
||||
Some(&stale_selection),
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
@@ -165,7 +159,12 @@ async fn remote_resume_restores_saved_server_profile_without_permission_override
|
||||
codex_protocol::config_types::ApprovalsReviewer::AutoReview
|
||||
);
|
||||
let side = server
|
||||
.fork_side_thread(&local_settings, client_config, thread_id)
|
||||
.fork_side_thread(
|
||||
&local_settings,
|
||||
client_config,
|
||||
thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
side.session.active_permission_profile.unwrap().id,
|
||||
|
||||
@@ -150,7 +150,12 @@ stream_max_retries = 0
|
||||
.await?;
|
||||
assert_eq!(forked.session.runtime_workspace_roots, expected_roots);
|
||||
let side = app_server
|
||||
.fork_side_thread(&local_settings, session_config, thread_id)
|
||||
.fork_side_thread(
|
||||
&local_settings,
|
||||
session_config,
|
||||
thread_id,
|
||||
/*selected_profile*/ None,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(side.session.runtime_workspace_roots, expected_roots);
|
||||
|
||||
|
||||
@@ -56,9 +56,6 @@ impl ChatWidget {
|
||||
return;
|
||||
}
|
||||
match result {
|
||||
Ok(discovery) if !discovery.explicit_profile_mode => {
|
||||
self.open_legacy_permissions_popup()
|
||||
}
|
||||
Ok(discovery) => {
|
||||
self.permission_profiles_menu_opened = true;
|
||||
self.open_permission_profiles_popup(discovery);
|
||||
|
||||
@@ -17,7 +17,8 @@ impl ChatWidget {
|
||||
|
||||
/// Open a popup to choose the permissions mode.
|
||||
pub(crate) fn open_permissions_popup(&mut self) {
|
||||
if self.config.explicit_permission_profile_mode
|
||||
if self.thread_id.is_some()
|
||||
|| self.config.explicit_permission_profile_mode
|
||||
|| self.permission_profiles_menu_opened
|
||||
|| self
|
||||
.config
|
||||
|
||||
@@ -215,8 +215,17 @@ impl ChatWidget {
|
||||
SelectionItem {
|
||||
name: label.to_string(),
|
||||
description: Some(description),
|
||||
is_current: active_profile_id.as_deref() == Some(id)
|
||||
&& current_approval == approval_policy
|
||||
is_current: active_profile_id.as_deref().map_or_else(
|
||||
|| {
|
||||
Self::preset_matches_current(
|
||||
current_approval,
|
||||
self.config.permissions.permission_profile(),
|
||||
self.config.cwd.as_path(),
|
||||
preset,
|
||||
)
|
||||
},
|
||||
|active| active == id,
|
||||
) && current_approval == approval_policy
|
||||
&& current_reviewer == approvals_reviewer,
|
||||
actions: self.permission_mode_actions(
|
||||
preset,
|
||||
@@ -226,9 +235,11 @@ impl ChatWidget {
|
||||
/*return_to_permissions*/ true,
|
||||
),
|
||||
dismiss_on_select: true,
|
||||
disabled_reason: discovery
|
||||
.disabled_reason(id, Some(approval_policy), Some(approvals_reviewer.into()))
|
||||
.or_else(|| self.permission_mode_disabled_reason(preset, approval_policy)),
|
||||
disabled_reason: discovery.disabled_reason(
|
||||
id,
|
||||
Some(approval_policy),
|
||||
Some(approvals_reviewer.into()),
|
||||
),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,6 +31,7 @@ impl ChatWidget {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn set_permission_profile_with_active_profile(
|
||||
&mut self,
|
||||
profile: PermissionProfile,
|
||||
@@ -449,7 +450,16 @@ impl ChatWidget {
|
||||
self.apply_thread_settings_cwd(settings.cwd.clone());
|
||||
self.config.model_provider_id = settings.model_provider.clone();
|
||||
self.set_service_tier(settings.service_tier.clone());
|
||||
self.set_approval_policy(settings.approval_policy);
|
||||
if let Err(err) = self
|
||||
.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(settings.approval_policy.to_core())
|
||||
{
|
||||
tracing::warn!(%err, "failed to sync approval_policy from ThreadSettingsUpdated");
|
||||
self.config.permissions.approval_policy =
|
||||
Constrained::allow_only(settings.approval_policy.to_core());
|
||||
}
|
||||
self.set_approvals_reviewer(settings.approvals_reviewer.to_core());
|
||||
self.config.personality = settings.personality;
|
||||
|
||||
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/permissions.rs
|
||||
expression: popup
|
||||
---
|
||||
Update Model Permissions
|
||||
|
||||
|
||||
1. Ask for approval Read and edit workspace files and run commands, with approval required for internet
|
||||
access or edits outside the workspace
|
||||
› 2. Approve for me (current) Only ask for actions detected as potentially unsafe
|
||||
3. Full Access Use with caution: Codex can edit files outside this workspace and access the internet
|
||||
without approval
|
||||
4. Read Only Read workspace files, with approval required for edits or internet access
|
||||
|
||||
enter select · esc back
|
||||
@@ -625,6 +625,8 @@ async fn thread_settings_updated_updates_visible_state_without_transcript() {
|
||||
let mut session = configured_thread_session(thread_id);
|
||||
session.cwd = test_path_buf("/tmp/original-workspace").abs();
|
||||
chat.handle_thread_session(session);
|
||||
chat.config.permissions.approval_policy =
|
||||
Constrained::allow_only(AskForApproval::Never.to_core());
|
||||
let previous_generation = chat.connector_scope_generation();
|
||||
let old_app = serde_json::from_str(r#"{"id":"old","name":"Old","isAccessible":true}"#)
|
||||
.expect("valid app");
|
||||
|
||||
@@ -60,7 +60,13 @@ async fn permission_discovery_uses_server_catalog_for_remote_custom_selection()
|
||||
}))
|
||||
.unwrap(),
|
||||
);
|
||||
chat.request_permission_profiles();
|
||||
// Conflicting local availability must not disable server-provided choices.
|
||||
chat.config.config_layer_stack = requirements_stack(codex_config::ConfigRequirementsToml {
|
||||
allowed_sandbox_modes: Some(vec![codex_config::SandboxModeRequirement::ReadOnly]),
|
||||
..Default::default()
|
||||
});
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
chat.open_permissions_popup();
|
||||
let request_id = chat.permission_popup_request_id.unwrap();
|
||||
rx.try_recv().unwrap();
|
||||
assert_chatwidget_snapshot!(
|
||||
@@ -79,16 +85,6 @@ async fn permission_discovery_uses_server_catalog_for_remote_custom_selection()
|
||||
AppEvent::SelectPermissionProfile(PermissionProfileSelection { profile_id, .. })
|
||||
if profile_id == "server-only"
|
||||
));
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Esc));
|
||||
chat.open_permissions_popup();
|
||||
rx.try_recv().unwrap();
|
||||
let request_id = chat.permission_popup_request_id.unwrap();
|
||||
let mut legacy = Discovery::local(&chat.config);
|
||||
legacy.explicit_profile_mode = false;
|
||||
chat.on_permission_profiles_loaded(request_id, Ok(legacy));
|
||||
let actual = render_bottom_popup(&chat, /*width*/ 110);
|
||||
chat.open_legacy_permissions_popup();
|
||||
assert_eq!(actual, render_bottom_popup(&chat, /*width*/ 110));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -265,7 +261,14 @@ async fn profile_permissions_selection_popup_with_disallowed_full_access_snapsho
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
chat.open_permission_profiles_popup(Discovery::local(&chat.config));
|
||||
let mut discovery = Discovery::local(&chat.config);
|
||||
discovery
|
||||
.profiles
|
||||
.iter_mut()
|
||||
.find(|profile| profile.id == ":danger-full-access")
|
||||
.unwrap()
|
||||
.allowed = false;
|
||||
chat.open_permission_profiles_popup(discovery);
|
||||
|
||||
assert_chatwidget_snapshot!(
|
||||
"profile_permissions_selection_popup_with_disallowed_full_access",
|
||||
@@ -1234,8 +1237,13 @@ async fn permissions_selection_marks_auto_review_current_after_session_configure
|
||||
});
|
||||
|
||||
chat.open_permissions_popup();
|
||||
chat.on_permission_profiles_loaded(
|
||||
chat.permission_popup_request_id.unwrap(),
|
||||
Ok(Discovery::local(&chat.config)),
|
||||
);
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 120);
|
||||
|
||||
assert_chatwidget_snapshot!("permissions_unnamed_server_profile", popup);
|
||||
assert!(
|
||||
popup.contains("Approve for me (current)"),
|
||||
"expected Approve for me to be current after SessionConfigured sync: {popup}"
|
||||
@@ -1283,6 +1291,10 @@ async fn permissions_selection_marks_auto_review_current_with_custom_workspace_w
|
||||
});
|
||||
|
||||
chat.open_permissions_popup();
|
||||
chat.on_permission_profiles_loaded(
|
||||
chat.permission_popup_request_id.unwrap(),
|
||||
Ok(Discovery::local(&chat.config)),
|
||||
);
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 120);
|
||||
|
||||
assert!(
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
//! Bounded discovery for permission pickers; fixed request IDs bound abandoned RPCs.
|
||||
//! This never applies a profile; the app chooses local or server-owned selection.
|
||||
//! Discovery and availability come from the connected server; selection uses its settings API.
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
@@ -10,14 +10,13 @@ use codex_app_server_client::TypedRequestError;
|
||||
use codex_app_server_protocol::ApprovalsReviewer;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigRequirements;
|
||||
use codex_app_server_protocol::ConfigRequirementsReadResponse;
|
||||
use codex_app_server_protocol::PermissionProfileListParams;
|
||||
use codex_app_server_protocol::PermissionProfileListResponse;
|
||||
use codex_app_server_protocol::PermissionProfileSummary;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
#[cfg(test)]
|
||||
use codex_utils_approval_presets::builtin_approval_presets;
|
||||
use std::collections::HashSet;
|
||||
use std::time::Duration;
|
||||
@@ -27,10 +26,10 @@ use uuid::Uuid;
|
||||
pub(crate) struct PermissionDiscovery {
|
||||
pub(crate) profiles: Vec<PermissionProfileSummary>,
|
||||
pub(crate) requirements: Option<ConfigRequirements>,
|
||||
pub(crate) explicit_profile_mode: bool,
|
||||
}
|
||||
|
||||
impl PermissionDiscovery {
|
||||
#[cfg(test)]
|
||||
pub(crate) fn local(config: &Config) -> Self {
|
||||
let mut profiles = builtin_approval_presets()
|
||||
.into_iter()
|
||||
@@ -50,7 +49,6 @@ impl PermissionDiscovery {
|
||||
Self {
|
||||
profiles,
|
||||
requirements: None,
|
||||
explicit_profile_mode: true,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -96,47 +94,8 @@ pub(crate) fn fetch(
|
||||
ThreadParamsMode::Remote => app_server.remote_cwd_override(),
|
||||
})
|
||||
.map(|cwd| cwd.to_string_lossy().into_owned());
|
||||
// The daemon's catalog cannot see permission definitions from this invocation.
|
||||
let local_discovery = (!app_server.uses_embedded_app_server()
|
||||
&& mode == ThreadParamsMode::Embedded
|
||||
&& config.config_layer_stack.layers_low_to_high().any(|layer| {
|
||||
matches!(layer.name, codex_config::ConfigLayerSource::SessionFlags)
|
||||
&& layer.config.get("permissions").is_some()
|
||||
}))
|
||||
.then(|| PermissionDiscovery::local(config));
|
||||
let active_custom_profile = config
|
||||
.permissions
|
||||
.active_permission_profile()
|
||||
.is_some_and(|profile| !profile.id.starts_with(':'));
|
||||
tokio::spawn(async move {
|
||||
let request = async {
|
||||
if let Some(discovery) = local_discovery {
|
||||
return Ok(discovery);
|
||||
}
|
||||
if mode == ThreadParamsMode::Remote && !active_custom_profile {
|
||||
let config: ConfigReadResponse = handle
|
||||
.request_typed(ClientRequest::ConfigRead {
|
||||
request_id: RequestId::String("tui-permission-config".to_string()),
|
||||
params: ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: cwd.clone(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.map_err(discovery_error)?;
|
||||
if !config
|
||||
.config
|
||||
.additional
|
||||
.get("default_permissions")
|
||||
.is_some_and(serde_json::Value::is_string)
|
||||
{
|
||||
return Ok(PermissionDiscovery {
|
||||
profiles: Vec::new(),
|
||||
requirements: None,
|
||||
explicit_profile_mode: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
let requirements: ConfigRequirementsReadResponse = handle
|
||||
.request_typed(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::String("tui-permission-requirements".to_string()),
|
||||
@@ -174,7 +133,6 @@ pub(crate) fn fetch(
|
||||
return Ok(PermissionDiscovery {
|
||||
profiles,
|
||||
requirements: requirements.requirements,
|
||||
explicit_profile_mode: true,
|
||||
});
|
||||
};
|
||||
if !cursors.insert(next.clone()) {
|
||||
|
||||
@@ -76,14 +76,7 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() {
|
||||
json!({"result": {"data": [data[1]], "nextCursor": null}}),
|
||||
],
|
||||
});
|
||||
if mode == ThreadParamsMode::Remote {
|
||||
replies.insert(
|
||||
/*index*/ 0,
|
||||
json!({"result": {"config": {
|
||||
"default_permissions": (case != "legacy").then_some(":workspace")
|
||||
}, "origins": {}}}),
|
||||
);
|
||||
} else if case == "timeout" {
|
||||
if case == "timeout" {
|
||||
replies = vec![Value::Null];
|
||||
}
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||
@@ -151,15 +144,7 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() {
|
||||
continue;
|
||||
}
|
||||
let discovery = result.unwrap();
|
||||
assert_eq!(discovery.explicit_profile_mode, case != "legacy");
|
||||
if case == "session-only" {
|
||||
assert!(
|
||||
discovery
|
||||
.profiles
|
||||
.iter()
|
||||
.any(|profile| profile.id == "session-only" && profile.allowed)
|
||||
);
|
||||
} else if matches!(case, "empty" | "legacy") {
|
||||
if case == "empty" {
|
||||
assert!(discovery.profiles.is_empty());
|
||||
} else {
|
||||
assert_eq!(serde_json::to_value(&discovery.profiles).unwrap(), data);
|
||||
@@ -172,9 +157,8 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() {
|
||||
session.shutdown().await.unwrap();
|
||||
let requests = server.await.unwrap();
|
||||
match case {
|
||||
"session-only" => assert!(requests.is_empty()),
|
||||
"timeout" | "legacy" => assert_eq!(requests.len(), 1),
|
||||
"local" | "remote" | "remote-default" | "thread" => {
|
||||
"timeout" => assert_eq!(requests.len(), 1),
|
||||
"local" | "session-only" | "remote" | "remote-default" | "thread" | "legacy" => {
|
||||
let pages: Vec<_> = requests
|
||||
.iter()
|
||||
.filter(|request| request["method"] == "permissionProfile/list")
|
||||
@@ -187,9 +171,6 @@ async fn discovery_preserves_config_scope_and_bounds_server_requests() {
|
||||
json!({"cwd": cwd, "limit": 100, "cursor": "second"})
|
||||
]
|
||||
);
|
||||
if mode == ThreadParamsMode::Remote {
|
||||
assert_eq!(requests[0]["params"]["cwd"], json!(cwd));
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user