fix(manage-ui): reuse existing Nango integration for MCP OAuth login#2729
fix(manage-ui): reuse existing Nango integration for MCP OAuth login#2729omar-inkeep merged 1 commit intomainfrom
Conversation
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
|
This run croaked 😵 The workflow encountered an error before any progress could be reported. Please check the link below for details. |
🦋 Changeset detectedLatest commit: c164ba5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
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:
- PR #2708 introduced
buildMaskedCredentials()which determinesareCredentialsSetbased on credential type - For OAUTH2/TBA providers:
areCredentialsSet = !!(client_id && client_secret) - For MCP-generic integrations:
credentialsisnull→ returnsareCredentialsSet: falseat line 114 - 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:
areCredentialsSetanswers: "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 usesdisabled={!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.ts→openNangoConnectHeadless() - 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 |
|
No documentation updates needed — this is an internal bug fix for Nango integration reuse logic with no user-facing API or behavior changes. |
Summary
buildMaskedCredentialsrefactor removed the catch-allareCredentialsSet = truefor non-OAUTH/TBA/APP credential types — MCP-generic integrations have no client credentials, soareCredentialsSetbecamefalse, causing the code to skip reuse and attempt a duplicate createexistingIntegrationinstead ofexistingIntegration?.areCredentialsSetincreateProviderConnectSessionTest plan
Made with Cursor