fix(mcp): deliver OAuth callback result over BroadcastChannel so COOP can't strand the connect#5767
Conversation
… can't strand the connect An MCP provider whose authorization page sets `Cross-Origin-Opener-Policy: same-origin` (e.g. Gauge) severs `window.opener` when the popup navigates through it, so the callback's `window.opener.postMessage` was silently lost and the row hung on "Connecting…" forever — even when the callback succeeded. - Signal completion over a same-origin `BroadcastChannel` instead, which is origin-scoped and immune to opener severance (the MDN/Chrome-recommended COOP workaround). Scope the message by workspaceId so other open workspaces ignore it. - Log every OAuth callback failure with its reason + serverId. The early-return gates previously returned a silent `ok:false` popup close, so a failed authorization was undiagnosable from the server logs.
PR SummaryMedium Risk Overview The callback HTML now posts results on a same-origin The popup hook listens on that channel, tracks in-flight flows by Reviewed by Cursor Bugbot for commit eb69205. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes a live staging regression where connecting an OAuth MCP server whose authorization page sends
Confidence Score: 5/5Safe to merge. The fix is well-scoped and directly targets the confirmed live regression without touching unrelated paths. The BroadcastChannel approach is the correct solution for COOP-severed openers, and the state-nonce correlation scheme correctly isolates each flow to the tab that started it — other same-origin tabs exit early before reaching any toast or cache invalidation. The previous review concern about spurious error toasts in other-workspace tabs is fully addressed: those tabs have no matching state in their pendingFlowsRef and bail at the guard. The popup-closed poll is correctly kept as a best-effort spinner reset, never touching pendingFlowsRef, so it cannot race with a genuine completion. Safety timeout, cleanup on unmount, and idempotent state transitions all look correct. No files require special attention. All five changed files are consistent with each other and the approach is sound end-to-end. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Parent Tab
participant MQ as useStartMcpOauth
participant API as /api/mcp/oauth/start
participant Popup as OAuth Popup
participant Provider as Provider (COOP same-origin)
participant CB as /api/mcp/oauth/callback
participant BC as BroadcastChannel
UI->>MQ: startOauthForServer(serverId)
MQ->>API: GET /api/mcp/oauth/start
API-->>MQ: authorizationUrl with state nonce
MQ->>MQ: extract state from URL
MQ->>Popup: window.open(authorizationUrl)
MQ-->>UI: redirect with popup and state
UI->>UI: pendingFlowsRef.set(state, serverId + timeout)
UI->>BC: subscribe on mcp-oauth channel
Popup->>Provider: navigate to authorizationUrl
Note over Provider: COOP same-origin severs window.opener
Provider-->>Popup: user authorizes then redirected
Popup->>CB: "GET /callback?state=...&code=..."
CB->>CB: token exchange and tool discovery
CB-->>Popup: HTML with inline BroadcastChannel script
Popup->>BC: ch.postMessage with ok, state, serverId, reason
BC-->>UI: onmessage fires
UI->>UI: pendingFlowsRef.get(state) matches flow
UI->>UI: settleFlow clears timeout and spinner
alt authorized
UI->>UI: invalidateQueries and toast success
else failed
UI->>UI: toast error with reason message
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as Parent Tab
participant MQ as useStartMcpOauth
participant API as /api/mcp/oauth/start
participant Popup as OAuth Popup
participant Provider as Provider (COOP same-origin)
participant CB as /api/mcp/oauth/callback
participant BC as BroadcastChannel
UI->>MQ: startOauthForServer(serverId)
MQ->>API: GET /api/mcp/oauth/start
API-->>MQ: authorizationUrl with state nonce
MQ->>MQ: extract state from URL
MQ->>Popup: window.open(authorizationUrl)
MQ-->>UI: redirect with popup and state
UI->>UI: pendingFlowsRef.set(state, serverId + timeout)
UI->>BC: subscribe on mcp-oauth channel
Popup->>Provider: navigate to authorizationUrl
Note over Provider: COOP same-origin severs window.opener
Provider-->>Popup: user authorizes then redirected
Popup->>CB: "GET /callback?state=...&code=..."
CB->>CB: token exchange and tool discovery
CB-->>Popup: HTML with inline BroadcastChannel script
Popup->>BC: ch.postMessage with ok, state, serverId, reason
BC-->>UI: onmessage fires
UI->>UI: pendingFlowsRef.get(state) matches flow
UI->>UI: settleFlow clears timeout and spinner
alt authorized
UI->>UI: invalidateQueries and toast success
else failed
UI->>UI: toast error with reason message
end
Reviews (6): Last reviewed commit: "fix(mcp): drop a server's prior in-fligh..." | Re-trigger Greptile |
…e popup A BroadcastChannel reaches every same-origin tab, so the previous workspace-id scoping (which the success path carried but failure paths omitted) still let unrelated tabs clear state, refetch, and show spurious toasts. Gate every reaction on whether this tab actually has an in-flight popup for the result's server (`popupIntervalsRef`) — strictly more precise than workspace scoping and correct for both cross-workspace and same-workspace-second-tab cases. Removes the now-redundant workspaceId from the callback message.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
Cursor flagged two defects in the previous gate, both rooted in keying the BroadcastChannel filter on `popupIntervalsRef` (the popup.closed poll map): - A genuine completion was dropped whenever the poll had already removed the server's entry — and COOP can make `popup.closed` misreport, which is exactly the case this PR targets, so the fix could silently fail to apply. - A result without a serverId fell back to "any in-flight popup", waking unrelated same-origin tabs with spurious toasts/refetches. Introduce `pendingFlowsRef` (serverId -> safety timeout) as the correlation source of truth: cleared only on completion or a 10-min timeout (matching the server OAuth start TTL), never by popup.closed. The popup.closed poll now only clears the spinner (best-effort abandon UX) and never touches correlation, so a real completion is always processed. Results without a serverId are ignored; the initiating tab's safety timeout clears its own "Connecting…".
|
@cursor review |
Cursor flagged that a failure which can't resolve a serverId (notably invalid_state) broadcasts ok:false with no serverId, so the serverId-keyed gate ignored it and the initiating tab sat on "Connecting…" until the safety timeout with no error feedback. Correlate on the OAuth `state` instead — the per-flow nonce the callback echoes on every result, success or failure. The client parses it from the authorization URL and keys the in-flight map by it; the callback includes it on every response via a `respond` helper. This is the canonical popup-OAuth correlation: it reaches the initiating tab even when no serverId exists, and — because each flow has a unique state — it also fixes the same-user-same-server-in-two-tabs edge a serverId key left open. Results with no parseable state (a malformed callback) are still ignored; that flow clears via its timeout.
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3553676. Configure here.
Each start mints a new `state`, so an abandoned attempt's safety timeout was keyed under a different state than its retry and never cleared. In the contrived case where both stayed pending ~10 min, the stale timer would clear the newer flow's spinner. Sweep any prior in-flight flow for the same serverId on a new start (replacing the can't-happen same-state check). Result delivery was already correct; this makes the state machine airtight.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit eb69205. Configure here.
1 participant
Summary
https://app.withgauge.com/mcp) hangs on "Connecting…" forever. AWS logs showed the OAuth round-trip completing (/oauth/start200, user authorized,/oauth/callbackhit) but the UI never updating.Cross-Origin-Opener-Policy: same-origin. Per MDN, that setswindow.opener = nullfor the cross-origin popup, so the callback'swindow.opener.postMessagewas silently dropped — the parent never learns the flow finished. Our page already usessame-origin-allow-popups, but that doesn't help when the provider's page forcessame-origin.BroadcastChannelinstead ofwindow.opener.postMessage. BroadcastChannel is origin-scoped and COOP-immune by design — the documented workaround for exactly this. The message is scoped byworkspaceIdso a concurrent flow in another open workspace is ignored.ok:falsepopup close, so a failed authorization couldn't be diagnosed from server logs. (Separately, the reported attempt's callback returned in 11ms — an early-gate failure before token exchange; this logging makes that reason visible on the next attempt.)Type of Change
Testing
tsc, Biome, and 40 MCP OAuth tests pass.same-originprovider).Out of scope (deliberately not changed)
Checklist