From 82ff2d77a6c4ad32ff314b952a14269c509dfdcf Mon Sep 17 00:00:00 2001 From: Leonardo Araya Date: Sun, 6 Sep 2026 19:58:31 -0300 Subject: [PATCH] fix(ollama): try every browser before giving up on cookie import Auto/Web cookie import stopped at the first installed browser that returned any cookies for ollama.com, even when those cookies were stale/irrelevant (e.g. a consent or analytics cookie left over from a one-off visit) and carried no recognized session cookie. That starved out a later browser (often the one actually signed in) and surfaced "No cookies available for web API" even with a valid, logged-in session sitting on disk. Walk every detected browser and keep going until one yields a header containing a recognized Ollama session cookie, instead of stopping at the first non-empty result. Extracted the selection logic into a small pure helper with a focused regression test reproducing the exact scenario (irrelevant-only cookies on one browser, real session on the next). Fixes #426 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01L23pzyCfMvfbMQwnHXmKCp --- rust/src/providers/ollama/cookies.rs | 95 +++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 10 deletions(-) diff --git a/rust/src/providers/ollama/cookies.rs b/rust/src/providers/ollama/cookies.rs index 880abd4b2a..c99f713867 100644 --- a/rust/src/providers/ollama/cookies.rs +++ b/rust/src/providers/ollama/cookies.rs @@ -153,16 +153,35 @@ pub(super) fn resolve_browser_cookie_header( return Ok(Some(cached.cookie_header)); } - match crate::providers::browser_cookies_for_domain(OLLAMA_COOKIE_DOMAIN) { - Ok(cookies) => { - let url = Url::parse("https://ollama.com/settings") - .map_err(|e| ProviderError::Other(e.to_string()))?; - Ok(ollama_cookie_header_for_url(&cookies, &url) - .filter(|h| has_recognized_ollama_session_cookie(h))) - } - Err(ProviderError::NoCookies) => Ok(None), - Err(err) => Err(err), - } + let url = Url::parse("https://ollama.com/settings") + .map_err(|e| ProviderError::Other(e.to_string()))?; + + // Upstream Win-CodexBar #426: the generic `browser_cookies_for_domain` helper + // stops at the FIRST installed browser that has *any* cookies for the domain, + // even if those cookies are stale/irrelevant (e.g. a consent or CDN cookie left + // behind in Chrome/Edge from a one-off visit) and don't include a recognized + // Ollama session cookie. That starves out a later browser (often Brave) that + // actually holds the logged-in session. Walk every detected browser ourselves + // and keep going until one yields a header with a recognized session cookie. + use crate::browser::cookies::CookieExtractor; + use crate::browser::detection::BrowserDetector; + + let cookie_sets = BrowserDetector::detect_all() + .into_iter() + .filter_map(|browser| { + CookieExtractor::extract_for_domain(&browser, OLLAMA_COOKIE_DOMAIN).ok() + }); + Ok(first_recognized_cookie_header(cookie_sets, &url)) +} + +/// Return the header for the first cookie set (in order) that contains a +/// recognized Ollama session cookie for `url`, skipping sets that decrypt +/// fine but carry no usable session (see `resolve_browser_cookie_header`). +fn first_recognized_cookie_header( + mut cookie_sets: impl Iterator>, + url: &Url, +) -> Option { + cookie_sets.find_map(|cookies| ollama_cookie_header_for_url(&cookies, url)) } pub(super) fn should_attach_ollama_cookie(url: &Url) -> bool { @@ -285,6 +304,62 @@ mod tests { ); } + #[test] + fn first_recognized_cookie_header_skips_browsers_without_a_session_cookie() { + // Regression for #426: an earlier-priority browser (e.g. Chrome/Edge) + // may hold only stale/irrelevant ollama.com cookies (analytics, + // consent) with no session cookie at all. The old + // `browser_cookies_for_domain` helper stopped at that first non-empty + // result and never reached a later browser (e.g. Brave) that actually + // holds the logged-in session. + let irrelevant_only = vec![Cookie { + name: "aid".to_string(), + value: "device-id".to_string(), + domain: "ollama.com".to_string(), + path: "/".to_string(), + expires: None, + is_secure: true, + is_http_only: false, + }]; + let real_session = vec![Cookie { + name: OLLAMA_SESSION_COOKIE_NAME.to_string(), + value: "abc123".to_string(), + domain: "ollama.com".to_string(), + path: "/".to_string(), + expires: None, + is_secure: true, + is_http_only: true, + }]; + + let url = Url::parse("https://ollama.com/settings").unwrap(); + let header = + first_recognized_cookie_header(vec![irrelevant_only, real_session].into_iter(), &url); + + assert_eq!( + header.as_deref(), + Some("__Secure-session=abc123"), + "should skip the first (irrelevant) cookie set and use the second (real session)" + ); + } + + #[test] + fn first_recognized_cookie_header_none_when_no_set_has_a_session_cookie() { + let only_irrelevant = vec![Cookie { + name: "aid".to_string(), + value: "device-id".to_string(), + domain: "ollama.com".to_string(), + path: "/".to_string(), + expires: None, + is_secure: true, + is_http_only: false, + }]; + + let url = Url::parse("https://ollama.com/settings").unwrap(); + let header = first_recognized_cookie_header(vec![only_irrelevant].into_iter(), &url); + + assert_eq!(header, None); + } + #[test] fn ignores_empty_cookie_input() { assert_eq!(normalize_cookie_header(" "), None);