From a6d771d3624c54b9d5b584dc3bb5b07f4e9b50da Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 3 Apr 2026 15:59:33 -0400 Subject: [PATCH] fix: correct stale-client cleanup in both OAuth paths - Blocking path: remove result?.clientInfo guard that made cleanup unreachable (handleOAuthRequired returns null on failure, so result?.clientInfo was always false in the failure branch) - returnOnOAuth path: only clear stored client when the prior flow status is FAILED, not on COMPLETED or PENDING flows, to avoid deleting valid registrations during normal flow replacement --- packages/api/src/mcp/MCPConnectionFactory.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/api/src/mcp/MCPConnectionFactory.ts b/packages/api/src/mcp/MCPConnectionFactory.ts index 8561b0c480..09323385db 100644 --- a/packages/api/src/mcp/MCPConnectionFactory.ts +++ b/packages/api/src/mcp/MCPConnectionFactory.ts @@ -356,7 +356,11 @@ export class MCPConnectionFactory { if (existingFlow) { const oldMeta = existingFlow.metadata as MCPOAuthFlowMetadata | undefined; - if (oldMeta?.reusedStoredClient && this.tokenMethods?.deleteTokens) { + if ( + existingFlow.status === 'FAILED' && + oldMeta?.reusedStoredClient && + this.tokenMethods?.deleteTokens + ) { await MCPTokenStorage.deleteClientRegistration({ userId: this.userId!, serverName: this.serverName, @@ -425,9 +429,9 @@ export class MCPConnectionFactory { } else { // OAuth failed — if we reused a stored client registration, clear it // so the next attempt falls through to fresh DCR - if (result?.clientInfo && this.tokenMethods?.deleteTokens) { + if (this.tokenMethods?.deleteTokens && this.flowManager) { const flowId = MCPOAuthHandler.generateFlowId(this.userId!, this.serverName); - const failedFlow = await this.flowManager?.getFlowState(flowId, 'mcp_oauth'); + const failedFlow = await this.flowManager.getFlowState(flowId, 'mcp_oauth'); const failedMeta = failedFlow?.metadata as MCPOAuthFlowMetadata | undefined; if (failedMeta?.reusedStoredClient) { await MCPTokenStorage.deleteClientRegistration({ @@ -435,7 +439,7 @@ export class MCPConnectionFactory { serverName: this.serverName, deleteTokens: this.tokenMethods.deleteTokens, }).catch((err) => { - logger.debug(`${this.logPrefix} Failed to clear stale client registration`, err); + logger.warn(`${this.logPrefix} Failed to clear stale client registration`, err); }); } }