From 826e477768a5ed7879d4b1d406254a05a9b5dffc Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Tue, 7 Apr 2026 22:35:11 +0200 Subject: [PATCH 1/3] feat(oauth): proactive token refresh to avoid re-auth on every session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When connecting to OAuth-protected MCP servers, goose now checks for stored credentials before attempting an unauthenticated connection. If credentials exist, it silently refreshes the token and connects directly, avoiding the unnecessary 401 → browser re-auth cycle that previously happened on every new chat session. Also extracts a connect_with_auth() helper to eliminate duplicated HTTP client + transport construction between the proactive and fallback OAuth paths, and adds diagnostic logging to surface whether servers issue refresh tokens. Signed-off-by: Vincenzo Palazzo --- crates/goose/src/agents/extension_manager.rs | 107 ++++++++++++++----- crates/goose/src/oauth/mod.rs | 50 ++++++++- crates/goose/src/oauth/persist.rs | 8 ++ 3 files changed, 132 insertions(+), 33 deletions(-) diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 6f80e976d167..9458cdf1e365 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -23,7 +23,7 @@ use tokio::process::Command; use tokio::sync::Mutex; use tokio_stream::wrappers::ReceiverStream; use tokio_util::sync::CancellationToken; -use tracing::{error, warn}; +use tracing::{error, info, warn}; use super::container::Container; use super::extension::{ @@ -39,7 +39,7 @@ use crate::builtin_extension::get_builtin_extension; use crate::config::extensions::name_to_key; use crate::config::search_path::SearchPaths; use crate::config::{get_all_extensions, Config}; -use crate::oauth::oauth_flow; +use crate::oauth::{has_stored_credentials, oauth_flow}; use crate::prompt_template; use crate::subprocess::configure_subprocess; use rmcp::model::{ @@ -411,6 +411,43 @@ pub(crate) fn substitute_env_vars(value: &str, env_map: &HashMap const GOOSE_USER_AGENT: reqwest::header::HeaderValue = reqwest::header::HeaderValue::from_static(concat!("goose/", env!("CARGO_PKG_VERSION"))); +#[allow(clippy::too_many_arguments)] +async fn connect_with_auth( + auth_manager: rmcp::transport::AuthorizationManager, + uri: &str, + timeout: Duration, + provider: SharedProvider, + client_name: String, + capabilities: GooseMcpClientCapabilities, + roots_dir: &std::path::Path, +) -> ExtensionResult> { + let mut auth_headers = HeaderMap::new(); + auth_headers.insert(reqwest::header::USER_AGENT, GOOSE_USER_AGENT); + let auth_http_client = reqwest::Client::builder() + .default_headers(auth_headers) + .build() + .map_err(|_| ExtensionError::ConfigError("could not construct http client".to_string()))?; + let auth_client = AuthClient::new(auth_http_client, auth_manager); + let transport = StreamableHttpClientTransport::with_client( + auth_client, + StreamableHttpClientTransportConfig { + uri: uri.into(), + ..Default::default() + }, + ); + Ok(Box::new( + McpClient::connect( + transport, + timeout, + provider, + client_name, + capabilities, + roots_dir.to_path_buf(), + ) + .await?, + )) +} + #[allow(clippy::too_many_arguments)] async fn create_streamable_http_client( uri: &str, @@ -451,6 +488,35 @@ async fn create_streamable_http_client( let timeout_duration = Duration::from_secs(resolve_timeout(timeout)); + // If we have stored OAuth credentials, try refreshing and connecting directly. + // This avoids the unnecessary 401 → browser re-auth cycle on every new session. + if has_stored_credentials(name) { + info!( + "[OAuth:{}] Stored credentials found, attempting proactive token refresh", + name + ); + match oauth_flow(&uri.to_string(), &name.to_string()).await { + Ok(auth_manager) => { + return connect_with_auth( + auth_manager, + uri, + timeout_duration, + provider, + client_name, + capabilities, + roots_dir, + ) + .await; + } + Err(e) => { + warn!( + "[OAuth:{}] Proactive refresh failed: {}, falling back to unauthenticated attempt", + name, e + ); + } + } + } + let client_res = McpClient::connect( transport, timeout_duration, @@ -464,33 +530,16 @@ async fn create_streamable_http_client( if should_attempt_oauth_fallback(&client_res) { match oauth_flow(&uri.to_string(), &name.to_string()).await { Ok(auth_manager) => { - let mut auth_headers = HeaderMap::new(); - auth_headers.insert(reqwest::header::USER_AGENT, GOOSE_USER_AGENT); - let auth_http_client = reqwest::Client::builder() - .default_headers(auth_headers) - .build() - .map_err(|_| { - ExtensionError::ConfigError("could not construct http client".to_string()) - })?; - let auth_client = AuthClient::new(auth_http_client, auth_manager); - let transport = StreamableHttpClientTransport::with_client( - auth_client, - StreamableHttpClientTransportConfig { - uri: uri.into(), - ..Default::default() - }, - ); - Ok(Box::new( - McpClient::connect( - transport, - timeout_duration, - provider, - client_name, - capabilities, - roots_dir.to_path_buf(), - ) - .await?, - )) + connect_with_auth( + auth_manager, + uri, + timeout_duration, + provider, + client_name, + capabilities, + roots_dir, + ) + .await } Err(_) => Ok(Box::new(client_res?)), } diff --git a/crates/goose/src/oauth/mod.rs b/crates/goose/src/oauth/mod.rs index 84efa33c73f4..312cd26aa067 100644 --- a/crates/goose/src/oauth/mod.rs +++ b/crates/goose/src/oauth/mod.rs @@ -1,5 +1,7 @@ mod persist; +pub use persist::has_stored_credentials; + use axum::extract::{Query, State}; use axum::response::Html; use axum::routing::get; @@ -12,7 +14,7 @@ use serde::Deserialize; use std::net::SocketAddr; use std::sync::Arc; use tokio::sync::{oneshot, Mutex}; -use tracing::warn; +use tracing::{debug, info, warn}; use crate::oauth::persist::GooseCredentialStore; @@ -38,13 +40,37 @@ pub async fn oauth_flow( auth_manager.set_credential_store(credential_store.clone()); if auth_manager.initialize_from_store().await? { - if auth_manager.refresh_token().await.is_ok() { - return Ok(auth_manager); + info!( + "[OAuth:{}] Found stored credentials, attempting token refresh", + name + ); + + match auth_manager.refresh_token().await { + Ok(token_response) => { + let has_refresh = token_response.refresh_token().is_some(); + let expires_in = token_response.expires_in(); + info!( + "[OAuth:{}] Token refresh succeeded - has_refresh_token: {}, expires_in: {:?}", + name, has_refresh, expires_in + ); + return Ok(auth_manager); + } + Err(e) => { + warn!( + "[OAuth:{}] Token refresh failed: {} - clearing stored credentials and falling back to browser auth", + name, e + ); + } } if let Err(e) = credential_store.clear().await { - warn!("error clearing bad credentials: {}", e); + warn!("[OAuth:{}] error clearing bad credentials: {}", name, e); } + } else { + info!( + "[OAuth:{}] No stored credentials found, starting browser OAuth flow", + name + ); } // No existing credentials or they were invalid - need to do the full oauth flow @@ -98,6 +124,22 @@ pub async fn oauth_flow( let (client_id, token_response) = oauth_state.get_credentials().await?; + let has_refresh_token = token_response + .as_ref() + .and_then(|tr| tr.refresh_token()) + .is_some(); + let expires_in = token_response.as_ref().and_then(|tr| tr.expires_in()); + let scopes: Vec = token_response + .as_ref() + .and_then(|tr| tr.scopes()) + .map(|s| s.iter().map(|sc| sc.to_string()).collect()) + .unwrap_or_default(); + + debug!( + "[OAuth:{}] Browser auth completed - has_refresh_token: {}, expires_in: {:?}, scopes: {:?}", + name, has_refresh_token, expires_in, scopes + ); + let mut auth_manager = oauth_state .into_authorization_manager() .ok_or_else(|| anyhow::anyhow!("Failed to get authorization manager"))?; diff --git a/crates/goose/src/oauth/persist.rs b/crates/goose/src/oauth/persist.rs index b0c4155ecd02..9f223fdfc41b 100644 --- a/crates/goose/src/oauth/persist.rs +++ b/crates/goose/src/oauth/persist.rs @@ -22,6 +22,14 @@ impl GooseCredentialStore { } } +/// Check if stored OAuth credentials exist for an extension. +/// Used to decide whether to attempt token refresh before connecting. +pub fn has_stored_credentials(name: &str) -> bool { + let config = Config::global(); + let key = format!("oauth_creds_{}", name); + config.get_secret::(&key).is_ok() +} + #[async_trait::async_trait] impl CredentialStore for GooseCredentialStore { async fn load(&self) -> Result, AuthError> { From f44795cd291ccd5b715c660d0e50e71c163e5bc4 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Wed, 8 Apr 2026 18:10:29 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20address=20PR=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20remove=20verbose=20logging=20and=20deduplicate=20cr?= =?UTF-8?q?edential=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove 6 info!/debug! log lines that narrate control flow; keep only the warn! on refresh failure (genuinely useful for production). - Remove has_stored_credentials() which duplicated Config::global() logic; reuse GooseCredentialStore via the CredentialStore trait instead. Addresses review feedback from @DOsinga on #8386. Signed-off-by: Vincenzo Palazzo --- crates/goose/src/agents/extension_manager.rs | 13 +++---- crates/goose/src/oauth/mod.rs | 40 ++------------------ crates/goose/src/oauth/persist.rs | 8 ---- 3 files changed, 8 insertions(+), 53 deletions(-) diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 9458cdf1e365..8a323b5ea995 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -23,7 +23,7 @@ use tokio::process::Command; use tokio::sync::Mutex; use tokio_stream::wrappers::ReceiverStream; use tokio_util::sync::CancellationToken; -use tracing::{error, info, warn}; +use tracing::{error, warn}; use super::container::Container; use super::extension::{ @@ -39,14 +39,14 @@ use crate::builtin_extension::get_builtin_extension; use crate::config::extensions::name_to_key; use crate::config::search_path::SearchPaths; use crate::config::{get_all_extensions, Config}; -use crate::oauth::{has_stored_credentials, oauth_flow}; +use crate::oauth::{oauth_flow, GooseCredentialStore}; use crate::prompt_template; use crate::subprocess::configure_subprocess; use rmcp::model::{ CallToolRequestParams, Content, ErrorCode, ErrorData, GetPromptResult, Prompt, Resource, ResourceContents, ServerInfo, Tool, }; -use rmcp::transport::auth::AuthClient; +use rmcp::transport::auth::{AuthClient, CredentialStore}; use schemars::_private::NoSerialize; use serde_json::Value; @@ -490,11 +490,8 @@ async fn create_streamable_http_client( // If we have stored OAuth credentials, try refreshing and connecting directly. // This avoids the unnecessary 401 → browser re-auth cycle on every new session. - if has_stored_credentials(name) { - info!( - "[OAuth:{}] Stored credentials found, attempting proactive token refresh", - name - ); + let credential_store = GooseCredentialStore::new(name.to_string()); + if credential_store.load().await.is_ok_and(|c| c.is_some()) { match oauth_flow(&uri.to_string(), &name.to_string()).await { Ok(auth_manager) => { return connect_with_auth( diff --git a/crates/goose/src/oauth/mod.rs b/crates/goose/src/oauth/mod.rs index 312cd26aa067..84b088e052e7 100644 --- a/crates/goose/src/oauth/mod.rs +++ b/crates/goose/src/oauth/mod.rs @@ -1,6 +1,6 @@ mod persist; -pub use persist::has_stored_credentials; +pub use persist::GooseCredentialStore; use axum::extract::{Query, State}; use axum::response::Html; @@ -14,9 +14,7 @@ use serde::Deserialize; use std::net::SocketAddr; use std::sync::Arc; use tokio::sync::{oneshot, Mutex}; -use tracing::{debug, info, warn}; - -use crate::oauth::persist::GooseCredentialStore; +use tracing::warn; const CALLBACK_TEMPLATE: &str = include_str!("oauth_callback.html"); @@ -40,19 +38,8 @@ pub async fn oauth_flow( auth_manager.set_credential_store(credential_store.clone()); if auth_manager.initialize_from_store().await? { - info!( - "[OAuth:{}] Found stored credentials, attempting token refresh", - name - ); - match auth_manager.refresh_token().await { - Ok(token_response) => { - let has_refresh = token_response.refresh_token().is_some(); - let expires_in = token_response.expires_in(); - info!( - "[OAuth:{}] Token refresh succeeded - has_refresh_token: {}, expires_in: {:?}", - name, has_refresh, expires_in - ); + Ok(_) => { return Ok(auth_manager); } Err(e) => { @@ -66,11 +53,6 @@ pub async fn oauth_flow( if let Err(e) = credential_store.clear().await { warn!("[OAuth:{}] error clearing bad credentials: {}", name, e); } - } else { - info!( - "[OAuth:{}] No stored credentials found, starting browser OAuth flow", - name - ); } // No existing credentials or they were invalid - need to do the full oauth flow @@ -124,22 +106,6 @@ pub async fn oauth_flow( let (client_id, token_response) = oauth_state.get_credentials().await?; - let has_refresh_token = token_response - .as_ref() - .and_then(|tr| tr.refresh_token()) - .is_some(); - let expires_in = token_response.as_ref().and_then(|tr| tr.expires_in()); - let scopes: Vec = token_response - .as_ref() - .and_then(|tr| tr.scopes()) - .map(|s| s.iter().map(|sc| sc.to_string()).collect()) - .unwrap_or_default(); - - debug!( - "[OAuth:{}] Browser auth completed - has_refresh_token: {}, expires_in: {:?}, scopes: {:?}", - name, has_refresh_token, expires_in, scopes - ); - let mut auth_manager = oauth_state .into_authorization_manager() .ok_or_else(|| anyhow::anyhow!("Failed to get authorization manager"))?; diff --git a/crates/goose/src/oauth/persist.rs b/crates/goose/src/oauth/persist.rs index 9f223fdfc41b..b0c4155ecd02 100644 --- a/crates/goose/src/oauth/persist.rs +++ b/crates/goose/src/oauth/persist.rs @@ -22,14 +22,6 @@ impl GooseCredentialStore { } } -/// Check if stored OAuth credentials exist for an extension. -/// Used to decide whether to attempt token refresh before connecting. -pub fn has_stored_credentials(name: &str) -> bool { - let config = Config::global(); - let key = format!("oauth_creds_{}", name); - config.get_secret::(&key).is_ok() -} - #[async_trait::async_trait] impl CredentialStore for GooseCredentialStore { async fn load(&self) -> Result, AuthError> { From 9d5a2491bf15420aa62e5eb35895908d34876141 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Thu, 9 Apr 2026 19:28:41 +0200 Subject: [PATCH 3/3] Add chat history search feature to navigation panel Implement a search bar in the Goose navigation panel that allows users to search through their chat history by keywords. Features: - Search input with debounced API calls to /sessions/search endpoint - Displays search results in a dropdown with session names and message counts - Keyboard shortcut support (Cmd/Ctrl+K to focus, Escape to close) - Shows session status indicators (streaming, unread, error) - Integrates with existing navigation panel in both expanded and condensed modes - Click to navigate to any search result Resolves #8440 Signed-off-by: Vincenzo Palazzo --- .../components/Layout/CondensedRenderer.tsx | 16 ++ .../components/Layout/ExpandedRenderer.tsx | 11 + .../conversation/ChatHistorySearch.tsx | 243 ++++++++++++++++++ 3 files changed, 270 insertions(+) create mode 100644 ui/desktop/src/components/conversation/ChatHistorySearch.tsx diff --git a/ui/desktop/src/components/Layout/CondensedRenderer.tsx b/ui/desktop/src/components/Layout/CondensedRenderer.tsx index 20d0f45110fe..309792b22efa 100644 --- a/ui/desktop/src/components/Layout/CondensedRenderer.tsx +++ b/ui/desktop/src/components/Layout/CondensedRenderer.tsx @@ -5,6 +5,7 @@ import { defineMessages, useIntl } from '../../i18n'; import { cn } from '../../utils'; import { DropdownMenu, DropdownMenuTrigger } from '../ui/dropdown-menu'; import { ChatSessionsDropdown, SessionsList } from './navigation'; +import { ChatHistorySearch } from '../conversation/ChatHistorySearch'; import type { NavigationRendererProps } from './navigation/types'; const i18n = defineMessages({ @@ -76,6 +77,21 @@ export const CondensedRenderer: React.FC = ({
)} + {/* Search bar */} +
+ +
+ {/* Navigation items */} {isVertical ? (
diff --git a/ui/desktop/src/components/Layout/ExpandedRenderer.tsx b/ui/desktop/src/components/Layout/ExpandedRenderer.tsx index a379273632a2..e678db182ebf 100644 --- a/ui/desktop/src/components/Layout/ExpandedRenderer.tsx +++ b/ui/desktop/src/components/Layout/ExpandedRenderer.tsx @@ -5,6 +5,7 @@ import { Z_INDEX } from './constants'; import { cn } from '../../utils'; import { DropdownMenu, DropdownMenuTrigger } from '../ui/dropdown-menu'; import { ChatSessionsDropdown } from './navigation'; +import { ChatHistorySearch } from '../conversation/ChatHistorySearch'; import type { NavigationRendererProps } from './navigation/types'; export const ExpandedRenderer: React.FC = ({ @@ -141,6 +142,16 @@ export const ExpandedRenderer: React.FC = ({ alignContent: 'start', }} > + {/* Search bar - spans full width */} +
+ +
+ {visibleItems.map((item, index) => { const Icon = item.icon; const active = isActive(item.path); diff --git a/ui/desktop/src/components/conversation/ChatHistorySearch.tsx b/ui/desktop/src/components/conversation/ChatHistorySearch.tsx new file mode 100644 index 000000000000..d2151e344ce5 --- /dev/null +++ b/ui/desktop/src/components/conversation/ChatHistorySearch.tsx @@ -0,0 +1,243 @@ +import React, { useState, useCallback, useEffect, useRef } from 'react'; +import { Search, X, Loader2 } from 'lucide-react'; +import { defineMessages, useIntl } from '../../i18n'; +import { searchSessions } from '../../api'; +import { cn } from '../../utils'; +import { SessionIndicators } from '../SessionIndicators'; +import type { Session } from '../../api'; +import type { SessionStatus } from '../Layout/navigation/types'; + +const i18n = defineMessages({ + searchPlaceholder: { + id: 'chatHistorySearch.searchPlaceholder', + defaultMessage: 'Search chat history...', + }, + noResults: { + id: 'chatHistorySearch.noResults', + defaultMessage: 'No results found', + }, + searching: { + id: 'chatHistorySearch.searching', + defaultMessage: 'Searching...', + }, +}); + +interface ChatHistorySearchProps { + onSessionClick: (sessionId: string) => void; + getSessionStatus: (sessionId: string) => SessionStatus | undefined; + clearUnread: (sessionId: string) => void; + activeSessionId?: string; + className?: string; +} + +export const ChatHistorySearch: React.FC = ({ + onSessionClick, + getSessionStatus, + clearUnread, + activeSessionId, + className, +}) => { + const intl = useIntl(); + const [query, setQuery] = useState(''); + const [results, setResults] = useState([]); + const [isSearching, setIsSearching] = useState(false); + const [showResults, setShowResults] = useState(false); + const searchRef = useRef(null); + const inputRef = useRef(null); + const searchTimeoutRef = useRef | undefined>(undefined); + + const performSearch = useCallback(async (searchQuery: string) => { + if (!searchQuery.trim()) { + setResults([]); + setShowResults(false); + return; + } + + setIsSearching(true); + try { + const response = await searchSessions({ + query: { query: searchQuery, limit: 10 }, + throwOnError: false, + client: undefined, + }); + + if (response.data) { + setResults(response.data); + setShowResults(true); + } + } catch (error) { + console.error('Search failed:', error); + setResults([]); + } finally { + setIsSearching(false); + } + }, []); + + // Debounced search + useEffect(() => { + if (searchTimeoutRef.current) { + clearTimeout(searchTimeoutRef.current); + } + + if (query.trim()) { + searchTimeoutRef.current = setTimeout(() => { + performSearch(query); + }, 300); + } else { + setResults([]); + setShowResults(false); + } + + return () => { + if (searchTimeoutRef.current) { + clearTimeout(searchTimeoutRef.current); + } + }; + }, [query, performSearch]); + + const handleClear = useCallback(() => { + setQuery(''); + setResults([]); + setShowResults(false); + inputRef.current?.focus(); + }, []); + + const handleResultClick = useCallback( + (sessionId: string) => { + clearUnread(sessionId); + onSessionClick(sessionId); + setShowResults(false); + setQuery(''); + }, + [onSessionClick, clearUnread] + ); + + // Close results when clicking outside + useEffect(() => { + const handleClickOutside = (event: MouseEvent) => { + if (searchRef.current && !searchRef.current.contains(event.target as Node)) { + setShowResults(false); + } + }; + + document.addEventListener('mousedown', handleClickOutside); + return () => document.removeEventListener('mousedown', handleClickOutside); + }, []); + + // Focus input on mount and when search opens + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + // Cmd/Ctrl + K to focus search + if ((e.metaKey || e.ctrlKey) && e.key === 'k') { + e.preventDefault(); + inputRef.current?.focus(); + } + // Escape to clear and close + if (e.key === 'Escape' && showResults) { + e.preventDefault(); + setShowResults(false); + if (!query) { + inputRef.current?.blur(); + } + } + }; + + document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [showResults, query]); + + return ( +
+ {/* Search Input */} +
+ + setQuery(e.target.value)} + onFocus={() => { + if (results.length > 0) setShowResults(true); + }} + placeholder={intl.formatMessage(i18n.searchPlaceholder)} + className={cn( + 'w-full pl-10 pr-10 py-2 text-sm', + 'bg-background-secondary border border-border-primary rounded-lg', + 'text-text-primary placeholder-text-secondary', + 'focus:outline-none focus:ring-2 focus:ring-border-tertiary', + 'transition-all duration-200' + )} + /> + {query && ( + + )} +
+ + {/* Search Results Dropdown */} + {showResults && ( +
+ {isSearching ? ( +
+ + {intl.formatMessage(i18n.searching)} +
+ ) : results.length > 0 ? ( +
+ {results.map((session) => { + const status = getSessionStatus(session.id); + const isStreaming = status?.streamState === 'streaming'; + const hasError = status?.streamState === 'error'; + const hasUnread = status?.hasUnreadActivity ?? false; + const isActiveSession = session.id === activeSessionId; + + return ( + + ); + })} +
+ ) : query.trim() ? ( +
+ {intl.formatMessage(i18n.noResults)} +
+ ) : null} +
+ )} +
+ ); +};