fix(auth): auto-refresh expired Anthropic OAuth tokens#3510
fix(auth): auto-refresh expired Anthropic OAuth tokens#3510
Conversation
Anthropic credentials were read via authStorage.get() everywhere, so mastracode's built-in refresh flow never ran. Once the 1-hour access token expired, status flipped to "Reconnect" and users had to do a full PKCE re-auth, even though a valid refresh token was already stored. Resolvers now call authStorage.getApiKey() for oauth creds on expiry, which triggers refreshToken() and persists the refreshed credential. getAnthropicAuthStatus does the same before declaring issue: "expired". Mirrors the pattern already used for OpenAI small-model auth.
📝 WalkthroughWalkthroughMultiple credential-resolution functions are converted from synchronous to asynchronous operations across the codebase. New OAuth token refresh logic detects expired credentials and attempts to refresh them via auth storage. The small model provider interface is updated to accommodate both sync and async credential resolvers. Changes
Sequence DiagramsequenceDiagram
participant Client as Call Small Model
participant Provider as Small Model Provider
participant Auth as Auth Storage
participant Validator as Credential Validator
Client->>Provider: resolveCredentials() [async]
activate Provider
Provider->>Auth: getApiKey() if expired
activate Auth
Auth-->>Provider: refreshed token (or null)
deactivate Auth
Provider->>Auth: reload storage
activate Auth
Auth-->>Provider: updated credential
deactivate Auth
Provider->>Validator: validate expiresAt, isSupported
activate Validator
Validator-->>Provider: validation result
deactivate Validator
Provider-->>Client: SmallModelCredential | null
deactivate Provider
Client->>Client: create model & invoke
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a UX regression where expired Anthropic OAuth tokens always prompted a full PKCE re-auth, even though a valid refresh token was available. The fix routes expired OAuth credential reads through mastracode's The pattern is applied consistently across three resolvers ( Key observations:
Confidence Score: 4/5Safe to merge; the fix is correct and well-scoped, but one concrete improvement remains before the refresh path is properly tested. The approach is sound and mirrors the existing OpenAI refresh pattern. The
|
| Filename | Overview |
|---|---|
| packages/chat/src/server/desktop/auth/anthropic/anthropic.ts | Core auth resolver made async; getCredentialsFromAuthStorage now calls getApiKey for all OAuth credentials (not just expired), inconsistent with the expiry-guard pattern used in the other two changed files. |
| packages/chat/src/server/desktop/chat-service/chat-service.ts | getAnthropicAuthStatus correctly guards the getApiKey refresh call behind an expiry check before downgrading to issue: "expired", then reloads storage to pick up the refreshed credential. |
| packages/chat/src/server/desktop/chat-service/chat-service.test.ts | FakeAuthStorage mock lacks getApiKey; the new refresh path is swallowed by catch {} in tests rather than properly exercised, leaving the PR's primary behaviour untested. |
| packages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts | Correctly guards getApiKey behind an expiry check; on refresh failure, returns the expired credential so hasUsableCredential returns false and the caller falls back gracefully. |
| packages/chat/src/server/desktop/small-model/small-model.ts | Type widened to allow Promise from resolveCredentials; no logic changes, purely a type-level update to accommodate the now-async Anthropic resolver. |
| apps/desktop/src/lib/ai/call-small-model.ts | Adds await to provider.resolveCredentials() to handle the now-potentially-async return; straightforward and correct. |
| packages/host-service/src/providers/model-providers/LocalModelProvider/LocalModelProvider.ts | resolveRuntimeEnv already awaited resolveAnthropicCredential; this file cascades naturally from the async change in resolveAnthropicCredential.ts with no logic changes. |
Sequence Diagram
sequenceDiagram
participant App as Desktop App
participant CS as ChatService / Resolver
participant AS as AuthStorage (mastracode)
participant API as console.anthropic.com
App->>CS: getAnthropicAuthStatus() / resolveCredentials()
CS->>AS: authStorage.reload()
AS-->>CS: storedCredential (type: oauth, expires: past)
Note over CS: expires <= Date.now() → attempt refresh
CS->>AS: authStorage.getApiKey("anthropic")
AS->>API: POST /v1/oauth/token (refresh_token)
API-->>AS: new access_token + expires
AS-->>CS: new access_token (persisted to storage)
CS->>AS: authStorage.reload()
AS-->>CS: refreshed credential (expires: future)
CS-->>App: authenticated: true, method: "oauth"
Note over App,API: If refresh fails
CS->>AS: authStorage.getApiKey("anthropic")
AS-xAPI: POST /v1/oauth/token (refresh_token)
API-->>AS: 401 / error
AS-->>CS: throws / returns null
CS-->>App: authenticated: false, issue: "expired"
Comments Outside Diff (1)
-
packages/chat/src/server/desktop/chat-service/chat-service.test.ts, line 17-48 (link)FakeAuthStoragemissinggetApiKeymockgetAnthropicAuthStatusnow callsawait authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID)when a managed OAuth credential is expired (chat-service.ts lines 91–99). BecauseFakeAuthStoragehas nogetApiKeyproperty, any test that stores an expired OAuth credential and callsgetAnthropicAuthStatuswill throwTypeError: authStorage.getApiKey is not a function. That error is swallowed silently by thecatch {}block, so the test still passes — but the successful refresh path (the core new behaviour of this PR) is never exercised.The
FakeAuthStoragetype andcreateFakeAuthStoragefactory should be extended to include the new method. A new test should also be added to verify that when an expired managed OAuth credential exists and the refresh call returns a new token,getAnthropicAuthStatusreturnsauthenticated: truerather thanissue: "expired". Without this, a regression in the refresh flow would go completely undetected by the test suite.Prompt To Fix With AI
This is a comment left during a code review. Path: packages/chat/src/server/desktop/chat-service/chat-service.test.ts Line: 17-48 Comment: **`FakeAuthStorage` missing `getApiKey` mock** `getAnthropicAuthStatus` now calls `await authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID)` when a managed OAuth credential is expired (chat-service.ts lines 91–99). Because `FakeAuthStorage` has no `getApiKey` property, any test that stores an expired OAuth credential and calls `getAnthropicAuthStatus` will throw `TypeError: authStorage.getApiKey is not a function`. That error is swallowed silently by the `catch {}` block, so the test still passes — but the successful refresh path (the core new behaviour of this PR) is never exercised. The `FakeAuthStorage` type and `createFakeAuthStorage` factory should be extended to include the new method. A new test should also be added to verify that when an expired managed OAuth credential exists and the refresh call returns a new token, `getAnthropicAuthStatus` returns `authenticated: true` rather than `issue: "expired"`. Without this, a regression in the refresh flow would go completely undetected by the test suite. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/chat/src/server/desktop/auth/anthropic/anthropic.ts
Line: 190-207
Comment:
**`getApiKey` called unconditionally for all OAuth credentials**
`getCredentialsFromAuthStorage` calls `authStorage.getApiKey()` for every OAuth credential regardless of whether it has expired. The PR description states "Resolvers now call `authStorage.getApiKey("anthropic")` on **expired** oauth creds", but there is no expiry guard here.
Both `chat-service.ts` (lines 91–93) and `resolveAnthropicCredential.ts` (line 103) check expiry before calling `getApiKey`. Calling it unconditionally means every credential resolution for a still-valid OAuth token goes through `getApiKey` rather than just reading the already-loaded value from `authStorage.get()`. If mastracode's `getApiKey` were to do I/O or a network call for non-expired tokens in a future release, this would add unnecessary latency on every message send.
Aligning with the pattern used in the other two call sites — only calling `getApiKey` when `expires <= Date.now()` — would be more consistent and forward-safe.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/chat/src/server/desktop/chat-service/chat-service.test.ts
Line: 17-48
Comment:
**`FakeAuthStorage` missing `getApiKey` mock**
`getAnthropicAuthStatus` now calls `await authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID)` when a managed OAuth credential is expired (chat-service.ts lines 91–99). Because `FakeAuthStorage` has no `getApiKey` property, any test that stores an expired OAuth credential and calls `getAnthropicAuthStatus` will throw `TypeError: authStorage.getApiKey is not a function`. That error is swallowed silently by the `catch {}` block, so the test still passes — but the successful refresh path (the core new behaviour of this PR) is never exercised.
The `FakeAuthStorage` type and `createFakeAuthStorage` factory should be extended to include the new method. A new test should also be added to verify that when an expired managed OAuth credential exists and the refresh call returns a new token, `getAnthropicAuthStatus` returns `authenticated: true` rather than `issue: "expired"`. Without this, a regression in the refresh flow would go completely undetected by the test suite.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(auth): auto-refresh expired Anthropi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (credential.type === "oauth") { | ||
| // mastracode's getApiKey triggers refreshToken() when expires <= now, | ||
| // and persists the refreshed credential back into auth storage. | ||
| const accessToken = await authStorage.getApiKey( | ||
| ANTHROPIC_AUTH_PROVIDER_ID, | ||
| ); | ||
| if (!accessToken || accessToken.trim().length === 0) return null; | ||
| authStorage.reload(); | ||
| const refreshed = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID); | ||
| return { | ||
| apiKey: credential.access.trim(), | ||
| apiKey: accessToken.trim(), | ||
| source: "auth-storage", | ||
| kind: "oauth", | ||
| expiresAt: | ||
| typeof credential.expires === "number" | ||
| ? credential.expires | ||
| refreshed?.type === "oauth" && typeof refreshed.expires === "number" | ||
| ? refreshed.expires | ||
| : undefined, | ||
| }; |
There was a problem hiding this comment.
getApiKey called unconditionally for all OAuth credentials
getCredentialsFromAuthStorage calls authStorage.getApiKey() for every OAuth credential regardless of whether it has expired. The PR description states "Resolvers now call authStorage.getApiKey("anthropic") on expired oauth creds", but there is no expiry guard here.
Both chat-service.ts (lines 91–93) and resolveAnthropicCredential.ts (line 103) check expiry before calling getApiKey. Calling it unconditionally means every credential resolution for a still-valid OAuth token goes through getApiKey rather than just reading the already-loaded value from authStorage.get(). If mastracode's getApiKey were to do I/O or a network call for non-expired tokens in a future release, this would add unnecessary latency on every message send.
Aligning with the pattern used in the other two call sites — only calling getApiKey when expires <= Date.now() — would be more consistent and forward-safe.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/chat/src/server/desktop/auth/anthropic/anthropic.ts
Line: 190-207
Comment:
**`getApiKey` called unconditionally for all OAuth credentials**
`getCredentialsFromAuthStorage` calls `authStorage.getApiKey()` for every OAuth credential regardless of whether it has expired. The PR description states "Resolvers now call `authStorage.getApiKey("anthropic")` on **expired** oauth creds", but there is no expiry guard here.
Both `chat-service.ts` (lines 91–93) and `resolveAnthropicCredential.ts` (line 103) check expiry before calling `getApiKey`. Calling it unconditionally means every credential resolution for a still-valid OAuth token goes through `getApiKey` rather than just reading the already-loaded value from `authStorage.get()`. If mastracode's `getApiKey` were to do I/O or a network call for non-expired tokens in a future release, this would add unnecessary latency on every message send.
Aligning with the pattern used in the other two call sites — only calling `getApiKey` when `expires <= Date.now()` — would be more consistent and forward-safe.
How can I resolve this? If you propose a fix, please make it concise.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/chat/src/server/desktop/auth/anthropic/anthropic.ts (1)
178-213:⚠️ Potential issue | 🟡 MinorReturn stored OAuth credential as fallback when token refresh fails, preserving the "expired" signal for UI.
When
getApiKey()returns an empty token or fails (line 196),getCredentialsFromAuthStorage()now returnsnull, which preventsgetCredentialsFromAnySource()from tracking an expired OAuth credential as a fallback. This loses the "expired" signal thatchat-service.tsrelies on to distinguish "missing credentials" (issue: null) from "expired credentials" (issue: "expired"), affecting the user-facing auth status message.Since the function already attempts a refresh via
getApiKey(), the fallback should return the stored credential withexpiresAtwhen refresh fails, mirroring the pattern inchat-service.tslines 95–102. This preserves the intended expired-credentials fallback logic and provides callers with the signal they need.🛠️ Suggested diff
if (credential.type === "oauth") { + const fallbackAccess = + typeof credential.access === "string" && credential.access.trim().length > 0 + ? credential.access.trim() + : null; + const fallbackExpiresAt = + typeof credential.expires === "number" ? credential.expires : undefined; // mastracode's getApiKey triggers refreshToken() when expires <= now, // and persists the refreshed credential back into auth storage. - const accessToken = await authStorage.getApiKey( - ANTHROPIC_AUTH_PROVIDER_ID, - ); - if (!accessToken || accessToken.trim().length === 0) return null; + let accessToken: string | null = null; + try { + const refreshed = await authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID); + accessToken = refreshed?.trim() ? refreshed.trim() : null; + } catch (error) { + console.warn("[claude/auth] Failed to refresh OAuth token:", error); + } + if (!accessToken) { + if (!fallbackAccess) return null; + return { + apiKey: fallbackAccess, + source: "auth-storage", + kind: "oauth", + expiresAt: fallbackExpiresAt, + }; + } authStorage.reload(); const refreshed = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID); return { - apiKey: accessToken.trim(), + apiKey: accessToken, source: "auth-storage", kind: "oauth", expiresAt: refreshed?.type === "oauth" && typeof refreshed.expires === "number" ? refreshed.expires : undefined, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/desktop/auth/anthropic/anthropic.ts` around lines 178 - 213, In getCredentialsFromAuthStorage(), when credential.type === "oauth" and authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID) returns no token or throws, return the stored OAuth credential as a fallback (instead of null) so callers can detect an expired credential; specifically, after calling authStorage.getApiKey(...) if accessToken is empty or on failure, call authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID) to retrieve the stored credential and return an object with apiKey set to accessToken?.trim() (or undefined if empty), source: "auth-storage", kind: "oauth", and expiresAt taken from the refreshed or stored credential (refreshed?.expires when number, otherwise stored.expires when number, else undefined) so the expired signal is preserved by getCredentialsFromAnySource()/chat-service.ts. Ensure this logic is applied inside getCredentialsFromAuthStorage() around the authStorage.getApiKey call and its error path.
🧹 Nitpick comments (4)
packages/chat/src/server/desktop/auth/anthropic/anthropic.ts (1)
227-233: Minor: redundant nullish coalescing.
storageCredentialis already typedClaudeCredentials | null, sostorageCredential ?? nullcollapses tostorageCredential. Not incorrect, just noise.- firstExpired ??= storageCredential ?? null; + firstExpired ??= storageCredential;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/desktop/auth/anthropic/anthropic.ts` around lines 227 - 233, The assignment uses redundant nullish coalescing: storageCredential is already ClaudeCredentials | null, so replace "firstExpired ??= storageCredential ?? null" with "firstExpired ??= storageCredential" to remove the unnecessary "?? null"; update the block around getCredentialsFromAuthStorage, storageCredential, isClaudeCredentialExpired, and firstExpired accordingly so the logic and types remain unchanged.packages/chat/src/server/desktop/small-model/small-model.ts (1)
29-32: Nitpick: consider aMaybePromise<T>alias.+type MaybePromise<T> = T | Promise<T>; + export interface SmallModelProvider { id: SmallModelProviderId; name: string; - resolveCredentials: () => - | SmallModelCredential - | null - | Promise<SmallModelCredential | null>; + resolveCredentials: () => MaybePromise<SmallModelCredential | null>;Same pattern could apply to
createModel'sunknown | Promise<unknown>on line 39.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/desktop/small-model/small-model.ts` around lines 29 - 32, Introduce a generic alias MaybePromise<T> = T | Promise<T> and replace the union types in SmallModel interface: change resolveCredentials's return type to MaybePromise<SmallModelCredential | null> and change createModel's return type to MaybePromise<unknown> (or MaybePromise<YourSpecificType> if more precise). Update any imports/exports and usages of resolveCredentials and createModel in small-model.ts so they use the new MaybePromise<T> alias consistently.packages/chat/src/server/desktop/chat-service/chat-service.ts (1)
95-102: Consider logging refresh failures for debuggability.The silent catch makes it hard to diagnose "why did my token not refresh?" in the field. A debug-guarded log (matching the existing
logAuthResolutionpattern used throughout this method) would help without being noisy in production.} catch { // Refresh failed; fall through to expired-state handling below. + // (debug logging is gated by SUPERSET_DEBUG_AUTH=1 via logAuthResolution) }Or inline:
- } catch { + } catch (error) { // Refresh failed; fall through to expired-state handling below. + this.logAuthResolution("anthropic", { + event: "refresh-failed", + reason: error instanceof Error ? error.message : String(error), + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/desktop/chat-service/chat-service.ts` around lines 95 - 102, The empty catch in the auth refresh block (around calls to authStorage.getApiKey, authStorage.reload and reading storedCredential) hides refresh failures; update the catch to call the existing logAuthResolution-style logger with a debug/trace-level message that includes the error and context (e.g., provider id ANTHROPIC_AUTH_PROVIDER_ID and any partial state), so failures are recorded without being noisy in production; keep the rest of the flow unchanged and do not rethrow.packages/chat/src/server/desktop/chat-service/chat-service.test.ts (1)
17-48: Missing test coverage for the new OAuth refresh path.The PR's core fix —
getAnthropicAuthStatuscallingauthStorage.getApiKeyto refresh before marking expired — isn't exercised by any test. TheFakeAuthStoragehere doesn't mockgetApiKey, so the production code path that calls it is unverified.Consider adding a test where:
fakeAuthStorage.set("anthropic", { type: "oauth", access: "old", expires: Date.now() - 1 })- A mocked
getApiKeyimplementation that updates the stored credential to a fresh one and returns the new access token.await chatService.getAnthropicAuthStatus()should return{ authenticated: true, method: "oauth", issue: null, ... }.Also a companion test where
getApiKeyrejects → status still resolves toissue: "expired".🧪 Example additions
type FakeAuthStorage = { reload: ReturnType<typeof mock<() => void>>; get: ReturnType<typeof mock<(providerId: string) => Credential | undefined>>; + getApiKey: ReturnType< + typeof mock<(providerId: string) => Promise<string | null>> + >; set: ReturnType< typeof mock<(providerId: string, credential: Credential) => void> >; ...Then add tests invoking
fakeAuthStorage.getApiKey.mockImplementation(...)for success and failure scenarios.Want me to draft the two test cases?
Also applies to: 91-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/server/desktop/chat-service/chat-service.test.ts` around lines 17 - 48, The tests lack coverage for the new OAuth refresh path: extend FakeAuthStorage and createFakeAuthStorage to include a getApiKey mock (ReturnType<typeof mock<(providerId: string) => Promise<string>>>), then add two tests that use that fake storage and call chatService.getAnthropicAuthStatus(); in the success test call fakeAuthStorage.set("anthropic", { type: "oauth", access: "old", expires: Date.now() - 1 }) and fakeAuthStorage.getApiKey.mockImplementation(async (id) => { fakeAuthStorage.set("anthropic", { type: "oauth", access: "new", expires: Date.now() + 10000 }); return "new"; }) and assert the status shows authenticated: true, method: "oauth", issue: null; in the failure test mock getApiKey to reject and assert the status shows issue: "expired" (or equivalent), ensuring you reference FakeAuthStorage/createFakeAuthStorage and getAnthropicAuthStatus/authStorage.getApiKey when adding the mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chat/src/server/desktop/chat-service/chat-service.ts`:
- Around line 81-102: getAnthropicAuthStatus currently awaits
authStorage.getApiKey whenever a managed OAuth token is expired, which can block
tRPC status checks; change it to avoid synchronous network I/O on the status
path by returning the "expired" status immediately and triggering a background
refresh instead. Specifically, in getAnthropicAuthStatus, when
storedCredential.type === "oauth" and storedCredential.expires <= Date.now(), do
not await authStorage.getApiKey; instead kick off a non-blocking refresh (e.g.,
call authStorage.getApiKey without await or schedule a background task) and
guard against repeated refreshes by using an in-memory dedupe (a module-level
Map/Promise keyed by ANTHROPIC_AUTH_PROVIDER_ID) so concurrent calls only
trigger one refresh; leave the storedCredential reloading flow intact for
successful refresh completion but ensure the tRPC response is returned without
waiting.
---
Outside diff comments:
In `@packages/chat/src/server/desktop/auth/anthropic/anthropic.ts`:
- Around line 178-213: In getCredentialsFromAuthStorage(), when credential.type
=== "oauth" and authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID) returns no
token or throws, return the stored OAuth credential as a fallback (instead of
null) so callers can detect an expired credential; specifically, after calling
authStorage.getApiKey(...) if accessToken is empty or on failure, call
authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID) to retrieve the stored credential
and return an object with apiKey set to accessToken?.trim() (or undefined if
empty), source: "auth-storage", kind: "oauth", and expiresAt taken from the
refreshed or stored credential (refreshed?.expires when number, otherwise
stored.expires when number, else undefined) so the expired signal is preserved
by getCredentialsFromAnySource()/chat-service.ts. Ensure this logic is applied
inside getCredentialsFromAuthStorage() around the authStorage.getApiKey call and
its error path.
---
Nitpick comments:
In `@packages/chat/src/server/desktop/auth/anthropic/anthropic.ts`:
- Around line 227-233: The assignment uses redundant nullish coalescing:
storageCredential is already ClaudeCredentials | null, so replace "firstExpired
??= storageCredential ?? null" with "firstExpired ??= storageCredential" to
remove the unnecessary "?? null"; update the block around
getCredentialsFromAuthStorage, storageCredential, isClaudeCredentialExpired, and
firstExpired accordingly so the logic and types remain unchanged.
In `@packages/chat/src/server/desktop/chat-service/chat-service.test.ts`:
- Around line 17-48: The tests lack coverage for the new OAuth refresh path:
extend FakeAuthStorage and createFakeAuthStorage to include a getApiKey mock
(ReturnType<typeof mock<(providerId: string) => Promise<string>>>), then add two
tests that use that fake storage and call chatService.getAnthropicAuthStatus();
in the success test call fakeAuthStorage.set("anthropic", { type: "oauth",
access: "old", expires: Date.now() - 1 }) and
fakeAuthStorage.getApiKey.mockImplementation(async (id) => {
fakeAuthStorage.set("anthropic", { type: "oauth", access: "new", expires:
Date.now() + 10000 }); return "new"; }) and assert the status shows
authenticated: true, method: "oauth", issue: null; in the failure test mock
getApiKey to reject and assert the status shows issue: "expired" (or
equivalent), ensuring you reference FakeAuthStorage/createFakeAuthStorage and
getAnthropicAuthStatus/authStorage.getApiKey when adding the mocks.
In `@packages/chat/src/server/desktop/chat-service/chat-service.ts`:
- Around line 95-102: The empty catch in the auth refresh block (around calls to
authStorage.getApiKey, authStorage.reload and reading storedCredential) hides
refresh failures; update the catch to call the existing logAuthResolution-style
logger with a debug/trace-level message that includes the error and context
(e.g., provider id ANTHROPIC_AUTH_PROVIDER_ID and any partial state), so
failures are recorded without being noisy in production; keep the rest of the
flow unchanged and do not rethrow.
In `@packages/chat/src/server/desktop/small-model/small-model.ts`:
- Around line 29-32: Introduce a generic alias MaybePromise<T> = T | Promise<T>
and replace the union types in SmallModel interface: change resolveCredentials's
return type to MaybePromise<SmallModelCredential | null> and change
createModel's return type to MaybePromise<unknown> (or
MaybePromise<YourSpecificType> if more precise). Update any imports/exports and
usages of resolveCredentials and createModel in small-model.ts so they use the
new MaybePromise<T> alias consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9979f83f-112f-4f5e-8af5-170823bd31ef
📒 Files selected for processing (7)
apps/desktop/src/lib/ai/call-small-model.tspackages/chat/src/server/desktop/auth/anthropic/anthropic.tspackages/chat/src/server/desktop/chat-service/chat-service.test.tspackages/chat/src/server/desktop/chat-service/chat-service.tspackages/chat/src/server/desktop/small-model/small-model.tspackages/host-service/src/providers/model-providers/LocalModelProvider/LocalModelProvider.tspackages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts
| async getAnthropicAuthStatus(): Promise<AuthStatus> { | ||
| const authStorage = this.getAuthStorage(); | ||
| authStorage.reload(); | ||
| const storedCredential = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID); | ||
| let storedCredential = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID); | ||
| const hasManagedOAuth = storedCredential?.type === "oauth"; | ||
|
|
||
| // If managed OAuth is past its expiry, give mastracode a chance to | ||
| // refresh it before downgrading status to "expired". Mastracode's | ||
| // getApiKey uses the stored refresh token via the anthropic provider. | ||
| if ( | ||
| storedCredential?.type === "oauth" && | ||
| typeof storedCredential.expires === "number" && | ||
| storedCredential.expires <= Date.now() | ||
| ) { | ||
| try { | ||
| await authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID); | ||
| authStorage.reload(); | ||
| storedCredential = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID); | ||
| } catch { | ||
| // Refresh failed; fall through to expired-state handling below. | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
mastracode authStorage getApiKey in-flight refresh deduplication
💡 Result:
In MastraCode, authStorage is an instance of the AuthStorage class, created via createAuthStorage and returned by createMastraCode. It manages authentication credentials for AI providers like Anthropic (Claude) and OpenAI, supporting both API keys (stored in auth.json) and OAuth. Key methods and usage from source code (mastracode/src/index.ts): - authStorage.getStoredApiKey(provider): Retrieves a stored API key for a given provider (e.g., 'anthropic', 'openai', MEMORY_GATEWAY_PROVIDER). Used at startup to populate environment variables like ANTHROPIC_API_KEY if not already set. - authStorage.loadStoredApiKeysIntoEnv(providerEnvVars): Loads all stored API keys into process.env. - authStorage.get(providerId): Gets credentials for a provider, distinguishing between {type: 'api_key', key: string} and {type: 'oauth'}. - authStorage.hasStoredApiKey(provider): Checks if a stored API key exists for a provider. - authStorage.isLoggedIn(oauthId): Checks OAuth login status. - authStorage.login(providerId, callbacks) and authStorage.logout(providerId): Handle OAuth flows. API keys are entered interactively via prompts (stored persistently in auth.json), auto-detected from env vars, or loaded from storage. OAuth is used for subscriptions like Claude Max/OpenAI ChatGPT Plus. No explicit documentation or source snippets found for "in-flight refresh" (e.g., OAuth token renewal) or "deduplication" mechanisms in authStorage.getApiKey. Related concepts: - Mastra has past issues with duplicate messages/storage in multi-step/client-side executions (#7843, #9370), fixed via message ID checks and merging logic. - OAuth refresh races noted in similar tools (e.g., Claude Code #24317), suggesting potential need for locking/deduplication in concurrent sessions, but not confirmed in MastraCode authStorage. AuthStorage ensures a single shared instance across providers (tested in create-auth-storage.test.ts), preventing multiple initializations. For API keys, retrieval is synchronous from storage—no refresh needed as they don't expire like OAuth tokens. If implementing custom refresh/deduplication (e.g., for concurrent getApiKey calls), use a singleton pattern, Promise caching, or mutex (e.g., async-mutex) to dedupe in-flight operations: Example pattern: private refreshPromise: Promise | null = null; async getApiKey(provider: string): Promise { if (this.refreshPromise) return this.refreshPromise; // Dedupe in-flight this.refreshPromise = this.refreshStoredOrFetch(provider); try { return await this.refreshPromise; } finally { this.refreshPromise = null; } } Official docs: https://code.mastra.ai/reference, https://code.mastra.ai/configuration. Source: https://github.com/mastra-ai/mastra/tree/main/mastracode/src/auth (inferred from imports).
Citations:
- 1: feat(mastracode): support external tool injection and auth storage init mastra-ai/mastra#13561
- 2: https://code.mastra.ai/configuration
- 3: https://code.mastra.ai/reference
- 4: https://github.com/mastra-ai/mastra/blob/12923c60/mastracode/src/index.ts
- 5: https://github.com/mastra-ai/mastra/blob/12923c60/mastracode/src/tui/commands/login.ts
🏁 Script executed:
cd packages/chat && git ls-files src/server/desktop/chat-service/chat-service.tsRepository: superset-sh/superset
Length of output: 111
🏁 Script executed:
cat -n packages/chat/src/server/desktop/chat-service/chat-service.ts | sed -n '75,110p'Repository: superset-sh/superset
Length of output: 1724
🌐 Web query:
mastracode authStorage OAuth refresh deduplication 2025 2026
💡 Result:
Short answer: Mastra Code’s auth storage is designed to be a single shared AuthStorage instance (initialized via a dedicated createAuthStorage export), and it doesn’t document a specific “OAuth refresh token deduplication” mechanism in the sources found. Separately, Mastra/SDK-related “deduplication” issues seen in 2025–2026 appear to be about message duplication (not OAuth refresh), while OAuth refresh-token “race/reuse” handling is addressed in other projects (e.g., OpenAI Codex) and generally in auth libraries via refresh token rotation/reuse detection. What I found in Mastra Code (2025–2026) 1) Shared authStorage initialization (Mastra Code) - Mastra Code added/exports a createAuthStorage so auth providers can be initialized without creating a full createMastraCode instance. - The PR notes unit tests asserting createAuthStorage returns a single shared AuthStorage instance and wires it to multiple provider modules. - createMastraCode returns authStorage as an AuthStorage instance, described as “Storage for OAuth credentials”. Sources indicate the focus here is on centralized/one-instance auth storage wiring, not refresh-token request deduplication. 2) OAuth token lifecycle for HTTP MCP servers (Mastra) - Mastra added an authProvider option for HTTP-based MCP servers (merged July/Aug 2025) “to enable OAuth authentication with automatic token refresh.” - A later enhancement issue (Mar 2026) describes the lack of first-class dynamic auth configuration for HTTP MCP servers (static headers only), and mentions adding first-class support; it also references a PR adding OAuth 2.1 authentication for HTTP MCP servers. What I found about “deduplication” vs OAuth refresh - The Mastra issues surfaced in search results for 2025–2026 use “duplicating response messages” terminology and discuss deduping/merging UI messages from storage; this is not OAuth refresh-token deduplication. - For OAuth refresh, I found authoritative guidance from auth systems about refresh token rotation and the security problem of refresh token reuse (race conditions / replay). For example: - Auth.js explicitly notes there is no built-in automatic refresh token rotation and discusses that refresh tokens are usually only usable once, leading to potential race conditions if multiple requests refresh concurrently. - Auth0 describes refresh token rotation and “automatic reuse detection”/invalidating previously-used refresh tokens to prevent replay. - For an explicit “deduplicate/race-guard” approach around OAuth refresh across processes, I found a Codex PR: it fixes “refresh-token races across Codex processes” by doing a guarded reload flow—reloading shared credentials before refresh attempt and comparing persisted vs in-memory snapshots; if persisted credentials changed, it adopts them and skips local refresh; otherwise it refreshes and persists rotated credentials. Conclusion for your query - For Mastra Code: based on the sources found, the clearly documented change in 2025–2026 is centralized shared authStorage initialization (createAuthStorage + shared instance), and OAuth support for HTTP MCP servers via an authProvider (automatic token refresh). - However, I did not find Mastra-specific documentation or code excerpts in these sources that explicitly describe “OAuth refresh deduplication” (e.g., ensuring concurrent refresh calls share one in-flight refresh, or guarding refresh-token reuse across processes) as a named feature. - In general auth systems, dedup/race handling is often implemented via refresh token rotation + reuse detection (Auth0) and by addressing concurrency races (Auth.js mentions the inherent once-only limitation and race conditions). For a concrete guarded approach, the Codex PR provides an example pattern. If you want, I can do a deeper targeted code-level search in the Mastra repository for terms like “refresh_token_reused”, “reuse detection”, “inFlight”, “mutex”, “dedupe”, or “guarded reload” specifically within mastracode and HTTP MCP OAuth flows.
Citations:
- 1: https://mastra.ai/reference/mastra-code/createMastraCode
- 2: https://code.mastra.ai/reference
- 3: feat(mastracode): support external tool injection and auth storage init mastra-ai/mastra#13561
- 4: feat: expose authProvider for HTTP MCP servers to enable OAuth token refresh mastra-ai/mastra#6370
- 5: https://authjs.dev/guides/refresh-token-rotation?frameworks=core
- 6: https://auth0.com/docs/secure/tokens/refresh-tokens/refresh-token-rotation
- 7: Fix MCP OAuth refresh-token races across Codex processes openai/codex#12815
- 8: Refresh Token Rotation Grace Period (Overlap Window) better-auth/better-auth#8512
- 9: Mastra is duplicating response messages. mastra-ai/mastra#10071
- 10: [BUG] Mastra with ai-sdk v5 may cause duplicate assistant messages mastra-ai/mastra#9370
🏁 Script executed:
rg "getAnthropicAuthStatus" --type ts --type tsx -B 2 -A 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
cat -n packages/chat/src/server/desktop/chat-service/chat-service.ts | sed -n '1,80p' | tail -30Repository: superset-sh/superset
Length of output: 1286
🏁 Script executed:
rg "authStorage\.getApiKey|refresh.*oauth|dedupe|inFlight" packages/chat/src/server/desktop/chat-service/ -A 2Repository: superset-sh/superset
Length of output: 935
🏁 Script executed:
rg "getAnthropicAuthStatus" -t ts --type-add 'ts:*.ts' -B 3 -A 3Repository: superset-sh/superset
Length of output: 13673
🏁 Script executed:
cat -n packages/chat/src/server/desktop/chat-service/chat-service.ts | sed -n '100,150p'Repository: superset-sh/superset
Length of output: 2269
🏁 Script executed:
grep -r "getAnthropicAuthStatus" --include="*.ts" -l | head -10Repository: superset-sh/superset
Length of output: 301
Ensure getAnthropicAuthStatus doesn't cause status-check delays when OAuth tokens are expired.
The method now performs network I/O (via authStorage.getApiKey) on every call with an expired managed OAuth token. Since it's exposed as a tRPC query and called from the model-providers router (potentially on mount/polling), repeated calls while a refresh endpoint is slow or unavailable could block status checks. The review mentions mastracode's internal deduplication—confirm it handles concurrent refresh calls to prevent request stacking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chat/src/server/desktop/chat-service/chat-service.ts` around lines
81 - 102, getAnthropicAuthStatus currently awaits authStorage.getApiKey whenever
a managed OAuth token is expired, which can block tRPC status checks; change it
to avoid synchronous network I/O on the status path by returning the "expired"
status immediately and triggering a background refresh instead. Specifically, in
getAnthropicAuthStatus, when storedCredential.type === "oauth" and
storedCredential.expires <= Date.now(), do not await authStorage.getApiKey;
instead kick off a non-blocking refresh (e.g., call authStorage.getApiKey
without await or schedule a background task) and guard against repeated
refreshes by using an in-memory dedupe (a module-level Map/Promise keyed by
ANTHROPIC_AUTH_PROVIDER_ID) so concurrent calls only trigger one refresh; leave
the storedCredential reloading flow intact for successful refresh completion but
ensure the tRPC response is returned without waiting.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts">
<violation number="1" location="packages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts:123">
P2: The refresh failure path silently swallows errors; add contextual warning logging before falling back so OAuth refresh failures are observable.
(Based on your team's feedback about handling async failures explicitly and avoiding silent error swallowing.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/chat/src/server/desktop/chat-service/chat-service.ts">
<violation number="1" location="packages/chat/src/server/desktop/chat-service/chat-service.ts:99">
P2: Avoid silently swallowing Anthropic token refresh failures; log the error context so refresh issues can be diagnosed.
(Based on your team's feedback about handling async errors explicitly and avoiding empty catch blocks.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } catch { | ||
| return { kind: "oauth", expiresAt }; | ||
| } |
There was a problem hiding this comment.
P2: The refresh failure path silently swallows errors; add contextual warning logging before falling back so OAuth refresh failures are observable.
(Based on your team's feedback about handling async failures explicitly and avoiding silent error swallowing.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts, line 123:
<comment>The refresh failure path silently swallows errors; add contextual warning logging before falling back so OAuth refresh failures are observable.
(Based on your team's feedback about handling async failures explicitly and avoiding silent error swallowing.) </comment>
<file context>
@@ -97,18 +97,39 @@ function getAnthropicCredentialFromAuthStorage(): LocalResolvedCredential | null
+ };
+ }
+ return { kind: "oauth", expiresAt };
+ } catch {
+ return { kind: "oauth", expiresAt };
+ }
</file context>
| } catch { | |
| return { kind: "oauth", expiresAt }; | |
| } | |
| } catch (error) { | |
| console.warn("Failed to refresh Anthropic OAuth credential", error); | |
| return { kind: "oauth", expiresAt }; | |
| } |
| } catch { | ||
| // Refresh failed; fall through to expired-state handling below. | ||
| } |
There was a problem hiding this comment.
P2: Avoid silently swallowing Anthropic token refresh failures; log the error context so refresh issues can be diagnosed.
(Based on your team's feedback about handling async errors explicitly and avoiding empty catch blocks.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/chat/src/server/desktop/chat-service/chat-service.ts, line 99:
<comment>Avoid silently swallowing Anthropic token refresh failures; log the error context so refresh issues can be diagnosed.
(Based on your team's feedback about handling async errors explicitly and avoiding empty catch blocks.) </comment>
<file context>
@@ -78,11 +78,28 @@ export class ChatService {
+ await authStorage.getApiKey(ANTHROPIC_AUTH_PROVIDER_ID);
+ authStorage.reload();
+ storedCredential = authStorage.get(ANTHROPIC_AUTH_PROVIDER_ID);
+ } catch {
+ // Refresh failed; fall through to expired-state handling below.
+ }
</file context>
| } catch { | |
| // Refresh failed; fall through to expired-state handling below. | |
| } | |
| } catch (error) { | |
| console.warn("[chat-service] Failed to refresh Anthropic OAuth token", error); | |
| } |
1 participant
Summary
authStorage.get()everywhere, which never triggered mastracode's built-in refresh flow. When the 1-hour access token expired, the UI flipped to Reconnect and forced a full PKCE re-auth — even though a valid refresh token was already stored.authStorage.getApiKey("anthropic")on expired oauth creds, which triggers mastracode'srefreshToken()againstconsole.anthropic.com/v1/oauth/tokenand persists the refreshed credential.getAnthropicAuthStatusdoes the same before declaringissue: "expired".small-model.ts).Files changed
packages/chat/src/server/desktop/auth/anthropic/anthropic.ts—getCredentialsFromAuthStorage/getCredentialsFromAnySourcenowasync; refreshes viagetApiKeywhen oauth cred is past expiry.packages/chat/src/server/desktop/chat-service/chat-service.ts—getAnthropicAuthStatusnowasync; attempts refresh before downgrading managed OAuth toissue: "expired".packages/host-service/.../resolveAnthropicCredential.ts— same pattern;LocalModelProvider.resolveRuntimeEnvcascaded toasync.packages/chat/src/server/desktop/small-model/small-model.ts—SmallModelProvider.resolveCredentialstype widened to allowPromise.apps/desktop/src/lib/ai/call-small-model.ts— awaitsresolveCredentials().chat-service.test.ts): mocks return async, expired-OAuth callsites nowawait.Test plan
bun run typecheck— passes across all 25 packages.bun run lint— clean.bun test— chat (122), host-service (112), call-small-model (9) all pass.expires), send a chat message — should succeed silently without a Reconnect prompt.Summary by cubic
Auto-refresh expired Anthropic OAuth tokens to prevent “Reconnect” prompts and avoid full re-auth. This uses
mastracode’s refresh flow viaauthStorage.getApiKey("anthropic")and persists the refreshed token.Bug Fixes
authStorage.getApiKey("anthropic").ChatService.getAnthropicAuthStatus()is now async and refreshes before marking expired.SmallModelProvider.resolveCredentialsnow supportsPromise;call-small-modelawaits provider creds.LocalModelProviderruntime env resolution is async;resolveAnthropicCredentialattempts refresh on expired OAuth.Migration
ChatService.getAnthropicAuthStatus()in callers.SmallModelProvider, you can now return aPromisefromresolveCredentials; ensure callersawaitit.Written for commit e467666. Summary will update on new commits.
Summary by CodeRabbit