Skip to content

Commit 6fd3b56

Browse files
authored
⚒️ fix: MCP Initialization Flows (danny-avila#8734)
* fix: add OAuth flow back in to success state * feat: disable server clicks during initialization to prevent spam * fix: correct new tab behavior for OAuth between one-click and normal initialization flows * fix: stop polling on error during oauth (was infinite popping toasts because we didn't clear interval) * fix: cleanupServerState should be called after successful cancelOauth, not before * fix: change from completeFlow to failFlow to avoid stale client IDs on OAuth after cancellation * fix: add logic to differentiate between cancelled and failed flows when checking status for indicators (so error triangle indicator doesn't show up on cancellaiton)
1 parent 6671fcb commit 6fd3b56

6 files changed

Lines changed: 73 additions & 23 deletions

File tree

api/server/routes/mcp.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ router.post('/oauth/cancel/:serverName', requireJwtAuth, async (req, res) => {
303303
}
304304

305305
// Cancel the flow by marking it as failed
306-
await flowManager.completeFlow(flowId, 'mcp_oauth', null, 'User cancelled OAuth flow');
306+
await flowManager.failFlow(flowId, 'mcp_oauth', 'User cancelled OAuth flow');
307307

308308
logger.info(`[MCP OAuth Cancel] Successfully cancelled OAuth flow for ${serverName}`);
309309

@@ -463,7 +463,7 @@ router.post('/:serverName/reinitialize', requireJwtAuth, async (req, res) => {
463463
};
464464

465465
res.json({
466-
success: userConnection && !oauthRequired,
466+
success: (userConnection && !oauthRequired) || (oauthRequired && oauthUrl),
467467
message: getResponseMessage(),
468468
serverName,
469469
oauthRequired,

api/server/services/MCP.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -287,14 +287,26 @@ async function checkOAuthFlowStatus(userId, serverName) {
287287
const flowTTL = flowState.ttl || 180000; // Default 3 minutes
288288

289289
if (flowState.status === 'FAILED' || flowAge > flowTTL) {
290-
logger.debug(`[MCP Connection Status] Found failed OAuth flow for ${serverName}`, {
291-
flowId,
292-
status: flowState.status,
293-
flowAge,
294-
flowTTL,
295-
timedOut: flowAge > flowTTL,
296-
});
297-
return { hasActiveFlow: false, hasFailedFlow: true };
290+
const wasCancelled = flowState.error && flowState.error.includes('cancelled');
291+
292+
if (wasCancelled) {
293+
logger.debug(`[MCP Connection Status] Found cancelled OAuth flow for ${serverName}`, {
294+
flowId,
295+
status: flowState.status,
296+
error: flowState.error,
297+
});
298+
return { hasActiveFlow: false, hasFailedFlow: false };
299+
} else {
300+
logger.debug(`[MCP Connection Status] Found failed OAuth flow for ${serverName}`, {
301+
flowId,
302+
status: flowState.status,
303+
flowAge,
304+
flowTTL,
305+
timedOut: flowAge > flowTTL,
306+
error: flowState.error,
307+
});
308+
return { hasActiveFlow: false, hasFailedFlow: true };
309+
}
298310
}
299311

300312
if (flowState.status === 'PENDING') {

client/src/components/Chat/Input/MCPSelect.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ function MCPSelect() {
1313
batchToggleServers,
1414
getServerStatusIconProps,
1515
getConfigDialogProps,
16+
isInitializing,
1617
localize,
1718
} = useMCPServerManager();
1819

@@ -32,14 +33,18 @@ function MCPSelect() {
3233
const renderItemContent = useCallback(
3334
(serverName: string, defaultContent: React.ReactNode) => {
3435
const statusIconProps = getServerStatusIconProps(serverName);
36+
const isServerInitializing = isInitializing(serverName);
3537

3638
// Common wrapper for the main content (check mark + text)
3739
// Ensures Check & Text are adjacent and the group takes available space.
3840
const mainContentWrapper = (
3941
<button
4042
type="button"
41-
className="flex flex-grow items-center rounded bg-transparent p-0 text-left transition-colors focus:outline-none"
43+
className={`flex flex-grow items-center rounded bg-transparent p-0 text-left transition-colors focus:outline-none ${
44+
isServerInitializing ? 'opacity-50' : ''
45+
}`}
4246
tabIndex={0}
47+
disabled={isServerInitializing}
4348
>
4449
{defaultContent}
4550
</button>
@@ -58,7 +63,7 @@ function MCPSelect() {
5863

5964
return mainContentWrapper;
6065
},
61-
[getServerStatusIconProps],
66+
[getServerStatusIconProps, isInitializing],
6267
);
6368

6469
// Don't render if no servers are selected and not pinned

client/src/components/Chat/Input/MCPSubMenu.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const MCPSubMenu = React.forwardRef<HTMLDivElement, MCPSubMenuProps>(
2222
toggleServerSelection,
2323
getServerStatusIconProps,
2424
getConfigDialogProps,
25+
isInitializing,
2526
} = useMCPServerManager();
2627

2728
const menuStore = Ariakit.useMenuStore({
@@ -86,6 +87,7 @@ const MCPSubMenu = React.forwardRef<HTMLDivElement, MCPSubMenuProps>(
8687
{configuredServers.map((serverName) => {
8788
const statusIconProps = getServerStatusIconProps(serverName);
8889
const isSelected = mcpValues?.includes(serverName) ?? false;
90+
const isServerInitializing = isInitializing(serverName);
8991

9092
const statusIcon = statusIconProps && <MCPServerStatusIcon {...statusIconProps} />;
9193

@@ -96,12 +98,15 @@ const MCPSubMenu = React.forwardRef<HTMLDivElement, MCPSubMenuProps>(
9698
event.preventDefault();
9799
toggleServerSelection(serverName);
98100
}}
101+
disabled={isServerInitializing}
99102
className={cn(
100103
'flex items-center gap-2 rounded-lg px-2 py-1.5 text-text-primary hover:cursor-pointer',
101104
'scroll-m-1 outline-none transition-colors',
102105
'hover:bg-black/[0.075] dark:hover:bg-white/10',
103106
'data-[active-item]:bg-black/[0.075] dark:data-[active-item]:bg-white/10',
104107
'w-full min-w-0 justify-between text-sm',
108+
isServerInitializing &&
109+
'opacity-50 hover:bg-transparent dark:hover:bg-transparent',
105110
)}
106111
>
107112
<div className="flex flex-grow items-center gap-2">

client/src/components/MCP/ServerInitializationSection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export default function ServerInitializationSection({
3434
const serverOAuthUrl = getOAuthUrl(serverName);
3535

3636
const handleInitializeClick = useCallback(() => {
37-
initializeServer(serverName);
37+
initializeServer(serverName, false);
3838
}, [initializeServer, serverName]);
3939

4040
const handleCancelClick = useCallback(() => {

client/src/hooks/MCP/useMCPServerManager.ts

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ export function useMCPServerManager() {
171171
message: localize('com_ui_mcp_oauth_timeout', { 0: serverName }),
172172
status: 'error',
173173
});
174+
clearInterval(pollInterval);
174175
cleanupServerState(serverName);
175176
return;
176177
}
@@ -180,10 +181,15 @@ export function useMCPServerManager() {
180181
message: localize('com_ui_mcp_init_failed'),
181182
status: 'error',
182183
});
184+
clearInterval(pollInterval);
183185
cleanupServerState(serverName);
186+
return;
184187
}
185188
} catch (error) {
186189
console.error(`[MCP Manager] Error polling server ${serverName}:`, error);
190+
clearInterval(pollInterval);
191+
cleanupServerState(serverName);
192+
return;
187193
}
188194
}, 3500);
189195

@@ -201,7 +207,7 @@ export function useMCPServerManager() {
201207
);
202208

203209
const initializeServer = useCallback(
204-
async (serverName: string) => {
210+
async (serverName: string, autoOpenOAuth: boolean = true) => {
205211
updateServerState(serverName, { isInitializing: true });
206212

207213
try {
@@ -216,7 +222,9 @@ export function useMCPServerManager() {
216222
isInitializing: true,
217223
});
218224

219-
window.open(response.oauthUrl, '_blank', 'noopener,noreferrer');
225+
if (autoOpenOAuth) {
226+
window.open(response.oauthUrl, '_blank', 'noopener,noreferrer');
227+
}
220228

221229
startServerPolling(serverName);
222230
} else {
@@ -265,13 +273,25 @@ export function useMCPServerManager() {
265273

266274
const cancelOAuthFlow = useCallback(
267275
(serverName: string) => {
268-
queryClient.invalidateQueries([QueryKeys.mcpConnectionStatus]);
269-
cleanupServerState(serverName);
270-
cancelOAuthMutation.mutate(serverName);
276+
// Call backend cancellation first, then clean up frontend state on success
277+
cancelOAuthMutation.mutate(serverName, {
278+
onSuccess: () => {
279+
// Only clean up frontend state after backend confirms cancellation
280+
cleanupServerState(serverName);
281+
queryClient.invalidateQueries([QueryKeys.mcpConnectionStatus]);
271282

272-
showToast({
273-
message: localize('com_ui_mcp_oauth_cancelled', { 0: serverName }),
274-
status: 'warning',
283+
showToast({
284+
message: localize('com_ui_mcp_oauth_cancelled', { 0: serverName }),
285+
status: 'warning',
286+
});
287+
},
288+
onError: (error) => {
289+
console.error(`[MCP Manager] Failed to cancel OAuth for ${serverName}:`, error);
290+
showToast({
291+
message: localize('com_ui_mcp_init_failed', { 0: serverName }),
292+
status: 'error',
293+
});
294+
},
275295
});
276296
},
277297
[queryClient, cleanupServerState, showToast, localize, cancelOAuthMutation],
@@ -309,6 +329,10 @@ export function useMCPServerManager() {
309329
const disconnectedServers: string[] = [];
310330

311331
serverNames.forEach((serverName) => {
332+
if (isInitializing(serverName)) {
333+
return;
334+
}
335+
312336
const serverStatus = connectionStatus[serverName];
313337
if (serverStatus?.connectionState === 'connected') {
314338
connectedServers.push(serverName);
@@ -323,11 +347,15 @@ export function useMCPServerManager() {
323347
initializeServer(serverName);
324348
});
325349
},
326-
[connectionStatus, setMCPValues, initializeServer],
350+
[connectionStatus, setMCPValues, initializeServer, isInitializing],
327351
);
328352

329353
const toggleServerSelection = useCallback(
330354
(serverName: string) => {
355+
if (isInitializing(serverName)) {
356+
return;
357+
}
358+
331359
const currentValues = mcpValues ?? [];
332360
const isCurrentlySelected = currentValues.includes(serverName);
333361

@@ -343,7 +371,7 @@ export function useMCPServerManager() {
343371
}
344372
}
345373
},
346-
[mcpValues, setMCPValues, connectionStatus, initializeServer],
374+
[mcpValues, setMCPValues, connectionStatus, initializeServer, isInitializing],
347375
);
348376

349377
const handleConfigSave = useCallback(

0 commit comments

Comments
 (0)