Skip to content

fix(manage-ui): reuse existing Nango integration for MCP OAuth login#2729

Merged
omar-inkeep merged 1 commit intomainfrom
fix/nango-mcp-integration-reuse
Mar 17, 2026
Merged

fix(manage-ui): reuse existing Nango integration for MCP OAuth login#2729
omar-inkeep merged 1 commit intomainfrom
fix/nango-mcp-integration-reuse

Conversation

@omar-inkeep
Copy link
Copy Markdown
Contributor

Summary

  • Fix regression from feat(manage-ui): improve OAuth credentials CRUD with multi-app support #2708 where MCP OAuth login fails with "Nango integration already exists" when a credential reference and MCP server share the same name
  • The buildMaskedCredentials refactor removed the catch-all areCredentialsSet = true for non-OAUTH/TBA/APP credential types — MCP-generic integrations have no client credentials, so areCredentialsSet became false, causing the code to skip reuse and attempt a duplicate create
  • Fix: check existingIntegration instead of existingIntegration?.areCredentialsSet in createProviderConnectSession

Test plan

  • Create a credential reference named "test" linked to an MCP server
  • Disconnect the credential from the MCP server
  • Click "Login" on the MCP server again — should succeed without "already exists" error
  • Verify fresh MCP server OAuth login still works (no existing integration)
  • Verify provider OAuth (Slack, GitHub) credential creation still works

Made with Cursor

The `buildMaskedCredentials` refactor in #2708 removed the catch-all
`areCredentialsSet = true` for credential types other than OAUTH/TBA/APP.
MCP-generic integrations have no client credentials, so `areCredentialsSet`
became `false`, causing `createProviderConnectSession` to skip reuse and
attempt to create a duplicate integration — triggering "already exists".

Fix: check `existingIntegration` instead of `existingIntegration?.areCredentialsSet`.
Made-with: Cursor
@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Mar 17, 2026

This run croaked 😵

The workflow encountered an error before any progress could be reported. Please check the link below for details.

Pullfrog  | Rerun failed job ➔View workflow run | Triggered by Pullfrogpullfrog.com𝕏

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 17, 2026

🦋 Changeset detected

Latest commit: c164ba5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-manage-ui Patch
@inkeep/agents-api Patch
@inkeep/agents-cli Patch
@inkeep/agents-core Patch
@inkeep/agents-email Patch
@inkeep/agents-mcp Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch

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

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Mar 17, 2026 8:20pm
agents-docs Ready Ready Preview, Comment Mar 17, 2026 8:20pm
agents-manage-ui Ready Ready Preview, Comment Mar 17, 2026 8:20pm

Request Review

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

This is a clean, well-targeted bug fix that correctly addresses a regression from PR #2708.


Analysis

Root Cause Understanding:

The PR description accurately identifies the issue:

  1. PR #2708 introduced buildMaskedCredentials() which determines areCredentialsSet based on credential type
  2. For OAUTH2/TBA providers: areCredentialsSet = !!(client_id && client_secret)
  3. For MCP-generic integrations: credentials is null → returns areCredentialsSet: false at line 114
  4. The old conditional if (existingIntegration?.areCredentialsSet) then failed to reuse existing integrations

Why The Fix Is Correct:

The change from existingIntegration?.areCredentialsSet to existingIntegration is semantically correct because:

  • areCredentialsSet answers: "Does this integration have OAuth client credentials configured?"
  • Integration reuse should answer: "Does an integration with this unique_key already exist?"

These are orthogonal concerns. MCP-generic integrations legitimately have no client credentials (Nango brokers the OAuth flow), but they're still valid integrations that should be reused on reconnection.

Scope Verification:

I investigated the precision concern about whether areCredentialsSet: false might affect other code paths:

  • The /credentials/new/providers/[providerId] page uses disabled={!areCredentialsSet} (line 370)
  • However, this page only renders for providers where requiresCredentialForm(authMode) is true — i.e., OAUTH1, OAUTH2, TBA, APP, CUSTOM
  • MCP-generic uses a completely separate flow via use-oauth-login.tsopenNangoConnectHeadless()
  • No UI regression from this fix

Changeset

✅ Appropriate patch-level changeset included for @inkeep/agents-manage-ui


✅ APPROVE

Summary: This is a minimal, correctly-targeted fix. The one-line change restores the expected behavior for MCP OAuth reconnection without side effects. The test plan in the PR description covers the key scenarios. Ship it! 🚀

Discarded (2)
Location Issue Reason Discarded
nango.ts:418 Symptom patch: should fix buildMaskedCredentials instead Invalid — the fix is semantically correct. areCredentialsSet is about client credentials, not integration reuse. These are orthogonal concerns.
system Code divergence between nango.ts and nango-store.ts for areCredentialsSet Pre-existing divergence not introduced by this PR. The two files serve different purposes (UI vs backend runtime).
Reviewers (2)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-standards 0 0 0 0 0 0 0
pr-review-precision 2 0 0 0 0 0 2
Total 2 0 0 0 0 0 2

@github-actions github-actions Bot deleted a comment from claude Bot Mar 17, 2026
@omar-inkeep omar-inkeep added this pull request to the merge queue Mar 17, 2026
Merged via the queue into main with commit ad867c5 Mar 17, 2026
11 checks passed
@omar-inkeep omar-inkeep deleted the fix/nango-mcp-integration-reuse branch March 17, 2026 21:05
@inkeep
Copy link
Copy Markdown
Contributor

inkeep Bot commented Mar 17, 2026

No documentation updates needed — this is an internal bug fix for Nango integration reuse logic with no user-facing API or behavior changes.

@itoqa
Copy link
Copy Markdown

itoqa Bot commented Mar 17, 2026

Ito Test Report ❌

12 test cases ran. 10 passed, 2 failed.

✅ The run confirms core OAuth, navigation, responsive, and adversarial protections across included scenarios, while code-backed verification found a real concurrency defect in reconnect initiation: repeated rapid clicks can launch multiple OAuth attempts in parallel.

✅ Passed (10)
Test Case Summary Timestamp Screenshot
LOGIC-1 Observed reconnect attempts and console output without duplicate-create or already-exists error strings. 9:49 LOGIC-1_9-49.png
LOGIC-2 The flow produced explicit required-field validation and explicit recoverable error messaging without a silent broken state. 35:35 LOGIC-2_35-35.png
LOGIC-3 Reconnect on overlapping-name Notion fixtures showed no duplicate-create collision signature. 9:49 LOGIC-3_9-49.png
EDGE-3 Refresh during in-flight login still allowed immediate retry and usable login control. 38:27 EDGE-3_38-27.png
EDGE-4 Back/forward around login initiation did not break reconnect controls. 38:52 EDGE-4_38-52.png
EDGE-5 Mobile viewport retained visible and interactive reconnect control. 39:07 EDGE-5_39-07.png
EDGE-6 Two-tab reconnect attempt stayed aligned without contradictory UI state. 40:03 EDGE-6_40-03.png
ADV-1 Unauthorized tenant deep-link was denied with project-not-found behavior. 41:58 ADV-1_41-58.png
ADV-2 Custom MCP form rejected javascript/data URL payloads. 42:33 ADV-2_42-33.png
ADV-3 Script-like MCP name rendered as inert text and reconnect initiation remained functional. 44:28 ADV-3_44-28.png
❌ Failed (2)
Test Case Summary Timestamp Screenshot
EDGE-1 Double-clicking the login control spawned two OAuth windows, showing non-idempotent reconnect initiation. 37:43 EDGE-1_37-43.png
EDGE-2 High-frequency login clicks created many concurrent OAuth windows, indicating missing throttling/debouncing. 38:00 EDGE-2_38-00.png
Rapid double-click on login control – Failed
  • Where: MCP server detail page status badge (Click to Login) for a needs_auth tool.

  • Steps to reproduce: Open a needs_auth MCP tool detail page, rapidly double-click Click to Login.

  • What failed: Multiple OAuth windows can start from rapid repeated clicks; expected behavior is a single in-flight auth attempt.

  • Code analysis: I reviewed the project-scope detail UI and OAuth login hook. The clickable status badge triggers handleOAuthLogin without any in-flight guard or disabled state, and the OAuth handler does not debounce/throttle repeated invocations before starting auth.

  • Relevant code:

    agents-manage-ui/src/components/mcp-servers/view-mcp-server-details-project-scope.tsx (lines 101-118)

    {tool.status === 'needs_auth' ? (
      <Badge
        variant="outline"
        className="... cursor-pointer ..."
        onClick={(e) => {
          e.preventDefault();
          e.stopPropagation();
          handleOAuthLogin({
            toolId: tool.id,
            mcpServerUrl: tool.config.mcp.server.url,
            toolName: tool.name,

    agents-manage-ui/src/hooks/use-oauth-login.ts (lines 213-251)

    const handleOAuthLogin = useCallback(
      async ({ toolId, mcpServerUrl, toolName, thirdPartyConnectAccountUrl, credentialScope }) => {
        if (thirdPartyConnectAccountUrl) {
          await handleOAuthLoginManually(toolId, thirdPartyConnectAccountUrl);
          return;
        }
    
        const credentialStoresStatus = await listCredentialStores(tenantId, projectId);
        const isNangoReady = credentialStoresStatus.some(
          (store) => store.type === CredentialStoreType.nango && store.available
        );

    agents-manage-ui/src/hooks/use-oauth-login.ts (lines 239-245)

    if (isNangoReady) {
      await handleOAuthLoginWithNangoMCPGeneric({
        toolId,
        mcpServerUrl,
        toolName,
        credentialScope,
      });
    }
  • Why this is likely a bug: The production click path starts OAuth without any single-flight protection, so rapid user input can deterministically launch duplicate concurrent auth attempts.

  • Introduced by this PR: No - pre-existing bug (code not changed in this PR).

  • Timestamp: 37:43

Repeated login spam during loading – Failed
  • Where: Same MCP detail reconnect control used for OAuth initiation.

  • Steps to reproduce: On a needs_auth MCP tool, click Click to Login repeatedly (10+ times) before the first attempt settles.

  • What failed: Many OAuth windows/tabs can be created concurrently; expected behavior is throttled or disabled initiation while one auth flow is active.

  • Code analysis: The UI does not lock the reconnect control after first click, and the handler path repeatedly calls into auth start logic (handleOAuthLoginWithNangoMCPGeneric) with no per-tool mutex/debounce.

  • Relevant code:

    agents-manage-ui/src/components/mcp-servers/view-mcp-server-details-project-scope.tsx (lines 102-117)

    <Badge
      variant="outline"
      className="... cursor-pointer ..."
      onClick={(e) => {
        e.preventDefault();
        e.stopPropagation();
        handleOAuthLogin({
          toolId: tool.id,
          mcpServerUrl: tool.config.mcp.server.url,

    agents-manage-ui/src/hooks/use-oauth-login.ts (lines 146-163)

    const handleOAuthLoginWithNangoMCPGeneric = useCallback(
      async ({ toolId, mcpServerUrl, toolName, credentialScope }) => {
        const authResult = await openNangoConnectHeadless({
          mcpServerUrl,
          providerUniqueKey: `${generateIdFromName(toolName)}_${toolId.slice(0, 4)}`,
          providerDisplayName: toolName,
        });

    agents-manage-ui/src/hooks/use-oauth-login.ts (lines 239-247)

    if (isNangoReady) {
      await handleOAuthLoginWithNangoMCPGeneric({
        toolId,
        mcpServerUrl,
        toolName,
        credentialScope,
      });
    }
  • Why this is likely a bug: The reconnect path allows unbounded repeated invocations before completion, which is a production logic gap in idempotency/rate-limiting for auth initiation.

  • Introduced by this PR: No - pre-existing bug (code not changed in this PR).

  • Timestamp: 38:00

📋 View Recording

Screen Recording

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant