diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8b13175a6..92716cc41 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2297,6 +2297,7 @@ dependencies = [ "url", "uuid", "vt100", + "webbrowser", "which", "windows-sys 0.52.0", "winsplit", diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 137b2c005..b451d21b5 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -10,6 +10,7 @@ use async_channel::unbounded; pub use codex_app_server_protocol::AppInfo; use codex_protocol::protocol::SandboxPolicy; use tokio_util::sync::CancellationToken; +use tracing::warn; use crate::AuthManager; use crate::CodexAuth; @@ -96,6 +97,16 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( ) .await; + if force_refetch + && let Err(err) = mcp_connection_manager + .hard_refresh_codex_apps_tools_cache() + .await + { + warn!( + "failed to force-refresh tools for MCP server '{CODEX_APPS_MCP_SERVER_NAME}', using cached/startup tools: {err:#}" + ); + } + let codex_apps_ready = if let Some(cfg) = mcp_servers.get(CODEX_APPS_MCP_SERVER_NAME) { let timeout = cfg.startup_timeout_sec.unwrap_or(DEFAULT_STARTUP_TIMEOUT); mcp_connection_manager diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 4a6357162..6f6bfad6e 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -531,6 +531,33 @@ impl McpConnectionManager { tools } + /// Force-refresh codex apps tools by bypassing the in-process cache. + /// + /// On success, the refreshed tools replace the cache contents. On failure, + /// the existing cache remains unchanged. + pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result<()> { + let managed_client = self + .clients + .get(CODEX_APPS_MCP_SERVER_NAME) + .ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))? + .client() + .await + .context("failed to get client")?; + + let tools = list_tools_for_client_uncached( + CODEX_APPS_MCP_SERVER_NAME, + &managed_client.client, + managed_client.tool_timeout, + ) + .await + .with_context(|| { + format!("failed to refresh tools for MCP server '{CODEX_APPS_MCP_SERVER_NAME}'") + })?; + + write_cached_codex_apps_tools(&tools); + Ok(()) + } + /// Returns a single map that contains all resources. Each key is the /// server name and the value is a vector of resources. pub async fn list_all_resources(&self) -> HashMap> { diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 3252912ba..1dced39b7 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -95,6 +95,7 @@ tree-sitter-highlight = { workspace = true } unicode-segmentation = { workspace = true } unicode-width = { workspace = true } url = { workspace = true } +webbrowser = { workspace = true } uuid = { workspace = true } codex-windows-sandbox = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d18ec2354..b338a2462 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -645,6 +645,17 @@ impl App { } } + fn open_url_in_browser(&mut self, url: String) { + if let Err(err) = webbrowser::open(&url) { + self.chat_widget + .add_error_message(format!("Failed to open browser for {url}: {err}")); + return; + } + + self.chat_widget + .add_info_message(format!("Opened {url} in your browser."), None); + } + async fn shutdown_current_thread(&mut self) { if let Some(thread_id) = self.chat_widget.thread_id() { // Clear any in-flight rollback guard when switching threads. @@ -1601,6 +1612,12 @@ impl App { is_installed, ); } + AppEvent::OpenUrlInBrowser { url } => { + self.open_url_in_browser(url); + } + AppEvent::RefreshConnectors { force_refetch } => { + self.chat_widget.refresh_connectors(force_refetch); + } AppEvent::StartFileSearch(query) => { self.file_search.on_user_query(query); } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index b48960bbf..0f456f457 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -114,6 +114,16 @@ pub(crate) enum AppEvent { is_installed: bool, }, + /// Open the provided URL in the user's browser. + OpenUrlInBrowser { + url: String, + }, + + /// Refresh app connector state and mention bindings. + RefreshConnectors { + force_refetch: bool, + }, + InsertHistoryCell(Box), StartCommitAnimation, diff --git a/codex-rs/tui/src/bottom_pane/app_link_view.rs b/codex-rs/tui/src/bottom_pane/app_link_view.rs index 1f672607a..05ef32bfa 100644 --- a/codex-rs/tui/src/bottom_pane/app_link_view.rs +++ b/codex-rs/tui/src/bottom_pane/app_link_view.rs @@ -1,5 +1,6 @@ use crossterm::event::KeyCode; use crossterm::event::KeyEvent; +use crossterm::event::KeyModifiers; use ratatui::buffer::Buffer; use ratatui::layout::Constraint; use ratatui::layout::Layout; @@ -13,18 +14,33 @@ use textwrap::wrap; use super::CancellationEvent; use super::bottom_pane_view::BottomPaneView; +use super::scroll_state::ScrollState; +use super::selection_popup_common::GenericDisplayRow; +use super::selection_popup_common::measure_rows_height; +use super::selection_popup_common::render_rows; +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; use crate::key_hint; use crate::render::Insets; use crate::render::RectExt as _; use crate::style::user_message_style; use crate::wrapping::word_wrap_lines; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum AppLinkScreen { + Link, + InstallConfirmation, +} + pub(crate) struct AppLinkView { title: String, description: Option, instructions: String, url: String, is_installed: bool, + app_event_tx: AppEventSender, + screen: AppLinkScreen, + selected_action: usize, complete: bool, } @@ -35,6 +51,7 @@ impl AppLinkView { instructions: String, url: String, is_installed: bool, + app_event_tx: AppEventSender, ) -> Self { Self { title, @@ -42,11 +59,81 @@ impl AppLinkView { instructions, url, is_installed, + app_event_tx, + screen: AppLinkScreen::Link, + selected_action: 0, complete: false, } } + fn action_labels(&self) -> [&'static str; 2] { + match self.screen { + AppLinkScreen::Link => { + if self.is_installed { + ["Manage on ChatGPT", "Back"] + } else { + ["Install on ChatGPT", "Back"] + } + } + AppLinkScreen::InstallConfirmation => ["I already Installed it", "Back"], + } + } + + fn move_selection_prev(&mut self) { + self.selected_action = self.selected_action.saturating_sub(1); + } + + fn move_selection_next(&mut self) { + self.selected_action = (self.selected_action + 1).min(self.action_labels().len() - 1); + } + + fn handle_primary_action(&mut self) { + match self.screen { + AppLinkScreen::Link => { + self.app_event_tx.send(AppEvent::OpenUrlInBrowser { + url: self.url.clone(), + }); + if !self.is_installed { + self.screen = AppLinkScreen::InstallConfirmation; + self.selected_action = 0; + } + } + AppLinkScreen::InstallConfirmation => { + self.app_event_tx.send(AppEvent::RefreshConnectors { + force_refetch: true, + }); + self.complete = true; + } + } + } + + fn handle_secondary_action(&mut self) { + match self.screen { + AppLinkScreen::Link => { + self.complete = true; + } + AppLinkScreen::InstallConfirmation => { + self.screen = AppLinkScreen::Link; + self.selected_action = 0; + } + } + } + + fn activate_selected_action(&mut self) { + match self.selected_action { + 0 => self.handle_primary_action(), + _ => self.handle_secondary_action(), + } + } + fn content_lines(&self, width: u16) -> Vec> { + match self.screen { + AppLinkScreen::Link => self.link_content_lines(width), + AppLinkScreen::InstallConfirmation => self.install_confirmation_lines(width), + } + } + + fn link_content_lines(&self, width: u16) -> Vec> { let usable_width = width.max(1) as usize; let mut lines: Vec> = Vec::new(); @@ -61,6 +148,7 @@ impl AppLinkView { lines.push(Line::from(line.into_owned().dim())); } } + lines.push(Line::from("")); if self.is_installed { for line in wrap("Use $ to insert this app into the prompt.", usable_width) { @@ -91,21 +179,156 @@ impl AppLinkView { lines.push(Line::from("")); } - lines.push(Line::from(vec!["Open:".dim()])); + lines + } + + fn install_confirmation_lines(&self, width: u16) -> Vec> { + let usable_width = width.max(1) as usize; + let mut lines: Vec> = Vec::new(); + + lines.push(Line::from("Finish App Setup".bold())); + lines.push(Line::from("")); + + for line in wrap( + "Complete app setup on ChatGPT in the browser window that just opened.", + usable_width, + ) { + lines.push(Line::from(line.into_owned())); + } + for line in wrap( + "Sign in there if needed, then return here and select \"I already Installed it\".", + usable_width, + ) { + lines.push(Line::from(line.into_owned())); + } + + lines.push(Line::from("")); + lines.push(Line::from(vec!["Setup URL:".dim()])); let url_line = Line::from(vec![self.url.clone().cyan().underlined()]); lines.extend(word_wrap_lines(vec![url_line], usable_width)); lines } + + fn action_rows(&self) -> Vec { + self.action_labels() + .into_iter() + .enumerate() + .map(|(index, label)| { + let prefix = if self.selected_action == index { + '›' + } else { + ' ' + }; + GenericDisplayRow { + name: format!("{prefix} {}. {label}", index + 1), + ..Default::default() + } + }) + .collect() + } + + fn action_state(&self) -> ScrollState { + let mut state = ScrollState::new(); + state.selected_idx = Some(self.selected_action); + state + } + + fn action_rows_height(&self, width: u16) -> u16 { + let rows = self.action_rows(); + let state = self.action_state(); + measure_rows_height(&rows, &state, rows.len().max(1), width.max(1)) + } + + fn hint_line(&self) -> Line<'static> { + Line::from(vec![ + "Use ".into(), + key_hint::plain(KeyCode::Tab).into(), + " / ".into(), + key_hint::plain(KeyCode::Up).into(), + " ".into(), + key_hint::plain(KeyCode::Down).into(), + " to move, ".into(), + key_hint::plain(KeyCode::Enter).into(), + " to select, ".into(), + key_hint::plain(KeyCode::Esc).into(), + " to close".into(), + ]) + } } impl BottomPaneView for AppLinkView { fn handle_key_event(&mut self, key_event: KeyEvent) { - if let KeyEvent { - code: KeyCode::Esc, .. - } = key_event - { - self.on_ctrl_c(); + match key_event { + KeyEvent { + code: KeyCode::Esc, .. + } => { + self.on_ctrl_c(); + } + KeyEvent { + code: KeyCode::Up, .. + } + | KeyEvent { + code: KeyCode::Left, + .. + } + | KeyEvent { + code: KeyCode::BackTab, + .. + } + | KeyEvent { + code: KeyCode::Char('k'), + modifiers: KeyModifiers::NONE, + .. + } + | KeyEvent { + code: KeyCode::Char('h'), + modifiers: KeyModifiers::NONE, + .. + } => self.move_selection_prev(), + KeyEvent { + code: KeyCode::Down, + .. + } + | KeyEvent { + code: KeyCode::Right, + .. + } + | KeyEvent { + code: KeyCode::Tab, .. + } + | KeyEvent { + code: KeyCode::Char('j'), + modifiers: KeyModifiers::NONE, + .. + } + | KeyEvent { + code: KeyCode::Char('l'), + modifiers: KeyModifiers::NONE, + .. + } => self.move_selection_next(), + KeyEvent { + code: KeyCode::Char('1'), + modifiers: KeyModifiers::NONE, + .. + } => { + self.selected_action = 0; + self.activate_selected_action(); + } + KeyEvent { + code: KeyCode::Char('2'), + modifiers: KeyModifiers::NONE, + .. + } => { + self.selected_action = 1; + self.activate_selected_action(); + } + KeyEvent { + code: KeyCode::Enter, + modifiers: KeyModifiers::NONE, + .. + } => self.activate_selected_action(), + _ => {} } } @@ -123,7 +346,8 @@ impl crate::render::renderable::Renderable for AppLinkView { fn desired_height(&self, width: u16) -> u16 { let content_width = width.saturating_sub(4).max(1); let content_lines = self.content_lines(content_width); - content_lines.len() as u16 + 3 + let action_rows_height = self.action_rows_height(content_width); + content_lines.len() as u16 + action_rows_height + 3 } fn render(&self, area: Rect, buf: &mut Buffer) { @@ -135,13 +359,38 @@ impl crate::render::renderable::Renderable for AppLinkView { .style(user_message_style()) .render(area, buf); - let [content_area, hint_area] = - Layout::vertical([Constraint::Fill(1), Constraint::Length(1)]).areas(area); + let actions_height = self.action_rows_height(area.width.saturating_sub(4)); + let [content_area, actions_area, hint_area] = Layout::vertical([ + Constraint::Fill(1), + Constraint::Length(actions_height), + Constraint::Length(1), + ]) + .areas(area); + let inner = content_area.inset(Insets::vh(1, 2)); let content_width = inner.width.max(1); let lines = self.content_lines(content_width); Paragraph::new(lines).render(inner, buf); + if actions_area.height > 0 { + let actions_area = Rect { + x: actions_area.x.saturating_add(2), + y: actions_area.y, + width: actions_area.width.saturating_sub(2), + height: actions_area.height, + }; + let action_rows = self.action_rows(); + let action_state = self.action_state(); + render_rows( + actions_area, + buf, + &action_rows, + &action_state, + action_rows.len().max(1), + "No actions", + ); + } + if hint_area.height > 0 { let hint_area = Rect { x: hint_area.x.saturating_add(2), @@ -149,15 +398,7 @@ impl crate::render::renderable::Renderable for AppLinkView { width: hint_area.width.saturating_sub(2), height: hint_area.height, }; - hint_line().dim().render(hint_area, buf); + self.hint_line().dim().render(hint_area, buf); } } } - -fn hint_line() -> Line<'static> { - Line::from(vec![ - "Press ".into(), - key_hint::plain(KeyCode::Esc).into(), - " to close".into(), - ]) -} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 97f6a2831..e507a39a4 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1161,6 +1161,7 @@ impl ChatWidget { instructions, url, is_installed, + self.app_event_tx.clone(), ); self.bottom_pane.show_view(Box::new(view)); self.request_redraw(); @@ -4491,7 +4492,15 @@ impl ChatWidget { } } + pub(crate) fn refresh_connectors(&mut self, force_refetch: bool) { + self.prefetch_connectors_with_options(force_refetch); + } + fn prefetch_connectors(&mut self) { + self.prefetch_connectors_with_options(false); + } + + fn prefetch_connectors_with_options(&mut self, force_refetch: bool) { if !self.connectors_enabled() || self.connectors_prefetch_in_flight { return; } @@ -4505,7 +4514,12 @@ impl ChatWidget { let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { let accessible_connectors = - match connectors::list_accessible_connectors_from_mcp_tools(&config).await { + match connectors::list_accessible_connectors_from_mcp_tools_with_options( + &config, + force_refetch, + ) + .await + { Ok(connectors) => connectors, Err(err) => { app_event_tx.send(AppEvent::ConnectorsLoaded { @@ -6360,7 +6374,10 @@ impl ChatWidget { return; } - match self.connectors_cache.clone() { + let connectors_cache = self.connectors_cache.clone(); + self.prefetch_connectors_with_options(true); + + match connectors_cache { ConnectorsCacheState::Ready(snapshot) => { if snapshot.connectors.is_empty() { self.add_info_message("No apps available.".to_string(), None); @@ -6370,8 +6387,6 @@ impl ChatWidget { } ConnectorsCacheState::Failed(err) => { self.add_to_history(history_cell::new_error_event(err)); - // Retry on demand so `/apps` can recover after transient failures. - self.prefetch_connectors(); } ConnectorsCacheState::Loading => { self.add_to_history(history_cell::new_info_event( @@ -6380,7 +6395,6 @@ impl ChatWidget { )); } ConnectorsCacheState::Uninitialized => { - self.prefetch_connectors(); self.add_to_history(history_cell::new_info_event( "Apps are still loading.".to_string(), Some("Try again in a moment.".to_string()), @@ -6730,7 +6744,9 @@ impl ChatWidget { match result { Ok(snapshot) => { self.refresh_connectors_popup_if_open(&snapshot.connectors); - self.connectors_cache = ConnectorsCacheState::Ready(snapshot.clone()); + if is_final || !matches!(self.connectors_cache, ConnectorsCacheState::Ready(_)) { + self.connectors_cache = ConnectorsCacheState::Ready(snapshot.clone()); + } self.bottom_pane.set_connectors_snapshot(Some(snapshot)); } Err(err) => { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 5b6e35bff..1f1af74a1 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -3715,6 +3715,10 @@ async fn apps_popup_refreshes_when_connectors_snapshot_updates() { false, ); chat.add_connectors_output(); + assert!( + chat.connectors_prefetch_in_flight, + "expected /apps to trigger a forced connectors refresh" + ); let before = render_bottom_popup(&chat, 80); assert!( @@ -3761,6 +3765,71 @@ async fn apps_popup_refreshes_when_connectors_snapshot_updates() { ); } +#[tokio::test] +async fn apps_refresh_failure_keeps_existing_full_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.features.enable(Feature::Apps); + chat.bottom_pane.set_connectors_enabled(true); + + let full_connectors = vec![ + codex_chatgpt::connectors::AppInfo { + id: "connector_1".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + }, + codex_chatgpt::connectors::AppInfo { + id: "connector_2".to_string(), + name: "Linear".to_string(), + description: Some("Project tracking".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some("https://example.test/linear".to_string()), + is_accessible: false, + }, + ]; + chat.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: full_connectors.clone(), + }), + true, + ); + + chat.on_connectors_loaded( + Ok(ConnectorsSnapshot { + connectors: vec![codex_chatgpt::connectors::AppInfo { + id: "connector_1".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + }], + }), + false, + ); + chat.on_connectors_loaded(Err("failed to load apps".to_string()), true); + + assert_matches!( + &chat.connectors_cache, + ConnectorsCacheState::Ready(snapshot) if snapshot.connectors == full_connectors + ); + + chat.add_connectors_output(); + let popup = render_bottom_popup(&chat, 80); + assert!( + popup.contains("Installed 1 of 2 available apps."), + "expected previous full snapshot to be preserved, got:\n{popup}" + ); +} + #[tokio::test] async fn experimental_features_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;