fix(mcp): harden notification system against race conditions

[waleedlatif1]

Summary

• Guard concurrent connect() calls in connection manager with a connectingServers Set to prevent duplicate client creation
• Add if (!this.isConnected) return guard in MCP client notification handler to suppress post-disconnect callbacks
• Call removeAllListeners() on Redis pub/sub clients before quit() in dispose() to prevent listener accumulation
• Clear connectingServers in dispose() for consistency with other collection cleanup
• Add 11 tests covering all three fixes + dispose behavior

Type of Change

• Bug fix

Testing

• 11 new tests across 3 test files, all passing
• Full MCP test suite (165 tests) green
• tsc --noEmit clean

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Feb 10, 2026 3:21am

[greptile-apps]

Greptile Overview

Greptile Summary

This PR hardens the MCP notification system against race conditions with three key fixes:

• Concurrent connection guard: Added connectingServers Set in connection-manager.ts:83 to prevent duplicate client creation when multiple connect() calls race for the same server. The Set is properly managed in a try-finally block and cleared on dispose.

• Post-disconnect callback suppression: Added if (!this.isConnected) return guard in client.ts:124 to prevent notification handlers from firing after disconnect, eliminating spurious callbacks during cleanup.

• Listener cleanup: Added removeAllListeners() calls in pubsub.ts:134-135 before Redis quit() to prevent listener accumulation across reconnects.

The PR also introduces comprehensive real-time notification infrastructure: a new McpConnectionManager for persistent connections, Redis pub/sub adapter with local fallback, SSE endpoint for pushing tool changes to browsers, and React hooks with reference-counted EventSource connections. All three race condition fixes are covered by new tests (11 total across 3 test suites).

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• All three race condition fixes are well-targeted, properly tested (11 new tests), and use standard concurrency patterns (Set-based guards, boolean flags, cleanup ordering). The implementation follows existing codebase patterns, has comprehensive error handling, and the test suite validates all edge cases including concurrent calls, cleanup, and disposal.
• No files require special attention

Important Files Changed

Filename	Overview
apps/sim/lib/mcp/connection-manager.ts	New connection manager with connectingServers Set to prevent duplicate client creation during concurrent connect() calls. Properly cleans up the Set in finally block and dispose.
apps/sim/lib/mcp/client.ts	Added isConnected guard in notification handler to suppress post-disconnect callbacks. Constructor overload added for McpClientOptions.
apps/sim/lib/mcp/pubsub.ts	New pub/sub adapter with Redis and local fallback. Calls removeAllListeners() before quit() in dispose to prevent listener accumulation.
apps/sim/lib/mcp/connection-manager.test.ts	Tests for concurrent connect guard: validates only one client created when racing, cleanup on errors, and disposal behavior.
apps/sim/lib/mcp/client.test.ts	Tests for notification handler: validates callbacks fire when connected, suppressed after disconnect, and not registered without callback.
apps/sim/lib/mcp/pubsub.test.ts	Tests for Redis pub/sub: validates client creation, listener registration, removeAllListeners() before quit, and disposed flag behavior.
apps/sim/lib/mcp/service.ts	Subscribes to connection manager events for cache invalidation. Initiates persistent connections for servers supporting listChanged after tool discovery.
apps/sim/app/api/mcp/events/route.ts	New SSE endpoint for pushing tool-change events to browsers. Subscribes to both external MCP and workflow CRUD events with proper cleanup.

Sequence Diagram

sequenceDiagram
    participant MCP as External MCP Server
    participant Client as McpClient
    participant ConnMgr as ConnectionManager
    participant PubSub as Redis PubSub
    participant Service as McpService
    participant SSE as SSE Endpoint
    participant Browser as Browser/UI

    Note over ConnMgr,Client: Connection Setup (with race protection)
    Service->>ConnMgr: connect(config, userId, workspaceId)
    ConnMgr->>ConnMgr: Check connectingServers Set
    alt Already connecting or connected
        ConnMgr-->>Service: return existing state
    else Not connecting
        ConnMgr->>ConnMgr: Add to connectingServers Set
        ConnMgr->>Client: new McpClient(onToolsChanged callback)
        Client->>MCP: connect()
        MCP-->>Client: connection established
        Client->>Client: Register notification handler
        ConnMgr->>ConnMgr: Remove from connectingServers
        ConnMgr-->>Service: supportsListChanged: true
    end

    Note over Browser,SSE: Browser subscribes to updates
    Browser->>SSE: EventSource connection
    SSE->>ConnMgr: subscribe(listener)
    SSE->>PubSub: onToolsChanged(listener)

    Note over MCP,Browser: Tool change notification flow
    MCP->>Client: notifications/tools/list_changed
    Client->>Client: Check if (!this.isConnected) return
    Client->>ConnMgr: onToolsChanged(serverId)
    ConnMgr->>PubSub: publishToolsChanged(event)
    PubSub->>ConnMgr: broadcasts to all processes
    ConnMgr->>Service: cache invalidation
    ConnMgr->>SSE: forward event to listener
    SSE->>Browser: SSE event: tools_changed
    Browser->>Browser: Invalidate React Query cache

    Note over ConnMgr,PubSub: Cleanup on dispose
    ConnMgr->>PubSub: dispose()
    PubSub->>PubSub: removeAllListeners()
    PubSub->>PubSub: quit() clients
    ConnMgr->>ConnMgr: Clear connectingServers Set

Loading

[greptile-apps]

_{10 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx
Error message gets truncated

Deploy errors are now rendered in a Badge with truncate, so longer error messages will be cut off with no way to view the full text. This will make debugging failed deployments harder compared to the previous multi-line error box. If truncation is desired for layout, consider adding title={deployError} (and similarly for warnings) or rendering the full message in a non-truncated area.

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[greptile-apps]

_{6 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@cursor review

[cursor]

Bugbot couldn't run

Something went wrong. Try again by commenting "Cursor review" or "bugbot run", or contact support (requestId: serverGenReqId_9070a327-63c1-43eb-8631-c031c300a272).

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[greptile-apps]

_{8 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR