Skip to content

feat(platform): support static API-key/bearer-token auth for MCP servers - #13683

Open
Abhi1992002 wants to merge 14 commits into
devfrom
req-115
Open

feat(platform): support static API-key/bearer-token auth for MCP servers#13683
Abhi1992002 wants to merge 14 commits into
devfrom
req-115

Conversation

@Abhi1992002

@Abhi1992002 Abhi1992002 commented Jul 27, 2026

Copy link
Copy Markdown
Member

Why / What / How

Why: Some MCP servers (e.g. DataFast) authenticate with a static bearer token / API key issued in the vendor's own dashboard, not a full OAuth2 authorize/token exchange. The MCP tool block only advertised oauth2 credentials, and — critically — the graph builder's "Connect to MCP Server" dialog used a manually-entered token only for tool discovery and never persisted it. The placed block therefore ran with no credentials and failed with a 401 at execution time. (Linear: REQ-115)

What: Make static API-key / bearer-token a first-class credential type for MCP server connections, entered via the builder's secure credential dialog, and stored as a proper api_key credential the block uses at runtime.

How:

  • MCPToolBlock now advertises both oauth2 and api_key credential types and accepts either at runtime — the bearer token is pulled from access_token (OAuth2) or api_key (API key) via a single shared mcp_auth_token() helper.
  • POST /api/v2/mcp/token now stores the token as a first-class APIKeyCredentials (type api_key) instead of masquerading it as an OAuth2Credentials.
  • Backward compatible: credential lookup, cleanup, and get_host now match both oauth2 and api_key MCP credentials by server URL, so tokens already stored under the old (OAuth2-masquerade) shape keep working.
  • The builder MCP dialog now offers proactive "Use an API key / bearer token instead" entry (no failed-OAuth round-trip required), persists the token via /mcp/token, and attaches the returned credential to the block node.
  • The frontend credential classifier matches api_key MCP credentials by host, same as OAuth2.

The copilot (run_mcp_tool + MCPSetupCard) bearer-token flow already worked; this PR reuses the same /token endpoint and brings the builder to parity.

Changes 🏗️

Backend

  • blocks/mcp/block.py — widen MCPCredentials to Literal["oauth2", "api_key"]; run() accepts OAuth2Credentials | APIKeyCredentials; token extracted via mcp_auth_token().
  • blocks/mcp/helpers.py — add mcp_auth_token(), is_mcp_credential_for_server(), MCPCredential union; auto_lookup_mcp_credential() matches both credential types (OAuth2-only refresh_if_needed still guarded).
  • api/features/mcp/routes.py/token stores APIKeyCredentials; discovery token extraction and old-credential cleanup (both /token and OAuth callback) cover both types.
  • api/features/integrations/router.pyget_host() returns the MCP server URL for APIKeyCredentials too.
  • copilot/tools/run_mcp_tool.py — token extraction via the shared helper.

Frontend

  • build/components/MCPToolDialog.tsx — proactive API-key entry; persist the token via /mcp/token on successful discovery and attach the credential to the node.
  • hooks/useCredentials.tsclassifyCredentials matches api_key MCP credentials by host.

Tests — backend: helper token extraction + both-type matching, block run() with APIKeyCredentials, /token stores api_key + cleans up legacy OAuth2 and api_key rows. Frontend: dialog persists+attaches the token (and does not for public servers), classifier matches api_key MCP creds.

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • poetry run pytest backend/blocks/mcp backend/api/features/mcp/test_routes.py backend/copilot/tools/test_run_mcp_tool.py — all pass
    • pnpm test:unit for MCPToolDialog.test.tsx and classifyCredentials.test.ts — all pass
    • poetry run format, poetry run lint, pnpm lint, pnpm types — clean
    • Manual: in the builder, add the MCP Tool block, enter a bearer-token-only server URL, choose "Use an API key / bearer token instead", paste a token, add the block, and run it — the tool executes (no 401)

Update: rebased on dev + review round addressed

Merged latest dev (3 conflicts: useCredentials.ts, CredentialField.tsx, CredentialsInput.test.tsx) and addressed all 10 open review threads.

Behaviour changes worth a second look:

  • MCPToolBlock.run auto-lookup fallback is back. executor/manager.py nulls the credentials field for three indistinguishable reasons — the user picked "None", the node never had one, and the node's credential ID points at a row that has since been deleted. Reconnecting a server deletes its old credential and issues a new ID, so that third case is an ordinary flow; without the fallback every run after a reconnect goes out unauthenticated and 401s. The fallback can override an explicit "None" on a server the user also has a token for, which is the far milder failure. The test that locked in the old behaviour was inverted rather than deleted.
  • _mcp_credential_rank now ranks on usability, not expiry. Treating a non-expiring credential as infinitely-far-in-the-future made a static key permanently outrank a freshly obtained OAuth token, so re-authenticating could never take effect (the generic POST /{provider}/credentials endpoint does no per-server cleanup, so the two coexist through a normal flow). Rank is now 1 = still authenticates / 0 = lapsed, with store order breaking ties.
  • mcp_oauth_callback creates before deleting, mirroring mcp_store_token — a failed create no longer leaves the user with no credential at all.
  • New mcpStoreToken on the credentials provider. Storing a token deletes the server's previous credential, so the builder dialog now goes through the provider (like the OAuth path) and replaceMCPServerCredentials evicts the replaced rows from the cached list. Without this the picker kept re-selecting a deleted ID.
  • Cross-server token leak fixed. manualToken was left loaded (and hidden) after a successful connect, so "Back" → new URL sent server A's secret to server B and persisted it as B's credential. Cleared on store and on "Back".
  • The merge surfaced a second broken path. dev's new ConnectCredentialDialog (variant="default" — run dialogs, copilot) creates API keys via useApiKeyConnectFormpostV1CreateCredentials with no metadata, which for MCP produces a host: null credential that nothing can match. It now threads mcp_server_url through. The builder (variant="node") still uses the per-type flow.

Also: normalized MCP URL matching in copilot/tools/utils.py and CredentialsGroupedView/helpers.ts, blocked API-key submission when the node has no server_url, Record<string, unknown> over any, and hoisted function-local test imports.

Verified locally: pytest backend/blocks/mcp backend/copilot/tools/utils_test.py backend/api/features/mcp → 122 passed; pnpm test:unit → 6007 passed (562 files); pnpm types, pnpm lint, poetry run format, and pyright on the changed backend dirs all clean.

@Abhi1992002
Abhi1992002 requested a review from a team as a code owner July 27, 2026 08:22
@Abhi1992002
Abhi1992002 requested review from 0ubbe and kcze and removed request for a team July 27, 2026 08:22
@github-project-automation github-project-automation Bot moved this to 🆕 Needs initial review in AutoGPT development kanban Jul 27, 2026
@github-actions github-actions Bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end platform/blocks labels Jul 27, 2026
@Abhi1992002

Copy link
Copy Markdown
Member Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #13683 at e49151e.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

MCP credential handling now supports OAuth2 and API-key bearer credentials across backend storage, lookup, tool execution, copilot utilities, and frontend discovery. Manual tokens are persisted as API-key credentials and matched by normalized MCP server URL.

Changes

MCP credential abstraction

Layer / File(s) Summary
Shared credential abstraction
autogpt_platform/backend/backend/blocks/mcp/*, autogpt_platform/backend/backend/copilot/tools/*
MCP helpers and consumers extract tokens, match servers, select credentials, and execute tools with OAuth2 or API-key credentials.
Credential matching and behavior validation
autogpt_platform/backend/backend/blocks/mcp/test_*, autogpt_platform/backend/backend/copilot/tools/*_test.py, autogpt_platform/frontend/src/hooks/*, autogpt_platform/frontend/src/components/contextual/...
Tests cover token extraction, URL normalization, API-key execution, credential classification, and host-specific lookup.

Backend credential storage

Layer / File(s) Summary
Backend storage and replacement
autogpt_platform/backend/backend/api/features/mcp/*, autogpt_platform/backend/backend/api/features/integrations/router.py
MCP routes store manual tokens as APIKeyCredentials, replace matching credentials, and return standardized metadata.

Frontend manual-token flow

Layer / File(s) Summary
Frontend manual-token flow
autogpt_platform/frontend/src/app/(platform)/build/components/*
The dialog supports manual bearer tokens during discovery, persists accepted tokens, reports rejected tokens without OAuth escalation, and validates token-storage failures.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPToolDialog
  participant mcp_store_token
  participant CredentialsStore
  participant MCPToolBlock
  MCPToolDialog->>mcp_store_token: submit bearer token and server URL
  mcp_store_token->>CredentialsStore: store APIKeyCredentials
  CredentialsStore-->>mcp_store_token: return credential metadata
  MCPToolDialog->>MCPToolBlock: confirm tool with credential metadata
  MCPToolBlock->>CredentialsStore: resolve MCP credential
  CredentialsStore-->>MCPToolBlock: return API-key or OAuth2 credential
Loading

Suggested reviewers: 0ubbe, kcze

Poem

A rabbit hops with tokens bright,
OAuth and keys now share the night.
Servers match by URL’s trail,
Tools receive the proper hail.
“Connect!” the carrot banners say.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description clearly explains the static API-key/bearer-token support for MCP servers, the affected backend and frontend components, backward compatibility, tests, and verification results.
Title check ✅ Passed The title clearly and concisely summarizes the primary change: adding static API-key/bearer-token authentication support for MCP servers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch req-115

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
autogpt_platform/frontend/src/hooks/useCredentials.ts (1)

30-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize MCP URLs before matching credentials.

mcp_store_token persists a normalized URL, while the block discriminator can retain a trailing slash. Exact comparison then hides a valid saved credential for https://server/mcp/. Normalize both values before comparing and add a trailing-slash test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@autogpt_platform/frontend/src/hooks/useCredentials.ts` around lines 30 - 36,
Update the MCP credential matching logic in the credentials hook to normalize
both c.host and discriminatorValue before comparison, removing trailing slashes
so equivalent URLs match. Preserve the existing null guard and credential
collection behavior, and add a test covering a discriminator URL with a trailing
slash matching the normalized saved URL.
autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx (1)

143-164: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use unknown for the caught error.

Replace catch (e: any) with catch (e: unknown) and narrow before reading status, message, or detail; any disables type checking and conflicts with the frontend guideline to avoid any.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/MCPToolDialog.tsx
around lines 143 - 164, Update the catch block in the MCP tool connection flow
to use catch (e: unknown) instead of any. Narrow e before accessing status,
message, or detail, while preserving the existing 401/403 authentication
handling and fallback error message behavior.

Source: Coding guidelines

🧹 Nitpick comments (1)
autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx (1)

340-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the manual dark-mode overrides.

These controls use dark: classes even though the design system owns dark-mode styling. Use semantic design-system styling instead.

As per coding guidelines, “No dark: Tailwind classes — the design system handles dark mode.”

Also applies to: 355-363

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/MCPToolDialog.tsx
around lines 340 - 345, Remove the dark: Tailwind classes from the manual token
toggle button and the related controls near it, including dark:text-gray-400 and
dark:hover:text-gray-300. Preserve the existing semantic text, underline, hover,
and layout styling while relying on the design system for dark-mode behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@autogpt_platform/backend/backend/api/features/mcp/routes.py`:
- Around line 369-377: Make MCP credential replacement atomic in routes.py at
lines 369-377 and 443-473: update both OAuth and static-token flows to use the
same store-level atomic replace/create operation, passing the server-matching
criteria and new credential together. Remove the separate read/delete or
ID-collection steps so concurrent submissions cannot retain duplicate
credentials.

In `@autogpt_platform/backend/backend/blocks/mcp/helpers.py`:
- Around line 163-177: Update the credential selection loop around
_mcp_credential_expiry so expired credentials cannot outrank valid non-expiring
credentials. Rank matching credentials by not-expired status first, then use the
existing iteration-order recency tiebreaker; preserve the subsequent
refresh_if_needed handling for the selected OAuth2Credentials.

---

Outside diff comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/MCPToolDialog.tsx:
- Around line 143-164: Update the catch block in the MCP tool connection flow to
use catch (e: unknown) instead of any. Narrow e before accessing status,
message, or detail, while preserving the existing 401/403 authentication
handling and fallback error message behavior.

In `@autogpt_platform/frontend/src/hooks/useCredentials.ts`:
- Around line 30-36: Update the MCP credential matching logic in the credentials
hook to normalize both c.host and discriminatorValue before comparison, removing
trailing slashes so equivalent URLs match. Preserve the existing null guard and
credential collection behavior, and add a test covering a discriminator URL with
a trailing slash matching the normalized saved URL.

---

Nitpick comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/MCPToolDialog.tsx:
- Around line 340-345: Remove the dark: Tailwind classes from the manual token
toggle button and the related controls near it, including dark:text-gray-400 and
dark:hover:text-gray-300. Preserve the existing semantic text, underline, hover,
and layout styling while relying on the design system for dark-mode behavior.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 46c793da-9a65-4d8b-886d-4c79fa2b85f4

📥 Commits

Reviewing files that changed from the base of the PR and between 6ccfa17 and e49151e.

📒 Files selected for processing (14)
  • autogpt_platform/backend/backend/api/features/integrations/router.py
  • autogpt_platform/backend/backend/api/features/mcp/routes.py
  • autogpt_platform/backend/backend/api/features/mcp/test_routes.py
  • autogpt_platform/backend/backend/blocks/mcp/block.py
  • autogpt_platform/backend/backend/blocks/mcp/helpers.py
  • autogpt_platform/backend/backend/blocks/mcp/test_helpers.py
  • autogpt_platform/backend/backend/blocks/mcp/test_mcp.py
  • autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py
  • autogpt_platform/backend/backend/copilot/tools/utils.py
  • autogpt_platform/backend/backend/copilot/tools/utils_test.py
  • autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx
  • autogpt_platform/frontend/src/app/(platform)/build/components/__tests__/MCPToolDialog.test.tsx
  • autogpt_platform/frontend/src/hooks/__tests__/classifyCredentials.test.ts
  • autogpt_platform/frontend/src/hooks/useCredentials.ts

Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py Outdated
Comment thread autogpt_platform/backend/backend/blocks/mcp/helpers.py
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.58249% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.38%. Comparing base (8cf6787) to head (4110d0e).
⚠️ Report is 15 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #13683      +/-   ##
==========================================
+ Coverage   80.30%   80.38%   +0.08%     
==========================================
  Files        3333     3333              
  Lines      253534   253775     +241     
  Branches    23531    23554      +23     
==========================================
+ Hits       203593   203997     +404     
+ Misses      44721    44473     -248     
- Partials     5220     5305      +85     
Flag Coverage Δ
platform-backend 85.55% <94.55%> (+0.01%) ⬆️
platform-frontend 58.46% <88.04%> (+0.51%) ⬆️
platform-frontend-e2e 28.43% <2.08%> (-0.40%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Platform Backend 85.55% <94.55%> (+0.01%) ⬆️
Platform Frontend 60.83% <85.26%> (+0.43%) ⬆️
AutoGPT Libs ∅ <ø> (∅)
Classic AutoGPT 28.43% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread autogpt_platform/frontend/src/hooks/useCredentials.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts`:
- Around line 8-10: Rename the apiKeyCred helper to APIKeyCred and update every
call site in helpers.test.ts to use the capitalized acronym, preserving its
implementation and behavior.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a03ee85b-0483-4cfe-b964-1bf022b87663

📥 Commits

Reviewing files that changed from the base of the PR and between 38f99ac and 28d2394.

📒 Files selected for processing (5)
  • autogpt_platform/backend/backend/blocks/mcp/helpers.py
  • autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx
  • autogpt_platform/frontend/src/app/(platform)/build/components/__tests__/MCPToolDialog.test.tsx
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • autogpt_platform/frontend/src/app/(platform)/build/components/tests/MCPToolDialog.test.tsx
  • autogpt_platform/backend/backend/blocks/mcp/helpers.py
  • autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
  • GitHub Check: integration_test
  • GitHub Check: lint
  • GitHub Check: check API types
  • GitHub Check: Seer Code Review
  • GitHub Check: lint
  • GitHub Check: types
  • GitHub Check: test (3.12)
  • GitHub Check: type-check (3.13)
  • GitHub Check: test (3.13)
  • GitHub Check: type-check (3.12)
  • GitHub Check: type-check (3.11)
  • GitHub Check: test (3.11)
  • GitHub Check: lint
  • GitHub Check: end-to-end tests
  • GitHub Check: Analyze (python)
  • GitHub Check: Check PR Status
  • GitHub Check: check-docs-sync
  • GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (13)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development

Format frontend code using pnpm format

autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Fully capitalize acronyms in symbols, e.g. graphID, useBackendAPI
No linter suppressors (// @ts-ignore``, // eslint-disable) — fix the actual issue

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/**/*.{tsx,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/__generated__/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/
'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development

autogpt_platform/frontend/**/*.{ts,tsx}: Use function declarations (not arrow functions) for components/handlers
No any types unless the value genuinely can be anything
Keep render functions and hooks under ~50 lines; extract named helpers or sub-components when they grow longer

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/src/components/**/*.{tsx,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Structure React components as: ComponentName/ComponentName.tsx + useComponentName.ts + helpers.ts (exception: small 3-4 line components can be inline; render-only components can be direct files)

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Structure components as ComponentName/ComponentName.tsx + useComponentName.ts + helpers.ts, use design system components from src/components/ (atoms, molecules, organisms), and never use src/components/__legacy__/*

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

autogpt_platform/frontend/src/**/*.{ts,tsx}: Use generated API hooks from @/app/api/__generated__/endpoints/ following the pattern use{Method}{Version}{OperationName}, and regenerate with pnpm generate:api
Separate render logic from business logic using component.tsx + useComponent.ts + helpers.ts pattern, colocate state when possible and avoid creating large components, use sub-components in local /components folder
Use function declarations for components and handlers, use arrow functions only for callbacks
Do not use useCallback or useMemo unless asked to optimise a given function

autogpt_platform/frontend/src/**/*.{ts,tsx}: Keep files under ~200 lines; extract sub-components or hooks into their own files when a file grows beyond this
Use generated API hooks from @/app/api/__generated__/endpoints/ with pattern use{Method}{Version}{OperationName}
Always import the -Icon-suffixed alias from @phosphor-icons/react (e.g. TrashIcon, PlusIcon, SquareIcon) — bare exports are deprecated
Do not use useCallback or useMemo unless asked to optimize a given function
Never use src/components/__legacy__/* — use design system components from src/components/

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

No barrel files or index.ts re-exports in the frontend

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/src/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not type hook returns, let Typescript infer as much as possible

autogpt_platform/frontend/src/**/*.ts: Extract component logic into custom hooks grouped by concern, not by component, with each hook in its own .ts file
Do not type hook returns; let TypeScript infer as much as possible

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never type with any, if no types available use unknown

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

autogpt_platform/frontend/**/*.{test,spec}.{ts,tsx}: Use Vitest + RTL + MSW for integration tests as the primary testing approach (~90%, page-level), use Playwright for E2E critical flows, and use Storybook for design system components
Run frontend integration tests with pnpm test:unit (Vitest + RTL + MSW)

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
autogpt_platform/frontend/src/**/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)

Structure components as ComponentName/ComponentName.tsx + useComponentName.ts + helpers.ts

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
autogpt_platform/frontend/src/**/__tests__/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)

Use Orval-generated MSW handlers from @/app/api/__generated__/endpoints/{tag}/{tag}.msw.ts for API mocking

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
autogpt_platform/frontend/src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (autogpt_platform/frontend/AGENTS.md)

Avoid index and barrel files

Files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
🧠 Learnings (4)
📚 Learning: 2026-04-01T18:54:16.035Z
Learnt from: Bentlybro
Repo: Significant-Gravitas/AutoGPT PR: 12633
File: autogpt_platform/frontend/src/app/(platform)/library/components/AgentFilterMenu/AgentFilterMenu.tsx:3-10
Timestamp: 2026-04-01T18:54:16.035Z
Learning: In the frontend, the legacy Select component at `@/components/__legacy__/ui/select` is an intentional, codebase-wide visual-consistency pattern. During code reviews, do not flag or block PRs merely for continuing to use this legacy Select. If a migration to the newer design-system Select is desired, bundle it into a single dedicated cleanup/migration PR that updates all Select usages together (e.g., avoid piecemeal replacements).

Applied to files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
📚 Learning: 2026-04-07T09:24:16.582Z
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12686
File: autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/__tests__/PainPointsStep.test.tsx:1-19
Timestamp: 2026-04-07T09:24:16.582Z
Learning: In Significant-Gravitas/AutoGPT’s `autogpt_platform/frontend` (Vite + `vitejs/plugin-react` with the automatic JSX transform), do not flag usages of React types/components (e.g., `React.ReactNode`) in `.ts`/`.tsx` files as missing `React` imports. Since the React namespace is made available by the project’s TS/Vite setup, an explicit `import React from 'react'` or `import type { ReactNode } ...` is not required; only treat it as missing if typechecking (e.g., `pnpm types`) would actually fail.

Applied to files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
📚 Learning: 2026-04-02T05:43:49.128Z
Learnt from: 0ubbe
Repo: Significant-Gravitas/AutoGPT PR: 12640
File: autogpt_platform/frontend/src/app/(no-navbar)/onboarding/steps/WelcomeStep.tsx:13-13
Timestamp: 2026-04-02T05:43:49.128Z
Learning: Do not flag `import { Question } from "phosphor-icons/react"` as an invalid import. `Question` is a valid named export from `phosphor-icons/react` (as reflected in the package’s generated `.d.ts` files and re-exports via `dist/index.d.ts`), so it should be treated as a supported named export during code reviews.

Applied to files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts
📚 Learning: 2026-04-20T20:07:22.981Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11235
File: autogpt_platform/frontend/src/app/(platform)/admin/diagnostics/__tests__/ExecutionsTable.test.tsx:27-76
Timestamp: 2026-04-20T20:07:22.981Z
Learning: In this codebase, Orval-generated API modules under `src/app/api/__generated__/` are not committed to git and must be generated via `pnpm generate:api` (requires a running backend). In integration tests, it’s acceptable—and expected—to stub generated hooks/modules by mocking them with `vi.mock("`@/app/api/__generated__/endpoints/`{tag}/{tag}")`. Do not treat `vi.mock` of these generated hook modules as a violation of the MSW handler guideline, since the corresponding MSW handlers cannot be imported at test time when generated files are absent.

Applied to files:

  • autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts
🔇 Additional comments (2)
autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/__tests__/helpers.test.ts (1)

1-6: LGTM!

Also applies to: 12-47

autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts (1)

31-37: LGTM!

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Automated Review — PR #13683

PR #13683 — feat(platform): support static API-key/bearer-token auth for MCP servers
Author: Abhi1992002 | Files: 14

🎯 Verdict: REQUEST_CHANGES

PR Description Quality

✅ Has Why (placed MCP blocks ran with no credential → 401) + What (first-class api_key/bearer-token credential) + How (proactive dialog entry → /mcp/token persist → attach to node). One gap: the end-to-end manual test in the PR checklist is left unchecked — the exact "no 401 at runtime" journey the PR exists to fix is unconfirmed by the author.

What This PR Does

Previously, static bearer/API-key tokens for MCP servers were shoehorned into the OAuth2 credential model and effectively discarded, so a placed MCP block ran with no usable credential and failed with a 401. This PR makes static tokens a first-class APIKeyCredentials type: the builder dialog now surfaces "Use an API key / bearer token instead" up front, persists the token via /api/v2/mcp/token on successful discovery, and attaches the returned credential to the node. Shared helpers (mcp_auth_token, is_mcp_credential_for_server) centralize token extraction and cross-server matching, with dual-direction cleanup preserving backward compatibility for legacy OAuth2-masqueraded tokens.

Specialist Findings

🛡️ Security ✅ — Traced every credential path: SSRF validation (validate_url_host) runs before any lookup/outbound call, both endpoints require get_user_id, to_meta_response never serializes api_key/access_token, and cross-server scoping compares the full normalized URL (verified by ...rejects_other_server). Two low secret-hygiene notes only.
🟡 normalize_mcp_url isn't case-insensitive → stale token can survive cleanup (helpers.py:29).

🏗️ Architecture ✅ — Storing static tokens as APIKeyCredentials instead of masquerading as OAuth2 is the correct model fix; helper extraction is DRY and introduces no circular deps. Sound migration-free backward-compat strategy.
🟠 Mixed-type selection ranks by raw expiry magnitude, so an expired OAuth2 row (timestamp > 0) can outrank a valid non-expiring api_key (expiry 0) for the same server (helpers.py:172).

Performance ✅ — No regressions. New code is O(n) in a user's MCP credential count (single/low-double digits realistically). Sequential per-credential delete loop (routes.py:470) and per-execution full-credential scan (helpers.py:161) are pre-existing and bounded.

🧪 Testing ⚠️ — Test quality is genuinely strong (specific value assertions, negative cases, mockOAuthLogin not-called checks — not slop). But the riskiest new logic — the api_key-vs-oauth2 branching inside auto_lookup_mcp_credential (OAuth-only refresh guard + expiry selection) — is never directly tested because the function is mocked in every caller.
🟠 No unit test pins the refresh guard or mixed-type "best" selection (helpers.py:173,176).

📖 Quality ✅ — Readability A. Accurate docstrings, good naming, duplication removed (hand-rolled CredentialsMetaResponseto_meta_response). Minor: discovery and token-persist share one try/catch, so a persist failure is misattributed as an invalid-token error (MCPToolDialog.tsx:121).

📦 Product ⚠️ — Core user problem is genuinely solved and well-scoped. Polish gaps: stale error text persists when toggling between token/OAuth modes, token persists as a side effect of discovery (orphaned cred on cancel), and a11y (missing type="button", no role="alert" on the error).

📬 Discussion ⚠️ — GitHub CI green, no merge conflicts, 0 human reviews. Sentry flagged a HIGH parity gap that is unaddressed: the PR fixed the api_key host-classification in useCredentials.ts but missed the sibling classifier CredentialsGroupedView/helpers.ts:32, which still only filters oauth2 MCP creds. A CodeRabbit Major (TOCTOU on credential replace) is also unanswered.
🔴 Second classifier not updated for api_key (CredentialsGroupedView/helpers.ts:32).

🔎 QA ✅ — Verified live: /mcp/token persists a first-class api_key credential (type:"api_key", host:"https://api.github.com/mcp"), replace/cleanup leaves exactly one row, the builder shows the new proactive API-key entry, and negatives return 401/422/400 (incl. SSRF block of 169.254.169.254). Could not reach a real bearer-auth MCP server (no sandbox DNS), so live block runtime and persist-on-200-discovery weren't exercised — both covered by added unit tests.

🔴 Blockers

  1. api_key MCP creds match every server in the grouped credential picker (autogpt_platform/frontend/src/components/contextual/CredentialsInput/components/CredentialsGroupedView/helpers.ts:32) — matchesDiscriminatorValues only host-filters MCP credentials when type === "oauth2"; an api_key MCP credential falls through to return true, matching any server URL. This is the exact wrong-credential/401 bug this PR fixed in useCredentials.ts, left in place in the sibling classifier. Reachable path: a user with two api_key MCP creds for different servers is offered the wrong one when configuring a block. Since this PR introduced the api_key MCP credential type, closing this parity gap belongs in this PR. (Flagged by: discussion — Sentry HIGH)

🟠 Should Fix

  1. Add a direct unit test for auto_lookup_mcp_credential branching (autogpt_platform/backend/backend/blocks/mcp/helpers.py:173,176) — the OAuth-only refresh guard and mixed-type "best" selection are this PR's backward-compat promise and are only ever exercised through mocks. Pin them in test_helpers.py (the cred factory fixtures already exist). (Flagged by: testing)
  2. Validity-aware credential ranking (autogpt_platform/backend/backend/blocks/mcp/helpers.py:172) — rank not-expired before expired instead of by raw expiry magnitude, so an expired OAuth2 row can't shadow a valid non-expiring api_key when cleanup leaves two rows. (Flagged by: architect, discussion — 2 specialists)
  3. Clear stale error on auth-mode toggle (autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx:340,355) — both toggles should setError(null) so a prior "Authentication failed" message doesn't linger over the other flow. (Flagged by: product)
  4. Separate the token-persist try/catch from discovery (autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx:121) — a persistence failure is currently reported as an invalid-token error even though discovery already validated the token. (Flagged by: quality, product — 2 specialists)

🟡 Nice to Have

  1. Persist token on block-confirm rather than on discovery (MCPToolDialog.tsx:124) — avoids orphaned credentials if the user cancels after discovery. Encrypted + user-scoped + cleaned on re-store, so low risk. (Flagged by: security, performance, product — 3 specialists)
  2. Case-insensitive normalize_mcp_url (backend/blocks/mcp/helpers.py:29) — lowercase scheme+host (keep path/query) so differently-cased URLs don't leave a stale live token behind. (Flagged by: security)
  3. Batch the cleanup deletes (backend/api/features/mcp/routes.py:470) — asyncio.gather over old_cred_ids instead of sequential awaits. Bounded and pre-existing. (Flagged by: performance)

🔵 Nits

  1. catch (e: any) (MCPToolDialog.tsx:143) — violates the "never any" guideline; pre-existing context line, cheap cleanup while here.
  2. Add type="button" and role="alert" (MCPToolDialog.tsx:340,355,377) — consistency with MCPToolCard and screen-reader announcement of auth failures.

QA Screenshots

Screenshot Description
build canvas Builder canvas loaded ✅
block menu MCP Tool block in menu ✅
mcp dialog New proactive "Use an API key / bearer token instead" button ✅
token entry Token entry field + "Connect with Token" ✅
after connect Discovery attempt using entered token ✅

Human Review Needed

YES — This change alters how credentials/secrets are stored and how a credential is matched to a server (the security/trust boundary), and the Blocker involves a credential-to-server matching gap; a maintainer should confirm the classifier parity fix before merge.

Risk Assessment

Merge risk: MEDIUM | Rollback: EASY (additive credential type + isolated frontend dialog/classifier changes; no schema migration).

CI Status

GitHub CI (per PR discussion): lint, types, CodeQL, e2e, integration green; test (3.11/3.12/3.13) still running at review time — live status UNVERIFIED from this harness.
Local harness: ✅ frontend lint, ✅ frontend typecheck, ✅ frontend build; ❌ backend poetry run lint and ❌ frontend test:unit failed locally — GitHub reported the lint suite green on this head, so the backend-lint failure is treated as environment skew, not a code defect. The local test:unit failure could not be reconciled against a confirmed green GitHub run and should be checked against live CI before merge.


UI Testing — Variant Results

✅ local: MCP static API-key/bearer-token auth works end-to-end: /token persists a first-class api_key credential with correct host shape, replace/cleanup works, the builder shows the new proactive API-key entry, and negative cases return 401/422/400 correctly.

✅ hosted: MCP static bearer-token auth works end-to-end: /mcp/token persists api_key credentials with correct normalization/cleanup, and the builder dialog's proactive token entry and invalid-token error path behave correctly; all negative tests pass.

@github-project-automation github-project-automation Bot moved this from 🆕 Needs initial review to 🚧 Needs work in AutoGPT development kanban Jul 27, 2026
@Abhi1992002

Copy link
Copy Markdown
Member Author

Thanks — addressing the review. Most items were already fixed in the same commit the review ran against (28d2394); the rest are now pushed. Summary:

🔴 Blocker — api_key MCP creds match every server in the grouped picker (CredentialsGroupedView/helpers.ts:32): already fixed in 28d2394. matchesDiscriminatorValues now host-filters both oauth2 and api_key MCP credentials, mirroring the classifyCredentials fix. A regression test was added (CredentialsGroupedView/__tests__/helpers.test.ts) asserting an api_key cred for one server is not offered for another.

🟠 Should-fix 1 — direct test for auto_lookup_mcp_credential branching: added. test_helpers.py now pins mixed-type "best" selection (non-expiring api_key beats a stale OAuth row), the OAuth-only refresh guard, and no-match → None — no longer only exercised through mocks.

🟠 Should-fix 2 — validity-aware ranking: fixed in 28d2394. Non-expiring credentials now rank highest (sys.maxsize), so an expired OAuth row can't shadow a valid non-expiring api_key.

🟠 Should-fix 3 — clear stale error on auth-mode toggle: fixed. Both toggles now setError(null).

🟠 Should-fix 4 — persist-failure misattributed as invalid-token: fixed in 28d2394. A non-2xx /mcp/token response now throws a distinct "saving your API token failed" message rather than the raw body.

🔵 Nits — a11y: added type="button" to the toggle buttons and role="alert" on the error message.

Deferred (with rationale, noted in the CodeRabbit thread): the TOCTOU/atomic credential-replace is pre-existing (create-before-delete already avoids data loss; duplicates are tolerated by auto_lookup) and a store-level atomic replace is a cross-cutting change out of scope here. Persist-on-confirm vs on-discovery and case-insensitive URL normalization are noted as follow-ups.

@Abhi1992002

Copy link
Copy Markdown
Member Author

/review

@autogpt-pr-reviewer

Copy link
Copy Markdown

Queued a review for PR #13683 at 17ec99e.

Comment thread autogpt_platform/backend/backend/blocks/mcp/helpers.py

@autogpt-pr-reviewer autogpt-pr-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Automated Review — PR #13683

PR #13683 — feat(platform): support static API-key/bearer-token auth for MCP servers
Author: Abhi1992002 | Files: 16

🎯 Verdict: APPROVE

PR Description Quality

✅ Has Why + What + How — the description explains the 401-at-execution bug (builder discarded manually-entered tokens), the fix (persist as a first-class api_key credential), and the backend/frontend mechanics. One gap: the PR checklist's manual end-to-end item is left unchecked, though QA has since exercised that exact flow (see below). Worth noting the /token response contract change (type now api_key, scopes now null) in the description for downstream awareness.

What This PR Does

Previously, when a user pasted a static API key / bearer token for an MCP server in the builder, the token was thrown away and the placed block ran with no credential → 401 at execution. This PR makes the manual token a first-class APIKeyCredentials (replacing an OAuth2Credentials-as-bearer-token masquerade), persists it via POST /mcp/token, and auto-attaches it to the node. Shared helpers (mcp_auth_token, is_mcp_credential_for_server) centralize token extraction and server-matching for both credential shapes, and legacy OAuth2-masqueraded rows keep working with no DB migration.

Specialist Findings

🛡️ Security ✅ — SSRF guards (validate_url_host) on /discover-tools, /oauth/login, and /token including metadata-derived URLs; credentials strictly user-scoped and exact-host matched; tokens stored as SecretStr and not logged. Overall risk LOW.
🟡 Stale/revoked static token can outrank a fresh OAuth token if best-effort cleanup fails (helpers.py:64) — fail-degraded, not a privilege issue.

🏗️ Architecture ✅ — Replacing the OAuth2-masquerade hack with a real APIKeyCredentials type is genuine debt reduction; three duplicated match blocks collapse to one TypeGuard predicate; backward compat achieved purely read-side with no migration.
🟡 mcp/routes.py imports to_meta_response from sibling integrations/router.py (routes.py:16) — feature-to-feature coupling; consider relocating to a neutral module.

Performance ✅ — No new DB round trips on the runtime path; auto_lookup_mcp_credential stays single get_creds_by_provider call + O(n) in-memory filter over one user's (small, bounded) credential set. The extra /token POST is confined to the one-time interactive builder flow.

🧪 Testing ✅ — Strong (~90%), with meaningful negative cases (wrong token → no OAuth bounce, public server → no attach, /token 500 → no block, wrong-server cred untouched) and assertions on the actual token source (api_key vs access_token). The prior "OAuth2-only refresh guard untested" finding is ✅ Addressedtest_helpers.py now asserts it via assert_not_called().
🟠 Documented "most-recently-created wins" tiebreaker for multiple non-expiring creds (helpers.py:189) is uncovered; switching >= to > would regress silently.

📖 Quality ✅ — Readability grade A; descriptive names, comments explain why (ranking rationale, backward-compat intent), to_meta_response dedup is clean. Verified APIKeyCredentials.expires_at exists (model.py:361) so _mcp_credential_rank is safe.
🔵 MCP-credential predicate duplicated in useCredentials.ts:32 and CredentialsGroupedView/helpers.ts:35.

📦 Product ✅ — Feature matches its description, backward compatible, well tested. Terminology consistent, accessibility solid (role="alert" on errors, keyboard submit, real <button> toggles).
🟠 A /token persist failure after successful discovery throws away the discovered tools and strands the user on the URL step (MCPToolDialog.tsx:129) — a valid, working token becomes unusable on a transient save error.

📬 Discussion ⚠️ — 6/6 inline threads resolved; the prior Sentry HIGH classifier bug is ✅ Addressed (fixed in 28d2394 with regression test). Two items need a glance before merge: the CHANGES_REQUESTED bot decision is stale (predates 4 fix commits; re-review was queued but never posted), and CodeRabbit's TOCTOU concern was thread-resolved without an evident code change.
🟠 Non-atomic read→create→delete of MCP credentials (routes.py:443) — concurrent submits could leave duplicate tokens; confirm fixed or acknowledge as won't-fix.

🔎 QA ✅ — Full end-to-end verification against the running stack. Token stored as {"type":"api_key",...} (not OAuth2 masquerade), trailing-slash normalized, re-store cleanup deletes old cred while leaving other servers untouched, and the builder dialog persists + auto-attaches the credential to the placed block (MCP: mcp.deepwiki.com (API Key) selected). Negative cases all hold: no-auth → 401, blank → 422, missing URL → 422, private IP → 400 (SSRF blocked).

🟠 Should Fix

  1. Persist failure discards a successful discovery (MCPToolDialog.tsx:129) — On a non-200 from POST /mcp/token, the code throws before setStep("tool"), dropping the already-fetched tool list even though the token is valid. Keep the discovered tools visible and retry just the save, or let the user proceed and retry persistence. (Flagged by: product, security — 2 specialists)
  2. Tiebreaker for multiple non-expiring credentials untested (helpers.py:189) — The docstring calls the "most-recently-created wins" >= behavior load-bearing (for the failed-cleanup / duplicate-token case), but no test asserts it. Add an auto_lookup_mcp_credential test with two non-expiring api_key creds for the same server. (Flagged by: testing)
  3. Confirm MCP credential replacement race (routes.py:443) — CodeRabbit's TOCTOU finding (non-atomic read→create→delete; concurrent submits can leave duplicate tokens) was thread-resolved with no evident code change. Confirm it was fixed, make the replace atomic, or acknowledge as won't-fix with rationale. (Flagged by: discussion)

🟡 Nice to Have

  1. Ranking tiebreaker on recency/validity (helpers.py:64) — a revoked non-expiring static token can outrank a live OAuth token if best-effort cleanup fails; add a recency tiebreaker or document that single-cred-per-server cleanup is the safety invariant. (security, architect)
  2. Relocate to_meta_response to a neutral credentials module to avoid feature-to-feature router coupling (routes.py:16). (architect)
  3. Route-level test for the api_key discover-tools path (routes.py) — currently only the OAuth2 branch is exercised at the route level. (testing)
  4. Defer credential persistence until block is added (MCPToolDialog.tsx:124) — token is stored server-side on discovery, before commit; closing the dialog mid-flow leaves an orphaned credential (self-heals on next connect). (security, product)

🔵 Nits

  1. Reword persist-failure copy (MCPToolDialog.tsx:130) — "Connected, but saving your API token failed" says Connected when nothing was added; prefer "We reached the server, but couldn't save your token." (product)
  2. catch (e: any) (MCPToolDialog.tsx:143) — violates the repo's no-any guideline; use unknown and narrow. Pre-existing, untouched by this PR. (discussion, quality)
  3. Shared isMcpCredential helper — dedupe the MCP predicate across useCredentials.ts:32 and CredentialsGroupedView/helpers.ts:35. (quality)

QA Screenshots

Screenshot Description
proactive API-key option "Use an API key / bearer token instead" available up-front, no failed-OAuth round-trip needed ✅
token entry Token entry with OAuth toggle + disabled-until-filled "Connect with Token" ✅
tools discovered Discovery succeeded; persisted cred replaced (f5784cdebae2c9ac) ✅
block with credential Block on canvas with MCP: mcp.deepwiki.com (API Key) auto-attached & selected — the bug this PR fixes ✅

Human Review Needed

YES — This PR changes how MCP credentials/secrets are stored and handled (new api_key credential type, token persistence, cleanup semantics), which sits on the credential-storage boundary per the review policy. The security specialist and QA both cleared it, so this is a confirmation ask rather than an unresolved concern.

Risk Assessment

Merge risk: LOW | Rollback: EASY — read-side matching change, no DB migration; reverting restores prior behavior cleanly.

CI Status

Local harness: ✅ frontend lint, ✅ backend lint, ✅ frontend typecheck, ✅ frontend build. ⚠️ frontend test:unit failed in the sandbox (459s) — this suite runs green on the repo's GitHub CI (all codecov flags reported green per the discussion review), so the local failure is treated as environment skew, not a defect, per policy.
GitHub CI: PARTIALLY VERIFIED via discussion review — ~40/44 checks green (lint, types, e2e, CodeQL, Snyk, codecov); backend test (3.11/3.12/3.13) and integration_test were still pending at review time and the size label check fails (size/xl, non-functional). Confirm the backend matrix goes green before merge.


