mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
fix(tui): restore managed network for auto review
This commit is contained in:
@@ -7,10 +7,13 @@ use crate::AppServerTarget;
|
||||
use crate::app_backtrack::BacktrackState;
|
||||
use crate::app_command::AppCommand;
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event::AutoReviewPresetSelection;
|
||||
use crate::app_event::ExitMode;
|
||||
use crate::app_event::FeedbackCategory;
|
||||
use crate::app_event::HistoryLookupResponse;
|
||||
use crate::app_event::ManagedNetworkChoice;
|
||||
use crate::app_event::PaneSlot;
|
||||
use crate::app_event::PermissionPresetProfileUpdate;
|
||||
use crate::app_event::PermissionProfileSelection;
|
||||
use crate::app_event::PluginLocation;
|
||||
use crate::app_event::PluginRemoteSectionError;
|
||||
@@ -90,6 +93,7 @@ use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CodexErrorInfo as AppServerCodexErrorInfo;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
@@ -205,6 +209,7 @@ mod agent_status_feed;
|
||||
mod app_server_event_targets;
|
||||
mod app_server_events;
|
||||
pub(crate) mod app_server_requests;
|
||||
mod auto_review_permissions;
|
||||
mod background_requests;
|
||||
mod config_persistence;
|
||||
mod conversation_events;
|
||||
|
||||
@@ -0,0 +1,254 @@
|
||||
//! Auto Review permission transitions and managed-network restoration.
|
||||
|
||||
use super::*;
|
||||
|
||||
const WORKSPACE_NETWORK_ACCESS_KEY: &str = "sandbox_workspace_write.network_access";
|
||||
const WORKSPACE_NETWORK_ACCESS_POINTER: &str = "/sandbox_workspace_write/network_access";
|
||||
|
||||
fn user_network_restriction_overrides_managed_enabled(response: &ConfigReadResponse) -> bool {
|
||||
let user_override_is_effective = response
|
||||
.origins
|
||||
.get(WORKSPACE_NETWORK_ACCESS_KEY)
|
||||
.is_some_and(|metadata| matches!(metadata.name, ConfigLayerSource::User { .. }));
|
||||
user_override_is_effective
|
||||
&& response.layers.as_ref().is_some_and(|layers| {
|
||||
layers.iter().any(|layer| {
|
||||
matches!(
|
||||
layer.name,
|
||||
ConfigLayerSource::Mdm { .. }
|
||||
| ConfigLayerSource::System { .. }
|
||||
| ConfigLayerSource::EnterpriseManaged { .. }
|
||||
| ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. }
|
||||
| ConfigLayerSource::LegacyManagedConfigTomlFromMdm
|
||||
) && layer
|
||||
.config
|
||||
.pointer(WORKSPACE_NETWORK_ACCESS_POINTER)
|
||||
.and_then(serde_json::Value::as_bool)
|
||||
== Some(true)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) async fn apply_auto_review_preset(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
selection: AutoReviewPresetSelection,
|
||||
network_choice: ManagedNetworkChoice,
|
||||
) {
|
||||
let terminal_browser_enabled = self.config.features.enabled(Feature::TerminalBrowser);
|
||||
let network_is_restricted = !self
|
||||
.config
|
||||
.permissions
|
||||
.network_sandbox_policy()
|
||||
.is_enabled();
|
||||
if network_choice == ManagedNetworkChoice::Detect
|
||||
&& terminal_browser_enabled
|
||||
&& network_is_restricted
|
||||
{
|
||||
let cwd = self.chat_widget.config_ref().cwd.display().to_string();
|
||||
match crate::config_update::read_effective_config_with_layers(
|
||||
app_server.request_handle(),
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(response) if user_network_restriction_overrides_managed_enabled(&response) => {
|
||||
self.chat_widget
|
||||
.open_managed_network_restore_confirmation(selection);
|
||||
return;
|
||||
}
|
||||
Ok(_) => {}
|
||||
Err(err) => tracing::warn!(
|
||||
error = %err,
|
||||
"failed to inspect managed network config before enabling Auto Review"
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
let restore_managed_network = network_choice == ManagedNetworkChoice::RestoreManaged;
|
||||
let mut next_config = self.config.clone();
|
||||
if !self.try_set_approval_policy_on_config(
|
||||
&mut next_config,
|
||||
selection.approval_policy,
|
||||
"Failed to enable Approve for me",
|
||||
"failed to set Auto Review approval policy",
|
||||
) {
|
||||
return;
|
||||
}
|
||||
next_config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
|
||||
let mut edits = vec![crate::config_update::replace_config_value(
|
||||
"approvals_reviewer",
|
||||
serde_json::json!(ApprovalsReviewer::AutoReview.to_string()),
|
||||
)];
|
||||
if restore_managed_network {
|
||||
edits.push(crate::config_update::clear_config_value(
|
||||
WORKSPACE_NETWORK_ACCESS_KEY,
|
||||
));
|
||||
}
|
||||
if let Err(err) =
|
||||
crate::config_update::write_config_batch(app_server.request_handle(), edits).await
|
||||
{
|
||||
tracing::error!(error = %err, "failed to persist Auto Review selection");
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to update Auto Review permissions: {}",
|
||||
crate::config_update::format_config_error(&err)
|
||||
));
|
||||
return;
|
||||
}
|
||||
|
||||
if restore_managed_network {
|
||||
next_config = match self
|
||||
.rebuild_config_for_cwd(self.chat_widget.config_ref().cwd.to_path_buf())
|
||||
.await
|
||||
{
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
tracing::error!(
|
||||
error = %err,
|
||||
"failed to rebuild config after restoring managed network access"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Managed network access was restored on disk, but the current session could not refresh it: {err}"
|
||||
));
|
||||
return;
|
||||
}
|
||||
};
|
||||
if !self.try_set_approval_policy_on_config(
|
||||
&mut next_config,
|
||||
selection.approval_policy,
|
||||
"Failed to enable Approve for me",
|
||||
"failed to restore Auto Review approval policy",
|
||||
) {
|
||||
return;
|
||||
}
|
||||
next_config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
}
|
||||
|
||||
let (permission_profile, active_permission_profile) = if restore_managed_network {
|
||||
if !next_config
|
||||
.permissions
|
||||
.network_sandbox_policy()
|
||||
.is_enabled()
|
||||
{
|
||||
self.chat_widget.add_error_message(
|
||||
"The user network override was removed, but managed network access is still disabled."
|
||||
.to_string(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
(
|
||||
Some(next_config.permissions.permission_profile().clone()),
|
||||
next_config.permissions.active_permission_profile(),
|
||||
)
|
||||
} else {
|
||||
match &selection.profile_update {
|
||||
PermissionPresetProfileUpdate::Preserve => (None, None),
|
||||
PermissionPresetProfileUpdate::Replace {
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
} => (
|
||||
Some(permission_profile.clone()),
|
||||
Some(active_permission_profile.clone()),
|
||||
),
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(permission_profile) = permission_profile.as_ref()
|
||||
&& let Err(err) = next_config
|
||||
.permissions
|
||||
.set_permission_profile_from_session_snapshot(
|
||||
PermissionProfileSnapshot::from_session_snapshot(
|
||||
permission_profile.clone(),
|
||||
active_permission_profile.clone(),
|
||||
),
|
||||
)
|
||||
{
|
||||
tracing::warn!(error = %err, "failed to apply Auto Review permission profile");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to enable Approve for me: {err}"));
|
||||
return;
|
||||
}
|
||||
|
||||
let permission_network = next_config.permissions.network.clone();
|
||||
self.config = next_config;
|
||||
self.runtime_approval_policy_override = Some(selection.approval_policy);
|
||||
self.chat_widget
|
||||
.set_approval_policy(selection.approval_policy);
|
||||
self.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::AutoReview);
|
||||
if let Some(permission_profile) = permission_profile.as_ref()
|
||||
&& let Err(err) = self
|
||||
.chat_widget
|
||||
.set_permission_profile_from_session_snapshot(
|
||||
PermissionProfileSnapshot::from_session_snapshot(
|
||||
permission_profile.clone(),
|
||||
active_permission_profile.clone(),
|
||||
),
|
||||
)
|
||||
{
|
||||
tracing::warn!(error = %err, "failed to apply Auto Review profile to chat config");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to enable Approve for me: {err}"));
|
||||
return;
|
||||
}
|
||||
self.chat_widget.set_permission_network(permission_network);
|
||||
if permission_profile.is_some() {
|
||||
self.runtime_permission_profile_override =
|
||||
Some(RuntimePermissionProfileOverride::from_config(&self.config));
|
||||
}
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
|
||||
let op = AppCommand::override_turn_context(
|
||||
/*cwd*/ None,
|
||||
Some(selection.approval_policy),
|
||||
Some(ApprovalsReviewer::AutoReview),
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
/*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;
|
||||
}
|
||||
|
||||
let network_enabled = self
|
||||
.config
|
||||
.permissions
|
||||
.network_sandbox_policy()
|
||||
.is_enabled();
|
||||
let message = if restore_managed_network {
|
||||
format!(
|
||||
"Permissions updated to {}; managed network access enabled",
|
||||
selection.display_label
|
||||
)
|
||||
} else if matches!(
|
||||
selection.profile_update,
|
||||
PermissionPresetProfileUpdate::Replace { .. }
|
||||
) {
|
||||
format!("Permissions updated to {}", selection.display_label)
|
||||
} else {
|
||||
format!("Review mode updated to {}", selection.display_label)
|
||||
};
|
||||
let hint = (terminal_browser_enabled && !network_enabled).then(|| {
|
||||
"Network access remains disabled; the terminal browser is unavailable.".to_string()
|
||||
});
|
||||
self.chat_widget.add_info_message(message, hint);
|
||||
self.chat_widget.submit_initial_user_message_if_pending();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "auto_review_permissions_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,256 @@
|
||||
use super::*;
|
||||
use crate::app::test_support::make_test_app_with_channels;
|
||||
use crate::chatwidget::tests::helpers::render_bottom_popup;
|
||||
use crate::test_support::PathBufExt;
|
||||
use assert_matches::assert_matches;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
fn auto_review_selection() -> AutoReviewPresetSelection {
|
||||
AutoReviewPresetSelection {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
profile_update: PermissionPresetProfileUpdate::Preserve,
|
||||
display_label: "Approve for me".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
fn next_history_message(rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>) -> String {
|
||||
let cell = std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.find_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => Some(cell),
|
||||
_ => None,
|
||||
})
|
||||
.expect("expected permission transition history");
|
||||
cell.display_lines(/*width*/ 120)
|
||||
.into_iter()
|
||||
.map(|line| line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminal_browser_detects_user_network_restriction_over_managed_enabled_default()
|
||||
-> color_eyre::eyre::Result<()> {
|
||||
let config_dir = tempdir()?;
|
||||
let user_config = config_dir.path().join("user-config.toml");
|
||||
let system_config = config_dir.path().join("system-config.toml");
|
||||
let response: ConfigReadResponse = serde_json::from_value(serde_json::json!({
|
||||
"config": {
|
||||
"sandbox_workspace_write": { "network_access": false },
|
||||
},
|
||||
"origins": {
|
||||
WORKSPACE_NETWORK_ACCESS_KEY: {
|
||||
"name": {
|
||||
"type": "user",
|
||||
"file": user_config,
|
||||
"profile": null,
|
||||
},
|
||||
"version": "user-v1",
|
||||
},
|
||||
},
|
||||
"layers": [
|
||||
{
|
||||
"name": {
|
||||
"type": "user",
|
||||
"file": user_config,
|
||||
"profile": null,
|
||||
},
|
||||
"version": "user-v1",
|
||||
"config": {
|
||||
"sandbox_workspace_write": { "network_access": false },
|
||||
},
|
||||
},
|
||||
{
|
||||
"name": {
|
||||
"type": "system",
|
||||
"file": system_config,
|
||||
},
|
||||
"version": "system-v1",
|
||||
"config": {
|
||||
"sandbox_workspace_write": { "network_access": true },
|
||||
},
|
||||
},
|
||||
],
|
||||
}))?;
|
||||
|
||||
assert!(user_network_restriction_overrides_managed_enabled(
|
||||
&response
|
||||
));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn terminal_browser_auto_review_restores_managed_network_override_before_reporting_success()
|
||||
-> color_eyre::eyre::Result<()> {
|
||||
let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
let config_dir = tempdir()?;
|
||||
let user_config_path = config_dir.path().join("config.toml").abs();
|
||||
let system_config_path = config_dir.path().join("system-config.toml");
|
||||
std::fs::write(
|
||||
&system_config_path,
|
||||
r#"
|
||||
sandbox_mode = "workspace-write"
|
||||
|
||||
[sandbox_workspace_write]
|
||||
network_access = true
|
||||
|
||||
[features]
|
||||
guardian_approval = true
|
||||
terminal_browser = true
|
||||
"#,
|
||||
)?;
|
||||
std::fs::write(
|
||||
user_config_path.as_path(),
|
||||
r#"
|
||||
approvals_reviewer = "auto_review"
|
||||
approval_policy = "on-request"
|
||||
sandbox_mode = "workspace-write"
|
||||
|
||||
[sandbox_workspace_write]
|
||||
network_access = false
|
||||
|
||||
[features]
|
||||
guardian_approval = true
|
||||
terminal_browser = true
|
||||
"#,
|
||||
)?;
|
||||
app.config.codex_home = config_dir.path().to_path_buf().abs();
|
||||
app.loader_overrides.user_config_path = Some(user_config_path.clone());
|
||||
app.loader_overrides.system_config_path = Some(system_config_path);
|
||||
let rebuilt = app
|
||||
.rebuild_config_for_cwd(app.config.cwd.to_path_buf())
|
||||
.await?;
|
||||
assert_eq!(
|
||||
rebuilt.permissions.network_sandbox_policy(),
|
||||
NetworkSandboxPolicy::Restricted
|
||||
);
|
||||
app.config = rebuilt.clone();
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ true);
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true);
|
||||
app.chat_widget
|
||||
.set_permission_profile_from_session_snapshot(
|
||||
PermissionProfileSnapshot::from_session_snapshot(
|
||||
rebuilt.permissions.permission_profile().clone(),
|
||||
rebuilt.permissions.active_permission_profile(),
|
||||
),
|
||||
)?;
|
||||
app.chat_widget
|
||||
.set_permission_network(rebuilt.permissions.network.clone());
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
|
||||
app.apply_auto_review_preset(
|
||||
&mut app_server,
|
||||
auto_review_selection(),
|
||||
ManagedNetworkChoice::Detect,
|
||||
)
|
||||
.await;
|
||||
|
||||
let popup = render_bottom_popup(&app.chat_widget, /*width*/ 100);
|
||||
assert!(popup.contains("Enable managed network access?"), "{popup}");
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
let (selection, network_choice) = std::iter::from_fn(|| app_event_rx.try_recv().ok())
|
||||
.find_map(|event| match event {
|
||||
AppEvent::ApplyAutoReviewPreset {
|
||||
selection,
|
||||
network_choice,
|
||||
} => Some((selection, network_choice)),
|
||||
_ => None,
|
||||
})
|
||||
.expect("restore confirmation should emit an Auto Review transition");
|
||||
assert_eq!(network_choice, ManagedNetworkChoice::RestoreManaged);
|
||||
|
||||
app.apply_auto_review_preset(&mut app_server, selection, network_choice)
|
||||
.await;
|
||||
|
||||
assert!(app.config.permissions.network_sandbox_policy().is_enabled());
|
||||
assert!(
|
||||
app.chat_widget
|
||||
.config_ref()
|
||||
.permissions
|
||||
.network_sandbox_policy()
|
||||
.is_enabled()
|
||||
);
|
||||
assert_matches!(
|
||||
op_rx.try_recv().expect("expected permission context update"),
|
||||
AppCommand::OverrideTurnContext {
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
permission_profile: Some(profile),
|
||||
..
|
||||
} if profile.network_sandbox_policy().is_enabled()
|
||||
);
|
||||
let rendered = next_history_message(&mut app_event_rx);
|
||||
assert!(
|
||||
rendered.contains("Permissions updated to Approve for me; managed network access enabled"),
|
||||
"{rendered}"
|
||||
);
|
||||
let user_config = std::fs::read_to_string(user_config_path.as_path())?;
|
||||
assert!(!user_config.contains("network_access = false"));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn terminal_browser_auto_review_keep_restricted_reports_review_only()
|
||||
-> color_eyre::eyre::Result<()> {
|
||||
let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||
app.config
|
||||
.features
|
||||
.enable(Feature::TerminalBrowser)
|
||||
.expect("terminal browser feature should be mutable");
|
||||
app.chat_widget
|
||||
.set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true);
|
||||
assert_eq!(
|
||||
app.config.permissions.network_sandbox_policy(),
|
||||
NetworkSandboxPolicy::Restricted
|
||||
);
|
||||
let permission_profile = app.config.permissions.permission_profile().clone();
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
|
||||
app.apply_auto_review_preset(
|
||||
&mut app_server,
|
||||
auto_review_selection(),
|
||||
ManagedNetworkChoice::KeepRestricted,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
app.config.permissions.permission_profile(),
|
||||
&permission_profile
|
||||
);
|
||||
assert_eq!(
|
||||
op_rx.try_recv(),
|
||||
Ok(AppCommand::OverrideTurnContext {
|
||||
cwd: None,
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
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 rendered = next_history_message(&mut app_event_rx);
|
||||
assert!(
|
||||
rendered.contains("Review mode updated to Approve for me"),
|
||||
"{rendered}"
|
||||
);
|
||||
assert!(
|
||||
rendered.contains("Network access remains disabled"),
|
||||
"{rendered}"
|
||||
);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
@@ -2014,6 +2014,13 @@ impl App {
|
||||
.add_error_message(format!("Failed to save approvals reviewer: {err}"));
|
||||
}
|
||||
}
|
||||
AppEvent::ApplyAutoReviewPreset {
|
||||
selection,
|
||||
network_choice,
|
||||
} => {
|
||||
self.apply_auto_review_preset(app_server, selection, network_choice)
|
||||
.await;
|
||||
}
|
||||
AppEvent::UpdateFeatureFlags { updates } => {
|
||||
self.update_feature_flags(app_server, updates).await;
|
||||
}
|
||||
|
||||
@@ -54,6 +54,7 @@ use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_protocol::config_types::CollaborationModeMask;
|
||||
use codex_protocol::config_types::Personality;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
|
||||
use crate::history_cell::HistoryCell;
|
||||
@@ -1010,6 +1011,12 @@ pub(crate) enum AppEvent {
|
||||
/// Update the current approvals reviewer in the running app and widget.
|
||||
UpdateApprovalsReviewer(ApprovalsReviewer),
|
||||
|
||||
/// Apply the Auto Review preset after resolving any managed-network override.
|
||||
ApplyAutoReviewPreset {
|
||||
selection: AutoReviewPresetSelection,
|
||||
network_choice: ManagedNetworkChoice,
|
||||
},
|
||||
|
||||
/// Update feature flags and persist them to the top-level config.
|
||||
UpdateFeatureFlags {
|
||||
updates: Vec<(Feature, bool)>,
|
||||
@@ -1252,6 +1259,32 @@ pub(crate) struct PermissionProfileSelection {
|
||||
pub display_label: String,
|
||||
}
|
||||
|
||||
/// Permission-profile portion of an Auto Review preset transition.
|
||||
#[derive(Debug, Clone)]
|
||||
pub(crate) enum PermissionPresetProfileUpdate {
|
||||
Preserve,
|
||||
Replace {
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: ActivePermissionProfile,
|
||||
},
|
||||
}
|
||||
|
||||
/// Complete Auto Review selection passed from the permissions popup to the app layer.
|
||||
#[derive(Debug, Clone)]
|
||||
pub(crate) struct AutoReviewPresetSelection {
|
||||
pub(crate) approval_policy: AskForApproval,
|
||||
pub(crate) profile_update: PermissionPresetProfileUpdate,
|
||||
pub(crate) display_label: String,
|
||||
}
|
||||
|
||||
/// How an Auto Review transition should handle a user network override.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum ManagedNetworkChoice {
|
||||
Detect,
|
||||
RestoreManaged,
|
||||
KeepRestricted,
|
||||
}
|
||||
|
||||
/// The exit strategy requested by the UI layer.
|
||||
///
|
||||
/// Most user-initiated exits should use `ShutdownFirst` so core cleanup runs and the UI exits only
|
||||
|
||||
@@ -258,9 +258,12 @@ fn normalize_thread_name(name: &str) -> Option<String> {
|
||||
}
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event::AutoReviewPresetSelection;
|
||||
use crate::app_event::ConversationOrigin;
|
||||
use crate::app_event::ExitMode;
|
||||
use crate::app_event::ManagedNetworkChoice;
|
||||
use crate::app_event::PaneSlot;
|
||||
use crate::app_event::PermissionPresetProfileUpdate;
|
||||
use crate::app_event::PermissionProfileSelection;
|
||||
use crate::app_event::RateLimitRefreshOrigin;
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -480,15 +483,6 @@ const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1";
|
||||
const DEFAULT_STATUS_LINE_ITEMS: [&str; 2] = ["model-with-reasoning", "current-dir"];
|
||||
const MAX_AGENT_COPY_HISTORY: usize = 32;
|
||||
|
||||
#[derive(Clone)]
|
||||
enum PermissionPresetProfileUpdate {
|
||||
Preserve,
|
||||
Replace {
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: ActivePermissionProfile,
|
||||
},
|
||||
}
|
||||
|
||||
/// Common initialization parameters shared by all `ChatWidget` constructors.
|
||||
pub(crate) struct ChatWidgetInit {
|
||||
pub(crate) config: Config,
|
||||
|
||||
@@ -244,6 +244,19 @@ impl ChatWidget {
|
||||
label: String,
|
||||
approvals_reviewer: ApprovalsReviewer,
|
||||
) -> Vec<SelectionAction> {
|
||||
if approvals_reviewer == ApprovalsReviewer::AutoReview {
|
||||
return vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::ApplyAutoReviewPreset {
|
||||
selection: AutoReviewPresetSelection {
|
||||
approval_policy: approval,
|
||||
profile_update: profile_update.clone(),
|
||||
display_label: label.clone(),
|
||||
},
|
||||
network_choice: ManagedNetworkChoice::Detect,
|
||||
});
|
||||
})];
|
||||
}
|
||||
|
||||
vec![Box::new(move |tx| {
|
||||
let (permission_profile, active_permission_profile) = match &profile_update {
|
||||
PermissionPresetProfileUpdate::Preserve => (None, None),
|
||||
@@ -282,13 +295,68 @@ impl ChatWidget {
|
||||
tx.send(AppEvent::UpdateApprovalsReviewer(approvals_reviewer));
|
||||
tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_info_event(
|
||||
format!("Permissions updated to {label}"),
|
||||
match profile_update {
|
||||
PermissionPresetProfileUpdate::Preserve => {
|
||||
format!("Review mode updated to {label}")
|
||||
}
|
||||
PermissionPresetProfileUpdate::Replace { .. } => {
|
||||
format!("Permissions updated to {label}")
|
||||
}
|
||||
},
|
||||
/*hint*/ None,
|
||||
),
|
||||
)));
|
||||
})]
|
||||
}
|
||||
|
||||
pub(crate) fn open_managed_network_restore_confirmation(
|
||||
&mut self,
|
||||
selection: AutoReviewPresetSelection,
|
||||
) {
|
||||
let restore_selection = selection.clone();
|
||||
let keep_selection = selection;
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
title: Some("Enable managed network access?".to_string()),
|
||||
subtitle: Some(
|
||||
"Your user config disables the managed network required by the terminal browser."
|
||||
.to_string(),
|
||||
),
|
||||
footer_hint: Some(standard_popup_hint_line()),
|
||||
items: vec![
|
||||
SelectionItem {
|
||||
name: "Restore managed network access".to_string(),
|
||||
description: Some(
|
||||
"Remove the user override and inherit the managed network policy"
|
||||
.to_string(),
|
||||
),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::ApplyAutoReviewPreset {
|
||||
selection: restore_selection.clone(),
|
||||
network_choice: ManagedNetworkChoice::RestoreManaged,
|
||||
});
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: "Keep network disabled".to_string(),
|
||||
description: Some(
|
||||
"Use Auto Review without enabling the terminal browser".to_string(),
|
||||
),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::ApplyAutoReviewPreset {
|
||||
selection: keep_selection.clone(),
|
||||
network_choice: ManagedNetworkChoice::KeepRestricted,
|
||||
});
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
],
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
|
||||
pub(super) fn permission_profile_selection_actions(
|
||||
selection: PermissionProfileSelection,
|
||||
) -> Vec<SelectionAction> {
|
||||
|
||||
+12
@@ -0,0 +1,12 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/permissions.rs
|
||||
expression: "render_bottom_popup(&chat, 100)"
|
||||
---
|
||||
Enable managed network access?
|
||||
Your user config disables the managed network required by the terminal browser.
|
||||
|
||||
› 1. Restore managed network access Remove the user override and inherit the managed network
|
||||
policy
|
||||
2. Keep network disabled Use Auto Review without enabling the terminal browser
|
||||
|
||||
Press enter to confirm or esc to go back
|
||||
@@ -89,6 +89,22 @@ async fn approvals_selection_popup_snapshot() {
|
||||
assert_chatwidget_snapshot!("approvals_selection_popup", popup);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn terminal_browser_managed_network_restore_confirmation_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.open_managed_network_restore_confirmation(AutoReviewPresetSelection {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
profile_update: PermissionPresetProfileUpdate::Preserve,
|
||||
display_label: "Approve for me".to_string(),
|
||||
});
|
||||
|
||||
assert_chatwidget_snapshot!(
|
||||
"terminal_browser_managed_network_restore_confirmation",
|
||||
render_bottom_popup(&chat, /*width*/ 100)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn profile_permissions_selection_popup_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
@@ -745,7 +761,7 @@ async fn approvals_popup_navigation_skips_disabled() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn permissions_selection_emits_history_cell_when_selection_changes() {
|
||||
async fn permissions_auto_review_selection_defers_history_until_app_transition() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
@@ -758,16 +774,18 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() {
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert_eq!(
|
||||
cells.len(),
|
||||
1,
|
||||
"expected one permissions selection history cell"
|
||||
);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::<Vec<_>>();
|
||||
assert!(
|
||||
rendered.contains("Permissions updated to"),
|
||||
"expected permissions selection history message, got: {rendered}"
|
||||
events
|
||||
.iter()
|
||||
.any(|event| matches!(event, AppEvent::ApplyAutoReviewPreset { .. })),
|
||||
"expected app-owned Auto Review transition: {events:?}"
|
||||
);
|
||||
assert!(
|
||||
events
|
||||
.iter()
|
||||
.all(|event| !matches!(event, AppEvent::InsertHistoryCell(_))),
|
||||
"history must wait until the app applies and persists Auto Review: {events:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -868,8 +886,8 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() {
|
||||
);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("Permissions updated to"),
|
||||
"expected permissions update history message, got: {rendered}"
|
||||
rendered.contains("Review mode updated to Ask for approval"),
|
||||
"expected reviewer-only history message, got: {rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1106,40 +1124,27 @@ async fn terminal_browser_auto_review_preserves_network_enabled_workspace_profil
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
let op = std::iter::from_fn(|| rx.try_recv().ok())
|
||||
let (selection, network_choice) = std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.find_map(|event| match event {
|
||||
AppEvent::CodexOp(op @ Op::OverrideTurnContext { .. }) => Some(op),
|
||||
AppEvent::ApplyAutoReviewPreset {
|
||||
selection,
|
||||
network_choice,
|
||||
} => Some((selection, network_choice)),
|
||||
_ => None,
|
||||
})
|
||||
.expect("expected OverrideTurnContext op");
|
||||
|
||||
assert_eq!(
|
||||
op,
|
||||
Op::OverrideTurnContext {
|
||||
cwd: None,
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
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,
|
||||
}
|
||||
);
|
||||
.expect("expected app-owned Auto Review transition");
|
||||
assert_eq!(selection.approval_policy, AskForApproval::OnRequest);
|
||||
assert!(matches!(
|
||||
selection.profile_update,
|
||||
PermissionPresetProfileUpdate::Preserve
|
||||
));
|
||||
assert_eq!(selection.display_label, "Approve for me");
|
||||
assert_eq!(network_choice, ManagedNetworkChoice::Detect);
|
||||
|
||||
assert_eq!(
|
||||
chat.config.permissions.permission_profile(),
|
||||
&permission_profile
|
||||
);
|
||||
assert!(
|
||||
std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.all(|event| !matches!(event, AppEvent::UpdateActivePermissionProfile(_))),
|
||||
"reviewer-only changes must not replace the effective workspace profile"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -173,13 +173,28 @@ pub(crate) async fn write_trusted_project(
|
||||
pub(crate) async fn read_effective_config(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: String,
|
||||
) -> Result<ConfigReadResponse> {
|
||||
read_effective_config_with_layer_option(request_handle, cwd, /*include_layers*/ false).await
|
||||
}
|
||||
|
||||
pub(crate) async fn read_effective_config_with_layers(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: String,
|
||||
) -> Result<ConfigReadResponse> {
|
||||
read_effective_config_with_layer_option(request_handle, cwd, /*include_layers*/ true).await
|
||||
}
|
||||
|
||||
async fn read_effective_config_with_layer_option(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: String,
|
||||
include_layers: bool,
|
||||
) -> Result<ConfigReadResponse> {
|
||||
let request_id = RequestId::String(format!("tui-config-read-{}", Uuid::new_v4()));
|
||||
request_handle
|
||||
.request_typed(ClientRequest::ConfigRead {
|
||||
request_id,
|
||||
params: ConfigReadParams {
|
||||
include_layers: false,
|
||||
include_layers,
|
||||
cwd: Some(cwd),
|
||||
},
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user