fix(tui): restore browser access and chat focus

This commit is contained in:
Felipe Coury
2026-07-07 11:26:53 -03:00
parent 0530d747c0
commit b47aeadeec
12 changed files with 428 additions and 31 deletions
+13
View File
@@ -546,6 +546,8 @@ pub(crate) struct App {
pub(crate) terminal_browser_generation: u64,
/// Closed runtimes retained by conversation so explicit reopen preserves profile selection.
terminal_browser_reopenable: HashMap<ThreadId, terminal_browser::ReopenableTerminalBrowser>,
/// One browser open held while the user decides whether to restore managed network access.
terminal_browser_pending_open: Option<terminal_browser::PendingTerminalBrowserOpen>,
pub(crate) deferred_history_lines: Vec<crate::terminal_hyperlinks::HyperlinkLine>,
has_emitted_history_lines: bool,
pub(crate) enhanced_keys_supported: bool,
@@ -1067,6 +1069,7 @@ See the Codex keymap documentation for supported actions and examples."
terminal_browser_owner_thread_id: None,
terminal_browser_generation: 0,
terminal_browser_reopenable: HashMap::new(),
terminal_browser_pending_open: None,
deferred_history_lines: Vec::new(),
has_emitted_history_lines: false,
status_line_invalid_items_warned: status_line_invalid_items_warned.clone(),
@@ -1364,6 +1367,16 @@ See the Codex keymap documentation for supported actions and examples."
return Ok(AppRunControl::Continue);
}
TuiEvent::MousePrimary(mouse_event) => {
if Self::terminal_browser_control_click_returns_to_app(
mouse_event,
browser_viewport,
) && let Some(target) = self.active_terminal_browser_control_target()
{
self.end_terminal_browser_control(target);
tui.set_raw_mouse_events(/*raw_mouse_events*/ false);
self.handle_owned_screen_mouse_primary(tui, mouse_event);
return Ok(AppRunControl::Continue);
}
self.forward_terminal_browser_mouse(mouse_event.into(), browser_viewport)
.await;
return Ok(AppRunControl::Continue);
+32
View File
@@ -70,8 +70,15 @@ impl App {
) {
match &notification {
ServerNotification::ServerRequestResolved(notification) => {
let terminal_browser_open_resolved =
self.discard_pending_terminal_browser_open(&notification.request_id);
match ThreadId::from_string(&notification.thread_id) {
Ok(thread_id) => {
if terminal_browser_open_resolved
&& let Some(pane) = self.chat_widget.by_thread_id_mut(thread_id)
{
pane.dismiss_managed_network_restore_confirmation();
}
if let Some(request) = self
.pending_app_server_requests
.resolve_notification(thread_id, &notification.request_id)
@@ -248,6 +255,31 @@ impl App {
});
return;
}
if params.tool == "open"
&& let Some(selection) = self.current_auto_review_selection()
&& self
.managed_network_restore_available(app_server_client)
.await
{
if !self.defer_terminal_browser_open(
request_id.clone(),
params.thread_id.clone(),
params.arguments.clone(),
) {
self.app_event_tx
.send(AppEvent::TerminalBrowserToolCompleted {
request_id: request_id.clone(),
response: dynamic_tool_response(Err(anyhow::anyhow!(
"browser_busy: another terminal browser open is waiting for permission"
))),
profile_approval: None,
});
return;
}
self.chat_widget
.open_managed_network_restore_confirmation(selection);
return;
}
let Some(browser) = self.terminal_browser_for_active_request().await else {
self.app_event_tx
.send(AppEvent::TerminalBrowserToolCompleted {
@@ -1,15 +1,26 @@
use super::*;
use crate::app::conversation_panes::ConversationPaneInit;
use crate::app::test_support::make_test_app_with_channels;
use crate::app_event::ManagedNetworkChoice;
use crate::app_event::PaneSlot;
use crate::chatwidget::tests::constructor::make_chatwidget_for_pane_with_sender;
use crate::chatwidget::tests::helpers::render_bottom_popup;
use crate::file_search::FileSearchManager;
use crate::legacy_core::config::PermissionProfileSnapshot;
use crate::test_support::PathBufExt;
use codex_app_server_protocol::AccountUpdatedNotification;
use codex_app_server_protocol::ConfigWarningNotification;
use codex_app_server_protocol::DynamicToolCallParams;
use codex_app_server_protocol::McpServerStartupState;
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_config::types::ApprovalsReviewer;
use codex_features::Feature;
use codex_protocol::account::PlanType;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use pretty_assertions::assert_eq;
use std::time::Duration;
use tokio::sync::mpsc::UnboundedReceiver;
async fn install_test_side(app: &mut App) {
@@ -147,3 +158,121 @@ async fn lag_finishes_mcp_startup_in_both_installed_panes() {
"each pane should report both observed and configured MCP servers: {warnings:?}"
);
}
#[tokio::test]
async fn terminal_browser_restricted_open_waits_for_managed_network_confirmation()
-> color_eyre::eyre::Result<()> {
let (mut app, mut app_events, _op_rx) = make_test_app_with_channels().await;
let config_dir = tempfile::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"
approvals_reviewer = "auto_review"
[sandbox_workspace_write]
network_access = true
[features]
terminal_browser = true
"#,
)?;
std::fs::write(
user_config_path.as_path(),
r#"
sandbox_mode = "workspace-write"
approvals_reviewer = "auto_review"
[sandbox_workspace_write]
network_access = false
[features]
terminal_browser = true
"#,
)?;
app.config.codex_home = config_dir.path().to_path_buf().abs();
app.loader_overrides.user_config_path = Some(user_config_path);
app.loader_overrides.system_config_path = Some(system_config_path);
let rebuilt = app
.rebuild_config_for_cwd(app.config.cwd.to_path_buf())
.await?;
app.config = rebuilt.clone();
app.chat_widget
.set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true);
app.chat_widget
.set_approvals_reviewer(ApprovalsReviewer::AutoReview);
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 thread_id = ThreadId::new();
app.chat_widget
.by_slot_mut(PaneSlot::Parent)
.expect("parent pane")
.attach_thread(thread_id, /*receiver*/ None);
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
app.handle_server_request_event(
&app_server,
ServerRequest::DynamicToolCall {
request_id: AppServerRequestId::Integer(42),
params: DynamicToolCallParams {
thread_id: thread_id.to_string(),
turn_id: "turn-1".to_string(),
call_id: "browser-open-1".to_string(),
namespace: Some(TERMINAL_BROWSER_NAMESPACE.to_string()),
tool: "open".to_string(),
arguments: serde_json::json!({
"url": "file:///tmp/not-allowed",
"visible": true,
}),
},
},
)
.await;
assert!(app.has_pending_terminal_browser_open());
let popup = render_bottom_popup(&app.chat_widget, /*width*/ 100);
assert!(popup.contains("Enable managed network access?"), "{popup}");
assert!(app_events.try_recv().is_err());
app.chat_widget
.handle_key_event(KeyEvent::from(KeyCode::Enter));
let (selection, network_choice) = std::iter::from_fn(|| app_events.try_recv().ok())
.find_map(|event| match event {
AppEvent::ApplyAutoReviewPreset {
selection,
network_choice,
} => Some((selection, network_choice)),
_ => None,
})
.expect("approval should resolve the pending browser permission decision");
assert_eq!(network_choice, ManagedNetworkChoice::RestoreManaged);
assert!(
app.apply_auto_review_preset(&mut app_server, selection, network_choice)
.await
);
assert!(app.config.permissions.network_sandbox_policy().is_enabled());
app.resolve_pending_terminal_browser_open(/*allow*/ true)
.await;
let response = tokio::time::timeout(Duration::from_secs(/*secs*/ 1), async {
loop {
if let Some(AppEvent::TerminalBrowserToolCompleted { response, .. }) =
app_events.recv().await
{
break response;
}
}
})
.await?;
assert!(!response.success);
app_server.shutdown().await?;
Ok(())
}
+55 -26
View File
@@ -30,12 +30,54 @@ fn user_network_restriction_overrides_managed_enabled(response: &ConfigReadRespo
}
impl App {
pub(super) fn current_auto_review_selection(&self) -> Option<AutoReviewPresetSelection> {
(self.config.approvals_reviewer == ApprovalsReviewer::AutoReview).then(|| {
AutoReviewPresetSelection {
approval_policy: AskForApproval::from(
self.config.permissions.approval_policy.value(),
),
profile_update: PermissionPresetProfileUpdate::Preserve,
display_label: "Approve for me".to_string(),
}
})
}
pub(super) async fn managed_network_restore_available(
&self,
app_server: &AppServerSession,
) -> bool {
if self
.config
.permissions
.network_sandbox_policy()
.is_enabled()
{
return false;
}
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) => user_network_restriction_overrides_managed_enabled(&response),
Err(err) => {
tracing::warn!(
error = %err,
"failed to inspect managed network config for terminal browser"
);
false
}
}
}
pub(super) async fn apply_auto_review_preset(
&mut self,
app_server: &mut AppServerSession,
selection: AutoReviewPresetSelection,
network_choice: ManagedNetworkChoice,
) {
) -> bool {
let terminal_browser_enabled = self.config.features.enabled(Feature::TerminalBrowser);
let network_is_restricted = !self
.config
@@ -45,25 +87,11 @@ impl App {
if network_choice == ManagedNetworkChoice::Detect
&& terminal_browser_enabled
&& network_is_restricted
&& self.managed_network_restore_available(app_server).await
{
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"
),
}
self.chat_widget
.open_managed_network_restore_confirmation(selection);
return false;
}
let restore_managed_network = network_choice == ManagedNetworkChoice::RestoreManaged;
@@ -74,7 +102,7 @@ impl App {
"Failed to enable Approve for me",
"failed to set Auto Review approval policy",
) {
return;
return false;
}
next_config.approvals_reviewer = ApprovalsReviewer::AutoReview;
@@ -95,7 +123,7 @@ impl App {
"Failed to update Auto Review permissions: {}",
crate::config_update::format_config_error(&err)
));
return;
return false;
}
if restore_managed_network {
@@ -112,7 +140,7 @@ impl App {
self.chat_widget.add_error_message(format!(
"Managed network access was restored on disk, but the current session could not refresh it: {err}"
));
return;
return false;
}
};
if !self.try_set_approval_policy_on_config(
@@ -121,7 +149,7 @@ impl App {
"Failed to enable Approve for me",
"failed to restore Auto Review approval policy",
) {
return;
return false;
}
next_config.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
@@ -136,7 +164,7 @@ impl App {
"The user network override was removed, but managed network access is still disabled."
.to_string(),
);
return;
return false;
}
(
Some(next_config.permissions.permission_profile().clone()),
@@ -168,7 +196,7 @@ impl App {
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;
return false;
}
let permission_network = next_config.permissions.network.clone();
@@ -191,7 +219,7 @@ impl App {
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;
return false;
}
self.chat_widget.set_permission_network(permission_network);
if permission_profile.is_some() {
@@ -246,6 +274,7 @@ impl App {
});
self.chat_widget.add_info_message(message, hint);
self.chat_widget.submit_initial_user_message_if_pending();
true
}
}
+15 -2
View File
@@ -2018,8 +2018,21 @@ impl App {
selection,
network_choice,
} => {
self.apply_auto_review_preset(app_server, selection, network_choice)
.await;
let pending_terminal_browser_open = self.has_pending_terminal_browser_open();
let restore_managed_network =
network_choice == ManagedNetworkChoice::RestoreManaged;
if pending_terminal_browser_open && !restore_managed_network {
self.resolve_pending_terminal_browser_open(/*allow*/ false)
.await;
} else {
let applied = self
.apply_auto_review_preset(app_server, selection, network_choice)
.await;
if pending_terminal_browser_open {
self.resolve_pending_terminal_browser_open(/*allow*/ applied)
.await;
}
}
}
AppEvent::UpdateFeatureFlags { updates } => {
self.update_feature_flags(app_server, updates).await;
+117
View File
@@ -5,12 +5,14 @@
use std::sync::Arc;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_features::Feature;
use codex_protocol::ThreadId;
use codex_terminal_browser::BrowserLaunchContext;
use codex_terminal_browser::BrowserMouseInput;
use codex_terminal_browser::BrowserNetworkPolicy;
use codex_terminal_browser::BrowserStatus;
use codex_terminal_browser::BrowserToolOutput;
use codex_terminal_browser::BrowserView;
use codex_terminal_browser::TerminalBrowser;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -31,8 +33,10 @@ use crate::terminal_browser::TerminalBrowserNetworkAvailability;
use crate::terminal_browser::browser_key_input;
use crate::terminal_browser::browser_mouse_input;
use crate::terminal_browser::browser_mouse_route;
use crate::terminal_browser::dynamic_tool_response;
use crate::terminal_browser::profile_approval_view_params;
use crate::tui::MousePrimaryEvent;
use crate::tui::MousePrimaryEventKind;
pub(super) struct ReopenableTerminalBrowser {
browser: Arc<TerminalBrowser>,
@@ -45,6 +49,12 @@ pub(super) struct TerminalBrowserControlClick {
inputs: [BrowserMouseInput; 2],
}
pub(super) struct PendingTerminalBrowserOpen {
request_id: AppServerRequestId,
session_key: String,
arguments: serde_json::Value,
}
impl ReopenableTerminalBrowser {
pub(super) fn terminate(&self) {
self.browser.terminate();
@@ -52,6 +62,103 @@ impl ReopenableTerminalBrowser {
}
impl App {
pub(super) fn defer_terminal_browser_open(
&mut self,
request_id: AppServerRequestId,
session_key: String,
arguments: serde_json::Value,
) -> bool {
if self.terminal_browser_pending_open.is_some() {
return false;
}
self.terminal_browser_pending_open = Some(PendingTerminalBrowserOpen {
request_id,
session_key,
arguments,
});
true
}
pub(super) fn has_pending_terminal_browser_open(&self) -> bool {
self.terminal_browser_pending_open.is_some()
}
pub(super) fn discard_pending_terminal_browser_open(
&mut self,
request_id: &AppServerRequestId,
) -> bool {
let matches_request = self
.terminal_browser_pending_open
.as_ref()
.is_some_and(|pending| &pending.request_id == request_id);
if matches_request {
self.terminal_browser_pending_open = None;
}
matches_request
}
fn complete_terminal_browser_open(
&self,
request_id: AppServerRequestId,
result: anyhow::Result<BrowserToolOutput>,
) {
self.app_event_tx
.unscoped()
.send(AppEvent::TerminalBrowserToolCompleted {
request_id,
response: dynamic_tool_response(result),
profile_approval: None,
});
}
pub(super) async fn resolve_pending_terminal_browser_open(&mut self, allow: bool) {
let Some(pending) = self.terminal_browser_pending_open.take() else {
return;
};
let PendingTerminalBrowserOpen {
request_id,
session_key,
arguments,
} = pending;
if !allow {
self.complete_terminal_browser_open(
request_id,
Err(anyhow::anyhow!(
"terminal browser network access remains disabled by the active permission profile"
)),
);
return;
}
if !self.terminal_browser_request_matches_active_thread(&session_key) {
self.complete_terminal_browser_open(
request_id,
Err(anyhow::anyhow!(
"terminal browser permission policy only allows the active TUI thread"
)),
);
return;
}
let Some(browser) = self.terminal_browser_for_active_request().await else {
self.complete_terminal_browser_open(
request_id,
Err(anyhow::anyhow!(
"terminal browser permission policy disables this TUI session"
)),
);
return;
};
let app_event_tx = self.app_event_tx.unscoped();
tokio::spawn(async move {
let response =
dynamic_tool_response(browser.execute(&session_key, "open", arguments).await);
app_event_tx.send(AppEvent::TerminalBrowserToolCompleted {
request_id,
response,
profile_approval: None,
});
});
}
fn terminal_browser_network_availability(&self) -> TerminalBrowserNetworkAvailability {
TerminalBrowserNetworkAvailability::from_config_and_runtime(
self.chat_widget.config_ref(),
@@ -210,6 +317,8 @@ impl App {
pub(super) fn terminal_browser_human_control_active(&self) -> bool {
self.has_owned_screen()
&& self.owned_screen_frame.focus()
== super::owned_screen_frame::OwnedScreenFrameFocus::Summary
&& self.owned_screen_frame.right_rail_content() == OwnedScreenRightRailContent::Browser
&& self
.owned_screen_frame
@@ -220,6 +329,14 @@ impl App {
.is_some_and(|browser| browser.is_human_control_active())
}
pub(super) fn terminal_browser_control_click_returns_to_app(
event: MousePrimaryEvent,
viewport: Option<Rect>,
) -> bool {
event.kind == MousePrimaryEventKind::Press
&& viewport.is_none_or(|viewport| !viewport.contains((event.column, event.row).into()))
}
pub(super) fn sync_terminal_browser_panel(&mut self) -> bool {
if !self.has_owned_screen() {
return false;
@@ -16,9 +16,14 @@ use codex_config::LoaderOverrides;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::NetworkSandboxPolicy;
use color_eyre::eyre::Result;
use crossterm::event::KeyModifiers;
use pretty_assertions::assert_eq;
use ratatui::layout::Rect;
use std::time::Duration;
use crate::tui::MousePrimaryEvent;
use crate::tui::MousePrimaryEventKind;
#[test]
fn terminal_browser_requests_require_the_displayed_thread() {
let displayed = ThreadId::new();
@@ -38,6 +43,38 @@ fn terminal_browser_requests_require_the_displayed_thread() {
));
}
#[test]
fn terminal_browser_control_click_outside_viewport_returns_to_app() {
let viewport = Rect::new(
/*x*/ 100, /*y*/ 1, /*width*/ 40, /*height*/ 20,
);
let outside_press = MousePrimaryEvent {
kind: MousePrimaryEventKind::Press,
column: 50,
row: 10,
modifiers: KeyModifiers::NONE,
};
assert!(App::terminal_browser_control_click_returns_to_app(
outside_press,
Some(viewport),
));
assert!(!App::terminal_browser_control_click_returns_to_app(
MousePrimaryEvent {
column: 110,
..outside_press
},
Some(viewport),
));
assert!(!App::terminal_browser_control_click_returns_to_app(
MousePrimaryEvent {
kind: MousePrimaryEventKind::Release,
..outside_press
},
Some(viewport),
));
}
#[tokio::test]
async fn terminal_browser_tools_remain_enabled_without_an_owned_screen() {
let mut app = make_test_app().await;
+1
View File
@@ -49,6 +49,7 @@ pub(super) async fn make_test_app_with_channels() -> (
terminal_browser_owner_thread_id: None,
terminal_browser_generation: 0,
terminal_browser_reopenable: HashMap::new(),
terminal_browser_pending_open: None,
deferred_history_lines: Vec::new(),
has_emitted_history_lines: false,
enhanced_keys_supported: false,
+2
View File
@@ -4615,6 +4615,7 @@ async fn make_test_app() -> App {
terminal_browser_owner_thread_id: None,
terminal_browser_generation: 0,
terminal_browser_reopenable: HashMap::new(),
terminal_browser_pending_open: None,
deferred_history_lines: Vec::new(),
has_emitted_history_lines: false,
enhanced_keys_supported: false,
@@ -4686,6 +4687,7 @@ async fn make_test_app_with_channels() -> (
terminal_browser_owner_thread_id: None,
terminal_browser_generation: 0,
terminal_browser_reopenable: HashMap::new(),
terminal_browser_pending_open: None,
deferred_history_lines: Vec::new(),
has_emitted_history_lines: false,
enhanced_keys_supported: false,
@@ -6,6 +6,8 @@
use super::*;
const MANAGED_NETWORK_RESTORE_VIEW_ID: &str = "managed-network-restore";
impl ChatWidget {
/// Open the permissions popup.
pub(crate) fn open_approvals_popup(&mut self) {
@@ -314,8 +316,10 @@ impl ChatWidget {
selection: AutoReviewPresetSelection,
) {
let restore_selection = selection.clone();
let keep_selection = selection;
let keep_selection = selection.clone();
let cancel_selection = selection;
self.bottom_pane.show_selection_view(SelectionViewParams {
view_id: Some(MANAGED_NETWORK_RESTORE_VIEW_ID),
title: Some("Enable managed network access?".to_string()),
subtitle: Some(
"Your user config disables the managed network required by the terminal browser."
@@ -353,10 +357,21 @@ impl ChatWidget {
..Default::default()
},
],
on_cancel: Some(Box::new(move |tx: &_| {
tx.send(AppEvent::ApplyAutoReviewPreset {
selection: cancel_selection.clone(),
network_choice: ManagedNetworkChoice::KeepRestricted,
});
})),
..Default::default()
});
}
pub(crate) fn dismiss_managed_network_restore_confirmation(&mut self) {
self.bottom_pane
.dismiss_view_by_id(MANAGED_NETWORK_RESTORE_VIEW_ID);
}
pub(super) fn permission_profile_selection_actions(
selection: PermissionProfileSelection,
) -> Vec<SelectionAction> {
@@ -91,7 +91,7 @@ async fn approvals_selection_popup_snapshot() {
#[tokio::test]
async fn terminal_browser_managed_network_restore_confirmation_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.open_managed_network_restore_confirmation(AutoReviewPresetSelection {
approval_policy: AskForApproval::OnRequest,
@@ -103,6 +103,15 @@ async fn terminal_browser_managed_network_restore_confirmation_snapshot() {
"terminal_browser_managed_network_restore_confirmation",
render_bottom_popup(&chat, /*width*/ 100)
);
chat.handle_key_event(KeyEvent::from(KeyCode::Esc));
assert!(matches!(
rx.try_recv(),
Ok(AppEvent::ApplyAutoReviewPreset {
network_choice: ManagedNetworkChoice::KeepRestricted,
..
})
));
}
#[tokio::test]
+1 -1
View File
@@ -109,7 +109,7 @@ test-terminal-browser-panel *args:
test-terminal-browser-routing *args:
just test \
-p codex-tui \
-E 'package(codex-tui) & test(/terminal_browser_(auto_review|managed_network|detects_user_network|open_is_eager)|permissions_auto_review_selection_defers/)' \
-E 'package(codex-tui) & test(/terminal_browser_(auto_review|control_click|managed_network|detects_user_network|open_is_eager|restricted_open)|permissions_auto_review_selection_defers/)' \
"$@"
# Run from the repository root so scripts that resolve paths from `cwd` see