UI Testing — Variant Results

✅ local: End-to-end verification confirms MCP static API-key/bearer-token auth works: the builder dialog persists the token as a first-class api_key credential and attaches it to the placed block, cleanup/host-matching/negative guards all hold.

✅ hosted: MCP static bearer-token auth works end-to-end: /token persists a first-class api_key credential with correct shape and per-server cleanup, negative/SSRF guards hold, and the builder dialog exposes the new proactive token flow correctly.

Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
Comment thread autogpt_platform/backend/backend/blocks/mcp/helpers.py Outdated
Comment thread autogpt_platform/backend/backend/blocks/mcp/helpers.py Outdated
Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py
Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py
Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py
Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
@autogpt-pr-reviewer
autogpt-pr-reviewer Bot dismissed their stale review July 27, 2026 10:16

Superseded by a newer automated review.

@0ubbe 0ubbe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits 💭 💜

Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py Outdated
Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py
Comment thread autogpt_platform/backend/backend/api/features/mcp/routes.py
Comment thread autogpt_platform/backend/backend/blocks/mcp/block.py
Comment thread autogpt_platform/frontend/src/app/(platform)/build/components/MCPToolDialog.tsx Outdated
Comment thread autogpt_platform/frontend/src/hooks/useCredentials.ts
0ubbe
0ubbe previously approved these changes Jul 28, 2026
@github-project-automation github-project-automation Bot moved this from 🚧 Needs work to 👍🏼 Mergeable in AutoGPT development kanban Jul 28, 2026
Abhi1992002 and others added 12 commits September 1, 2026 08:55
…r msg

- CredentialsGroupedView matchesDiscriminatorValues now filters api_key MCP
  credentials by host (like classifyCredentials); previously an api_key cred
  for one server matched every server, risking wrong auto-assignment + 401.
- auto_lookup ranking prefers non-expiring credentials so a valid static
  bearer token is not shadowed by a stale row that once had an expiry.
- MCPToolDialog shows a clear "saving your API token failed" message on a
  non-2xx /token response instead of throwing the raw body.
…a11y on MCP dialog

- Add direct unit tests for auto_lookup_mcp_credential: mixed-type "best"
  selection (non-expiring api_key beats stale OAuth), OAuth-only refresh
  guard, and no-match → None. Previously only exercised via mocks.
- MCP dialog: clear any stale error when toggling between token and OAuth
  entry, add type="button" to the toggles, and role="alert" on the error.
- Add tests: discover-tools with a stored api_key credential, auto_lookup
  recency tiebreaker among equal-rank creds, and TypeGuard rejection of a
  non-OAuth2/non-APIKey (host-scoped) MCP credential.
- MCPToolDialog: type the discovery catch as unknown (drop `any`) and reword
  the token-persist-failure message so it no longer says "Connected".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Five defects made an MCP block unusable end to end.

Frontend:
- MCPToolCard rendered a "Show details" <button> inside the card's own
  <button>, which is invalid HTML and threw a hydration error on the tool
  discovery step. The card is now a div with role="button" and keyboard
  handling.
- Adding an MCP block stamped `credentials_optional: true` on the node.
  That flag means "skip this node if credentials are missing" to the
  executor, so a graph with only an MCP block completed instantly with
  zero nodes run. The block already declares a schema default for
  `credentials`, so the flag was redundant as well as harmful.
  CredentialField now only lets the toggle relax a credential that the
  schema actually requires.
