fix(mastracode): check stored API key slot and env vars in key resolution#15483
Conversation
🦋 Changeset detectedLatest commit: 82a9ec8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdjusted API-key resolution in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Thank you for your contribution! Please ensure that your PR fixes an existing issue and that you have linked it in the description (e.g. with We use CodeRabbit for automated code reviews. Please address all feedback from CodeRabbit by either making changes to your PR or leaving a comment explaining why you disagree with the feedback. Since CodeRabbit is an AI, it may occasionally provide incorrect feedback. Addressing CodeRabbit's feedback will greatly increase the chances of your PR being merged. We appreciate your understanding and cooperation in helping us maintain high code quality standards. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mastracode/src/agents/model.ts`:
- Around line 105-107: The code currently treats whitespace-only environment
values as valid; update the retrieval for process.env.ANTHROPIC_API_KEY (and the
analogous block at lines around 130–132) to trim() the value and only return it
if the trimmed string is non-empty—i.e., const v =
process.env.ANTHROPIC_API_KEY?.trim(); if (v) return v;—so whitespace-only keys
are treated as missing and the provider fallback path is used.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5cf88450-7644-486f-9efc-29f6ac03f90b
📒 Files selected for processing (1)
mastracode/src/agents/model.ts
|
@Kitenite you need a changeset and make sure the rabbit is happy |
5c26d6f to
3fdafd8
Compare
…icApiKey/getOpenAIApiKey getAnthropicApiKey() and getOpenAIApiKey() only check the main credential slot (authStorage.get). This misses two valid sources: 1. The dedicated API key slot (authStorage.getStoredApiKey) — designed to persist independently of OAuth connect/disconnect. When OAuth login overwrites the main slot and then OAuth is disconnected, the main slot is empty but the dedicated slot still has the key. 2. Environment variables (ANTHROPIC_API_KEY / OPENAI_API_KEY) — the standard way to provide API keys in CI, containers, and dev envs. Both functions now check: main slot → dedicated slot → env var. This fixes the case where a user sets an API key, later connects OAuth (which overwrites the main slot), then disconnects OAuth — resolveModel falls through to the OAuth provider and throws "Not logged in" even though the API key still exists in the dedicated slot.
3fdafd8 to
4b0d95c
Compare
| return undefined; | ||
| const dedicatedKey = authStorage.getStoredApiKey('anthropic')?.trim(); | ||
| if (dedicatedKey) return dedicatedKey; | ||
| return process.env.ANTHROPIC_API_KEY?.trim() || undefined; |
There was a problem hiding this comment.
This part im not so sure if we want to do because in dev it'd pull in your process' env, lmk
Point to mastra-ai/mastra#15483 so the backup/restore code can be removed once upstream lands and we bump mastracode.
…3517) * remove 7 day rule * Upgrade mastra * upgrade ai * Ad mastra * refactor(desktop): remove dead provider-diagnostics plumbing The provider-diagnostics store was fed by callSmallModel's per-attempt reporting, which was removed when small-model tasks moved to direct AI-SDK + mastracode's AuthStorage. Nothing writes to the issue map anymore, so the clearIssue mutation, getStatuses query, and diagnosticStatus plumbing in ModelsSettings were all no-ops. Settings still surfaces "Session expired / Reconnect" via auth-status alone. ProviderIssue type collapsed from 8 codes to just "expired" to match. * fix(auth): auto-refresh expired Anthropic OAuth tokens 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. * review: address PR feedback from cubic + coderabbit + greptile - host-service ai-branch-name: run trailing-trim after slice so a 100-char truncation can't re-introduce a bare "." or "-" that git rejects as an invalid ref (coderabbit / cubic #2, #7). - host-service workspace-creation.generateBranchName: reuse the existing listBranchNames helper instead of the inline git walk, which classified off the short refname and could conflate a local "origin/foo" with refs/remotes/origin/foo (coderabbit #3). - packages/chat shared/small-model: drop the unused hasSmallModelCredentials export; only a test mock consumed it (greptile #4). - resolveAnthropicCredential: on refresh failure, return null instead of kind:"oauth" with a stale expiresAt so callers fall back cleanly (cubic #8). - chat-service.getAnthropicAuthStatus: log context when refresh throws instead of silently swallowing (cubic #9). * fix(chat): read auth.json directly instead of importing mastracode Importing createAuthStorage from mastracode loads the entire CLI tree (fastembed → onnxruntime-node's 208 MB native binary) via eager top-level requires in mastracode's CJS entry. This crashed electron-vite bundling and bloated the get-small-model chunk. getSmallModel now reads mastracode's auth.json file directly using the same path resolution logic (~/Library/Application Support/mastracode/ on macOS). Zero mastracode import, zero bundle impact. The chunk stays at 1.2 MB (just @ai-sdk/anthropic + @ai-sdk/openai). Production build verified: compile:app succeeds, Electron main process boots with no onnxruntime error. * docs(desktop): add manual testing plan for PR #3517 * fix api key storage slot * fix(auth): store API keys in dedicated slot so OAuth doesn't clobber them setApiKeyForProvider and setStoredAnthropicApiKeyFromEnvVariables now use authStorage.setStoredApiKey() (writes to "apikey:<provider>") instead of authStorage.set() (writes to the main "<provider>" slot shared with OAuth). This way connecting/disconnecting OAuth doesn't overwrite or delete a stored API key. resolveAuthMethodForProvider falls back to hasStoredApiKey() after checking the main slot, so status correctly reports authenticated when only an API key is stored. * fix(auth): backup/restore API keys across OAuth connect/disconnect mastracode's resolveModel only reads API keys from the main authStorage slot (authStorage.get("anthropic")). OAuth login overwrites this slot, and disconnect removes it — losing any previously saved API key. Fix: backup the API key to the dedicated apikey: slot before OAuth connect, restore it after disconnect. setApiKeyForProvider now writes to both slots (main for resolveModel compatibility, apikey: for backup). resolveAuthMethodForProvider checks both. Applies to both Anthropic and OpenAI providers. * chore: add upstream PR reference to auth workaround Point to mastra-ai/mastra#15483 so the backup/restore code can be removed once upstream lands and we bump mastracode. * refactor(desktop): derive settings provider action from status Replace the cascade of if/else + canDisconnect flag with a single getProviderAction(status) → connect | reconnect | logout | null. Fixes "Active" badge + "Connect" button showing simultaneously when authenticated via API key. * fix(desktop): always show Logout when provider is active Active providers now always show a Logout button. Clears OAuth or API key depending on authMethod — no more "Active" badge with no way to disconnect. * fix(desktop): simplify OpenAI OAuth dialog + auto-open browser Match Anthropic dialog's layout: remove the raw OAuth URL display and "Tip" block, auto-open the browser on OAuth start. Change "Back" to "Cancel" for consistency. * refactor(desktop): unify OAuth dialogs into shared OAuthDialog Extract shared OAuthDialog component with provider config object. AnthropicOAuthDialog and OpenAIOAuthDialog become thin wrappers that pass provider-specific labels and options. * fix(desktop): show 'Copied!' feedback on Copy URL button * refactor(desktop): merge provider account + API key into single card Each provider section now renders AccountCard + ConfigRow inside one rounded card with a divider, instead of two separate cards. Removes the standalone "API Keys" collapsible section. * refactor(desktop): compact OAuth row in provider settings card OAuth row is now a single inline row (label + status + action) instead of a stacked AccountCard. Both providers share the same 2-row card layout: OAuth row + API key row with divider. * fix(desktop): contextual buttons in provider settings Connect is now primary (filled). Save only shows when there's input. Clear only shows when a key is saved. Removes visual noise from empty-state provider cards. * ui(desktop): add provider icons to settings section headers * ui(desktop): show 'Not connected' badge instead of subtitle for disconnected providers * ui: remove redundant disconnected subtitle * ui: remove subtitle text from OAuth rows * chore: remove dead AccountCard + getProviderSubtitle * docs: update test plan to match current UI * chore: move shipped plans to done/ --------- Co-authored-by: AviPeltz <aj.peltz@gmail.com>
…uperset-sh#3517) * remove 7 day rule * Upgrade mastra * upgrade ai * Ad mastra * refactor(desktop): remove dead provider-diagnostics plumbing The provider-diagnostics store was fed by callSmallModel's per-attempt reporting, which was removed when small-model tasks moved to direct AI-SDK + mastracode's AuthStorage. Nothing writes to the issue map anymore, so the clearIssue mutation, getStatuses query, and diagnosticStatus plumbing in ModelsSettings were all no-ops. Settings still surfaces "Session expired / Reconnect" via auth-status alone. ProviderIssue type collapsed from 8 codes to just "expired" to match. * fix(auth): auto-refresh expired Anthropic OAuth tokens 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. * review: address PR feedback from cubic + coderabbit + greptile - host-service ai-branch-name: run trailing-trim after slice so a 100-char truncation can't re-introduce a bare "." or "-" that git rejects as an invalid ref (coderabbit / cubic #2, #7). - host-service workspace-creation.generateBranchName: reuse the existing listBranchNames helper instead of the inline git walk, which classified off the short refname and could conflate a local "origin/foo" with refs/remotes/origin/foo (coderabbit #3). - packages/chat shared/small-model: drop the unused hasSmallModelCredentials export; only a test mock consumed it (greptile #4). - resolveAnthropicCredential: on refresh failure, return null instead of kind:"oauth" with a stale expiresAt so callers fall back cleanly (cubic #8). - chat-service.getAnthropicAuthStatus: log context when refresh throws instead of silently swallowing (cubic #9). * fix(chat): read auth.json directly instead of importing mastracode Importing createAuthStorage from mastracode loads the entire CLI tree (fastembed → onnxruntime-node's 208 MB native binary) via eager top-level requires in mastracode's CJS entry. This crashed electron-vite bundling and bloated the get-small-model chunk. getSmallModel now reads mastracode's auth.json file directly using the same path resolution logic (~/Library/Application Support/mastracode/ on macOS). Zero mastracode import, zero bundle impact. The chunk stays at 1.2 MB (just @ai-sdk/anthropic + @ai-sdk/openai). Production build verified: compile:app succeeds, Electron main process boots with no onnxruntime error. * docs(desktop): add manual testing plan for PR superset-sh#3517 * fix api key storage slot * fix(auth): store API keys in dedicated slot so OAuth doesn't clobber them setApiKeyForProvider and setStoredAnthropicApiKeyFromEnvVariables now use authStorage.setStoredApiKey() (writes to "apikey:<provider>") instead of authStorage.set() (writes to the main "<provider>" slot shared with OAuth). This way connecting/disconnecting OAuth doesn't overwrite or delete a stored API key. resolveAuthMethodForProvider falls back to hasStoredApiKey() after checking the main slot, so status correctly reports authenticated when only an API key is stored. * fix(auth): backup/restore API keys across OAuth connect/disconnect mastracode's resolveModel only reads API keys from the main authStorage slot (authStorage.get("anthropic")). OAuth login overwrites this slot, and disconnect removes it — losing any previously saved API key. Fix: backup the API key to the dedicated apikey: slot before OAuth connect, restore it after disconnect. setApiKeyForProvider now writes to both slots (main for resolveModel compatibility, apikey: for backup). resolveAuthMethodForProvider checks both. Applies to both Anthropic and OpenAI providers. * chore: add upstream PR reference to auth workaround Point to mastra-ai/mastra#15483 so the backup/restore code can be removed once upstream lands and we bump mastracode. * refactor(desktop): derive settings provider action from status Replace the cascade of if/else + canDisconnect flag with a single getProviderAction(status) → connect | reconnect | logout | null. Fixes "Active" badge + "Connect" button showing simultaneously when authenticated via API key. * fix(desktop): always show Logout when provider is active Active providers now always show a Logout button. Clears OAuth or API key depending on authMethod — no more "Active" badge with no way to disconnect. * fix(desktop): simplify OpenAI OAuth dialog + auto-open browser Match Anthropic dialog's layout: remove the raw OAuth URL display and "Tip" block, auto-open the browser on OAuth start. Change "Back" to "Cancel" for consistency. * refactor(desktop): unify OAuth dialogs into shared OAuthDialog Extract shared OAuthDialog component with provider config object. AnthropicOAuthDialog and OpenAIOAuthDialog become thin wrappers that pass provider-specific labels and options. * fix(desktop): show 'Copied!' feedback on Copy URL button * refactor(desktop): merge provider account + API key into single card Each provider section now renders AccountCard + ConfigRow inside one rounded card with a divider, instead of two separate cards. Removes the standalone "API Keys" collapsible section. * refactor(desktop): compact OAuth row in provider settings card OAuth row is now a single inline row (label + status + action) instead of a stacked AccountCard. Both providers share the same 2-row card layout: OAuth row + API key row with divider. * fix(desktop): contextual buttons in provider settings Connect is now primary (filled). Save only shows when there's input. Clear only shows when a key is saved. Removes visual noise from empty-state provider cards. * ui(desktop): add provider icons to settings section headers * ui(desktop): show 'Not connected' badge instead of subtitle for disconnected providers * ui: remove redundant disconnected subtitle * ui: remove subtitle text from OAuth rows * chore: remove dead AccountCard + getProviderSubtitle * docs: update test plan to match current UI * chore: move shipped plans to done/ --------- Co-authored-by: AviPeltz <aj.peltz@gmail.com>
…tion (mastra-ai#15483) Fixes mastra-ai#15484 `getAnthropicApiKey()` and `getOpenAIApiKey()` only check `authStorage.get()` (main credential slot). They miss keys in `authStorage.getStoredApiKey()` and `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` env vars. This breaks when OAuth overwrites the main slot then gets disconnected — the API key in the dedicated slot is invisible to `resolveModel`, which falls through to OAuth and throws "Not logged in." Both functions now check: main slot → `getStoredApiKey()` → env var. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## ELI5 This PR fixes a problem where API keys for Anthropic and OpenAI would become unavailable in the mastracode application. The fix teaches the app to look for API keys in three places instead of just one: the main settings storage, a dedicated API key storage location, and environment variables. This helps users who save API keys through the Settings UI or set them up in containers/CI environments. ## Changes ### mastracode/src/agents/model.ts Updated `getAnthropicApiKey()` and `getOpenAIApiKey()` to implement a three-level fallback chain for API key resolution: 1. Check the main credential slot (`authStorage.get('anthropic')` / `authStorage.get('openai-codex')`) 2. Check the dedicated stored API key slot via `authStorage.getStoredApiKey(<providerId>)` 3. Fall back to environment variables (`process.env.ANTHROPIC_API_KEY` / `process.env.OPENAI_API_KEY`) All retrieved keys are trimmed before being returned. This prevents `resolveModel` from incorrectly falling back to OAuth authentication after OAuth connect/disconnect cycles that clear the main credential slot. ### .changeset/fix-api-key-resolution.md Added a Changesets entry documenting the patch release for the `mastracode` package with notes on the updated API key resolution behavior. ## Why This Matters - **Persisted API keys**: Keys saved via the Settings UI (using `authStorage.setStoredApiKey()`) are no longer lost after OAuth operations - **Environment variable support**: CI/container/development environments can now provide API keys through standard environment variables - **Prevents authentication errors**: Fixes the "Not logged in" error that occurred when API keys became unavailable after OAuth lifecycle changes <!-- end of auto-generated comment: release notes by coderabbit.ai -->
The vitest config had isolate:false, which let module state, mocks, and process patches leak between test files. That hid a handful of stale assertions and test fixtures that drifted out of sync with the implementations they were exercising. - vitest.config.ts: flip isolate to true so each test file runs in its own worker (matches the project's other suites) - index.test.ts: add the missing observability slot to the mock global settings factory so createMastraCode can read globalSettings.observability - agents/model.test.ts: align getAnthropicApiKey + resolveModel expectations with mastra-ai#15483, which intentionally added env-var fallback for ANTHROPIC_API_KEY - mcp/manager.test.ts: include the 7-day MCP client timeout that the manager now passes to MCPClient All 622 mastracode tests now pass under isolation, including the new multiline-input + ask-question opt-in coverage. Co-Authored-By: Mastra Code (anthropic/claude-opus-4-7) <noreply@mastra.ai>
Fixes #15484
getAnthropicApiKey()andgetOpenAIApiKey()only checkauthStorage.get()(main credential slot). They miss keys inauthStorage.getStoredApiKey()andANTHROPIC_API_KEY/OPENAI_API_KEYenv vars.This breaks when OAuth overwrites the main slot then gets disconnected — the API key in the dedicated slot is invisible to
resolveModel, which falls through to OAuth and throws "Not logged in."Both functions now check: main slot →
getStoredApiKey()→ env var.ELI5
This PR fixes a problem where API keys for Anthropic and OpenAI would become unavailable in the mastracode application. The fix teaches the app to look for API keys in three places instead of just one: the main settings storage, a dedicated API key storage location, and environment variables. This helps users who save API keys through the Settings UI or set them up in containers/CI environments.
Changes
mastracode/src/agents/model.ts
Updated
getAnthropicApiKey()andgetOpenAIApiKey()to implement a three-level fallback chain for API key resolution:authStorage.get('anthropic')/authStorage.get('openai-codex'))authStorage.getStoredApiKey(<providerId>)process.env.ANTHROPIC_API_KEY/process.env.OPENAI_API_KEY)All retrieved keys are trimmed before being returned. This prevents
resolveModelfrom incorrectly falling back to OAuth authentication after OAuth connect/disconnect cycles that clear the main credential slot..changeset/fix-api-key-resolution.md
Added a Changesets entry documenting the patch release for the
mastracodepackage with notes on the updated API key resolution behavior.Why This Matters
authStorage.setStoredApiKey()) are no longer lost after OAuth operations