Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,24 @@ struct OllamaStatusFetchStrategy: ProviderFetchStrategy {
} catch {
if self.shouldInvalidateCachedCookie(after: error) {
clearCached(cached)
// The ollama.com session cookie rotates independently of the user's signed-in state, so a
// background refresh can see a cached cookie go stale even when the user never signed out.
// BrowserCookieAccessGate already gates browser reads on its own no-UI preflight — it does
// NOT always deny a background attempt (Safari never needs Keychain decryption, and a
// Chromium browser with a prior "Always Allow" Keychain grant is also read without a
// prompt) — so still attempt `fetchBrowser` here rather than assuming it will fail. Only
// when that attempt itself fails do we surface the original auth error (not a misleading
// "no session cookie" one) instead of finalizing on a user-initiated refresh's explicit
// opt-in, mirroring the `shouldTryBrowserCandidates` recovery below.
return try await self.fetchBrowserOrRethrow(
originalError: error,
fetchBrowser: fetchBrowser,
storeResolved: storeResolved)
} else if self.shouldTryBrowserCandidates(afterCachedFailure: error) {
let cachedError = error
do {
let resolved = try await fetchBrowser()
storeResolved(resolved)
return resolved.snapshot
} catch {
throw cachedError
}
return try await self.fetchBrowserOrRethrow(
originalError: error,
fetchBrowser: fetchBrowser,
storeResolved: storeResolved)
} else {
throw error
}
Expand All @@ -190,6 +199,38 @@ struct OllamaStatusFetchStrategy: ProviderFetchStrategy {
return resolved.snapshot
}

/// Attempts browser-cookie recovery after a cached-cookie failure, surfacing the original failure — not
/// whatever `fetchBrowser` itself throws — if that attempt also fails with a generic, non-actionable error
/// (e.g. no browser candidates found). A specific, actionable browser-access diagnosis (Safari needs Full
/// Disk Access, a Chromium Keychain prompt was declined, etc.) is always more useful than the stale cached
/// auth error it would otherwise be swapped for, so it is re-thrown as-is instead.
private static func fetchBrowserOrRethrow(
originalError: Error,
fetchBrowser: () async throws -> OllamaUsageFetcher.ResolvedCookieFetch,
storeResolved: (OllamaUsageFetcher.ResolvedCookieFetch) -> Void) async throws -> OllamaUsageSnapshot
{
do {
let resolved = try await fetchBrowser()
storeResolved(resolved)
return resolved.snapshot
} catch let browserError where self.isActionableBrowserAccessError(browserError) {
throw browserError
} catch {
throw originalError
}
}

static func isActionableBrowserAccessError(_ error: Error) -> Bool {
switch error {
case OllamaUsageError.safariCookieAccessDenied,
OllamaUsageError.browserCookieDecryptionDenied,
OllamaUsageError.browserCookieDecryptionDisabled:
true
default:
false
}
}

static func shouldInvalidateCachedCookie(after error: Error) -> Bool {
switch error {
case OllamaUsageError.invalidCredentials, OllamaUsageError.notLoggedIn:
Expand Down
199 changes: 183 additions & 16 deletions Tests/CodexBarTests/OllamaUsageFetcherRetryMappingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,33 +139,200 @@ struct OllamaUsageFetcherRetryMappingTests {
}

@Test
func `automatic web fetch replaces cached cookie only after authentication failure`() async throws {
func `automatic web fetch replaces cached cookie only after authentication failure on a user initiated refresh`()
async throws
{
let cached = CookieHeaderCache.Entry(
cookieHeader: "session=expired",
storedAt: Date(timeIntervalSince1970: 100),
sourceLabel: "Chrome")
var events: [String] = []

let snapshot = try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
return OllamaUsageFetcher.ResolvedCookieFetch(
snapshot: Self.makeSnapshot(sessionUsedPercent: 34),
cookieHeader: "session=fresh",
sourceLabel: "Brave")
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { resolved in events.append("store:\(resolved.sourceLabel)") })
let snapshot = try await ProviderInteractionContext.$current.withValue(.userInitiated) {
try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
return OllamaUsageFetcher.ResolvedCookieFetch(
snapshot: Self.makeSnapshot(sessionUsedPercent: 34),
cookieHeader: "session=fresh",
sourceLabel: "Brave")
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { resolved in events.append("store:\(resolved.sourceLabel)") })
}

#expect(snapshot.sessionUsedPercent == 34)
#expect(events == ["cache", "clear:Chrome", "browser", "store:Brave"])
}

@Test
func `automatic web fetch still recovers via the browser on a background auth failure`() async throws {
// The ollama.com session cookie rotates independently of the user's signed-in state, so a background
// refresh can see the cached cookie go stale even though the user never signed out.
// BrowserCookieAccessGate gates the browser read on its own no-UI preflight — it does not always deny
// a background attempt (e.g. Safari never needs Keychain decryption) — so a background auth failure
// must still attempt browser recovery rather than assuming it will fail.
let cached = CookieHeaderCache.Entry(
cookieHeader: "session=expired",
storedAt: Date(timeIntervalSince1970: 100),
sourceLabel: "Chrome")
var events: [String] = []

let snapshot = try await ProviderInteractionContext.$current.withValue(.background) {
try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
return OllamaUsageFetcher.ResolvedCookieFetch(
snapshot: Self.makeSnapshot(sessionUsedPercent: 34),
cookieHeader: "session=fresh",
sourceLabel: "Safari")
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { resolved in events.append("store:\(resolved.sourceLabel)") })
}

#expect(snapshot.sessionUsedPercent == 34)
#expect(events == ["cache", "clear:Chrome", "browser", "store:Safari"])
}

@Test
func `automatic web fetch surfaces the original auth error when background browser recovery also fails`()
async
{
// When BrowserCookieAccessGate does deny the background attempt (e.g. an un-granted Chromium Keychain
// item), fetchBrowser fails too — surface the original, accurate auth error rather than a misleading
// "no session cookie" one from the failed recovery attempt.
let cached = CookieHeaderCache.Entry(
cookieHeader: "session=expired",
storedAt: Date(timeIntervalSince1970: 100),
sourceLabel: "Chrome")
var events: [String] = []

await ProviderInteractionContext.$current.withValue(.background) {
do {
_ = try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
throw OllamaUsageError.noSessionCookie
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { _ in events.append("store") })
Issue.record("Expected the original auth failure to be re-thrown")
} catch OllamaUsageError.invalidCredentials {
// expected — the original cached-cookie auth error, not noSessionCookie from fetchBrowser
} catch {
Issue.record("Expected invalidCredentials, got \(error)")
}
}

#expect(events == ["cache", "clear:Chrome", "browser"])
}

@Test
func `actionable browser access error classification covers every browser diagnosis case`() {
#expect(OllamaStatusFetchStrategy.isActionableBrowserAccessError(
OllamaUsageError.safariCookieAccessDenied))
#expect(OllamaStatusFetchStrategy.isActionableBrowserAccessError(
OllamaUsageError.browserCookieDecryptionDenied("Chrome")))
#expect(OllamaStatusFetchStrategy.isActionableBrowserAccessError(
OllamaUsageError.browserCookieDecryptionDisabled("Chrome")))
#expect(!OllamaStatusFetchStrategy.isActionableBrowserAccessError(
OllamaUsageError.noSessionCookie))
#expect(!OllamaStatusFetchStrategy.isActionableBrowserAccessError(
OllamaUsageError.invalidCredentials))
}

@Test
func `automatic web fetch surfaces an actionable browser access error on a user initiated refresh`() async {
// A user-initiated refresh is the explicit-retry path: if the browser recovery attempt fails with a
// specific, actionable diagnosis (e.g. Safari needs Full Disk Access, or a Chromium Keychain prompt was
// declined), that guidance must reach the user — swapping it for the generic expired-cached-cookie error
// would send them chasing a stale "please sign in again" instead of the real, fixable cause.
let cached = CookieHeaderCache.Entry(
cookieHeader: "session=expired",
storedAt: Date(timeIntervalSince1970: 100),
sourceLabel: "Chrome")
var events: [String] = []

await ProviderInteractionContext.$current.withValue(.userInitiated) {
do {
_ = try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
throw OllamaUsageError.safariCookieAccessDenied
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { _ in events.append("store") })
Issue.record("Expected the actionable browser access error to be re-thrown")
} catch OllamaUsageError.safariCookieAccessDenied {
// expected — the specific, actionable diagnosis, not the generic cached auth error
} catch {
Issue.record("Expected safariCookieAccessDenied, got \(error)")
}
}

#expect(events == ["cache", "clear:Chrome", "browser"])
}

@Test
func `automatic web fetch still recovers the original auth error for a non actionable background failure`()
async
{
// A background refresh's own failed recovery attempt only ever reports the generic "no session cookie"
// shape (BrowserCookieAccessGate denies before an actionable browser-access error could even occur),
// so it should keep surfacing the original, accurate cached-cookie auth error rather than that generic
// one — the existing behavior for the common case must not regress.
let cached = CookieHeaderCache.Entry(
cookieHeader: "session=expired",
storedAt: Date(timeIntervalSince1970: 100),
sourceLabel: "Chrome")
var events: [String] = []

await ProviderInteractionContext.$current.withValue(.userInitiated) {
do {
_ = try await OllamaStatusFetchStrategy.fetchAutomatic(
cached: cached,
fetchCached: { _ in
events.append("cache")
throw OllamaUsageError.invalidCredentials
},
fetchBrowser: {
events.append("browser")
throw OllamaUsageError.noSessionCookie
},
clearCached: { entry in events.append("clear:\(entry.sourceLabel)") },
storeResolved: { _ in events.append("store") })
Issue.record("Expected the original auth failure to be re-thrown")
} catch OllamaUsageError.invalidCredentials {
// expected
} catch {
Issue.record("Expected invalidCredentials, got \(error)")
}
}

#expect(events == ["cache", "clear:Chrome", "browser"])
}

@Test
func `cached cookie invalidation excludes network and parse errors`() {
#expect(OllamaStatusFetchStrategy.shouldInvalidateCachedCookie(
Expand Down