- The credentials picker compared the node's raw `server_url` against the
  normalized URL the backend stores MCP credentials under, so the saved
  credential never appeared. Added `normalizeMCPUrl` (mirror of the
  backend's `normalize_mcp_url`), used both when matching and when the
  dialog emits the node's `server_url`.
- Creating an API key from the picker never tagged it with
  `metadata.mcp_server_url`, so it came back with `host: null`, was
  filtered out of the list, and the selection made on create was
  immediately cleared again.

Backend:
- MCPToolBlock.run fell back to looking up a stored credential by server
  URL whenever none was injected. The executor nulls the field both when
  the user picks "None (skip this credential)" and when nothing was ever
  configured, so the fallback silently overrode an explicit choice. It
  also masked the picker bug above. Removed from the block; the copilot
  and discovery callers keep their own lookups, where no user selection
  exists to override.
…fallback, stop cross-server token reuse, keep the picker cache in sync

- block.py: restore the auto_lookup fallback. The executor nulls the
  credentials field for three different reasons — user picked "None",
  never configured, and "ID points at a deleted row" — and reconnecting a
  server deletes the old credential, so without the fallback every run
  after a reconnect goes out unauthenticated and 401s.
- helpers.py: rank MCP credentials on usability rather than expiry, so a
  static key can no longer permanently outrank a freshly obtained OAuth
  token (the generic credentials endpoint does no per-server cleanup, so
  the two coexist through an ordinary flow).
- routes.py: mirror mcp_store_token's create-before-delete ordering in
  the OAuth callback — a failed create no longer leaves the user with no
  credential at all.
- copilot/tools/utils.py: normalize both sides when matching an MCP
  credential to a node, like is_mcp_credential_for_server does.
- credentials-provider: add mcpStoreToken and evict the server's replaced
  credentials from the cached list; storing a token deletes the previous
  row, so upserting alone left the picker re-selecting a deleted ID.
- MCPToolDialog: clear the manual token once stored and on "Back" — it
  was still loaded, hidden, and would authenticate the next server typed.
- ConnectCredentialDialog: tag MCP API keys with mcp_server_url; the
  default variant's connect flow created credentials with host: null.
- useAPIKeyCredentialsModal: block submission when the node has no
  server_url instead of creating an unmatchable credential.
- CredentialsGroupedView/helpers: normalize MCP host matching.
- types.ts: Record<string, unknown> over any; hoist test-local imports.
… gate

codecov/patch/platform-frontend was at 51.6% against a 70% target.
Covers the changed lines that had no test:

- replaceMCPServerCredentials: replace/keep-others/normalize/no-op cases
- CredentialsProvider.mcpStoreToken and .mcpOAuthCallback rendered for
  real, asserting the cached list drops the row the backend deleted and
  survives a failed store
- MCPToolDialog: the provider-backed store path, and that Back does not
  carry server A's token to server B
- useApiKeyConnectForm: metadata forwarding, expiry conversion, failure
- the MCP api-key guard, plus trailing-slash matching in the grouped view

Also surfaces `formState.errors.root` in the API-key tab, which is where
the builder actually renders the "enter the server URL first" message —
APIKeyCredentialsModal is a separate component the tab flow never mounts.
mcpStoreToken toasted and rethrew, and its only caller then rendered the
same failure inline next to the token field the user has to correct.
The inline message is the useful one, so the provider now just rethrows.
@github-actions github-actions Bot removed the conflicts Automatically applied to PRs with merge conflicts label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions Bot added conflicts Automatically applied to PRs with merge conflicts documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation conflicts Automatically applied to PRs with merge conflicts labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

wr-Jiao pushed a commit to wr-Jiao/AutoGPT that referenced this pull request Sep 7, 2026
…cant-Gravitas#14075)

## Why

AutoGPT currently treats every manually entered MCP credential as a
Bearer token. Some MCP servers, including Langfuse, require HTTP Basic
authentication instead, so users can paste the documented credential but
AutoGPT sends the wrong `Authorization` header.

## What

- Support both `Bearer` and `Basic` authorization values in the shared
MCP HTTP client.
- Keep bare tokens backward-compatible: an unprefixed value is still
sent as Bearer authentication.
- Accept provider documentation in any of these forms:
  - a bare token
  - `Bearer <token>`
  - `Basic <token>`
  - `Authorization: Basic <token>` / `Authorization: Bearer <token>`
- Reject blank values, unsupported complete Authorization headers, and
control characters/header injection.
- Keep the existing `/mcp/token` request shape and generated frontend
client operation unchanged.
- Add a clear Bearer/Basic selector and scheme-specific guidance to each
existing MCP connection UI without combining or redesigning the three
surfaces.
- Persist manually entered credentials from the MCP block dialog and
attach the returned credential ID to the new block, so discovery and
later graph execution use the same credential.

## Backward compatibility

- Existing OAuth credentials continue to be sent as Bearer tokens.
- Existing manually stored bare tokens continue to be sent as Bearer
tokens.
- Existing callers can keep sending `{ server_url, token }` to the same
endpoint.
- No database migration or new required configuration is introduced.

## Validation

- Added backend tests for normalization, Basic header construction,
legacy Bearer behavior, credential persistence, unsupported schemes, and
header-injection rejection.
- Added frontend unit tests for explicit-scheme detection, no Base64
guessing, Basic prefixing, and Bearer compatibility.
- Python compilation and TypeScript parser checks pass for all changed
files.
- A separate critic pass verified backward compatibility, all three UI
surfaces, graph credential persistence, and the security boundary before
this draft PR was opened.

## Agents and large language models used

- Claude Code with Claude Fable 5.1 (commits from `27d39cf` onward:
review-round fixes, single-normalization contract, scope trim)
- Earlier commits by the original author were agent-assisted (the
`github-actions[bot]` review-helper commits); model details unknown

## Checklist

- [x] Based on latest `dev`
- [x] Existing Bearer behavior preserved
- [x] Basic authentication supported end to end
- [x] Three existing GUIs remain separate
- [x] Focused regression tests added
- [x] Full CI green


---

### Update: merged `dev` + addressed the review at `f027a75`

Merged latest `dev` (1 conflict: `McpConnectPanel.tsx` — `dev` changed
`onSuccess` to hand back a `CredentialsMetaResponse` while this branch
added the scheme selector and explicit status checks; resolved by
keeping both).

**Three fixes on top of the merge:**

- **Manual MCP credentials are stored through the credentials
provider.** The builder binds the new node to the ID returned by
`/mcp/token` but resolves that ID against `CredentialsProvidersContext`,
which the direct endpoint call never updated — so a first-run manual
connect rendered *"MCP: … was removed. Choose a connection to keep this
agent running."* on the node it had just configured, until a page
reload. Adds `mcpStoreToken` to the provider, next to
`mcpOAuthCallback`, which already upserts the same way. The
direct-endpoint path stays as a fallback for callers rendered outside
the provider.
- **A stored credential is never re-parsed.** The root cause of the
scheme-flipping bug was normalization running at both the persistence
and the transport boundary: the scheme word of the canonical form became
the first word of a "bare" credential on the second pass.
`normalize_mcp_authorization` now runs exactly once, where a
human-supplied value enters the system; stored credentials go through
`mcp_authorization_header`, which reads the scheme from metadata and
never inspects the secret. A scheme word takes the whole remainder after
it — `value.split(None, 1)` on the backend, the same split in
`mcp-auth.ts` — so `Bearer orgid api-key` means what the user meant by
it. `backend/blocks/mcp/mcp_auth_cases.json` is the shared
accepted/rejected table, asserted by both suites. The UI rejects a Basic
credential containing `:` or whitespace up front, and the backend
rejects it again for direct API callers.
- **The copilot setup card and the integrations connect panel probe the
server before storing.** A 2xx from `/mcp/token` only says the row was
written, so both surfaces reported "Connected" for a credential the
server rejects — reproduced against Langfuse with card=`Connected`,
DB=`status=revoked`. Both now discover first, matching what the builder
dialog already did.


**Verified:** `pnpm vitest run` → 6108 passed (571 files); `pnpm types`,
`pnpm lint`, `pnpm format` clean; `ruff check`/`ruff format` clean on
the changed backend files. Backend pytest was not run locally (the
docker test stack collides with a pre-existing dev-stack container in
this environment) — relying on CI for it.

**Not addressed here, deliberately:** the `DELETE` on a manually stored
MCP credential returning 400 (`missing 'mcp_token_url' metadata`) is a
pre-existing bug on the generic revoke path that already affects manual
Bearer credentials and needs its own fix; the credential picker
labelling a manual Basic credential "(OAuth)" is an artifact of storing
manual credentials as `OAuth2Credentials`, which Significant-Gravitas#13683 changes; the
duplicated `POST` per submit is React StrictMode in the dev container
and the endpoint is idempotent.

---

### Update: scope trimmed at `4551dc1`

The PR had grown to ~4,000 added lines over four bot review rounds, most
of it not Basic auth. This commit removes 747 lines and adds 215:

- **Session close on every MCP path is out.** On `dev`,
`MCPClient.close()` was only ever called on the copilot probe path; the
missing close on the block, discovery and copilot tool paths predates
this PR and has nothing to do with auth schemes. Reverting it also
removes the two new Should Fix items about its timeout and its tests. It
should come back as its own small PR, ideally as an async context
manager on `MCPClient`.
- **`credentials_optional` is untouched again.** The `dev` line in
`Block.tsx` is restored and the test that only guarded its removal is
gone. Last round established that it has no execution effect either way.
- **Narrative comments removed** across backend and frontend, per
`AGENTS.md`. The remaining comments state invariants, not history.
- **Error-extraction helpers are shared** in `lib/mcp-errors.ts` instead
of duplicated between the builder dialog and the integrations panel.
- **Inline backend normalization cases folded into
`mcp_auth_cases.json`**, so one table drives both suites and the
duplicated frontend cases are gone.

Also in this commit: the builder dialog uses the same scheme-specific
placeholder and `normalizeMcpUrl` as the other surfaces, and the
integrations panel compares server identity by origin so editing the
path no longer discards a typed credential.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Krzysztof Czerwinski <kpczerwinski@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: signed CLA signed by all contributors conflicts Automatically applied to PRs with merge conflicts documentation Improvements or additions to documentation platform/backend AutoGPT Platform - Back end platform/blocks platform/frontend AutoGPT Platform - Front end size/xl

Projects

Status: 👍🏼 Mergeable
Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants