Address PR review feedback (#5754)

Author: j15z

apps/sim/app/api/mcp/servers/[id]/refresh/route.test.ts

@@ -333,6 +333,48 @@ describe('MCP server refresh route', () => {
     expect(mockUpdate).not.toHaveBeenCalled()
   })
 
+  it.each(['unavailable', 'winner-cache'] as const)(
+    'preserves a newer cache invalidation over %s discovery tools',
+    async (state) => {
+      mockDiscoverServerTools.mockResolvedValueOnce({
+        tools: [
+          {
+            name: 'stale-search',
+            description: 'Search tool loaded before invalidation',
+            inputSchema: {},
+            serverId: 'server-1',
+            serverName: 'OAuth Server',
+          },
+        ],
+        state,
+      })
+      const cacheInvalidatedServer = {
+        ...initialServer,
+        lastToolsRefresh: new Date(initialServer.lastToolsRefresh.getTime() + 60_000),
+        toolCount: 0,
+      }
+      mockSelect.mockReturnValueOnce(selectRows([cacheInvalidatedServer]))
+
+      const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
+        method: 'POST',
+      }) as NextRequest
+      const response = await POST(request, { params: Promise.resolve({ id: 'server-1' }) })
+      const body = await response.json()
+
+      expect(body.data).toEqual(
+        expect.objectContaining({
+          status: 'connected',
+          error: null,
+          toolCount: 0,
+          workflowsUpdated: 0,
+          updatedWorkflowIds: [],
+        })
+      )
+      expect(mockSelect).toHaveBeenCalledTimes(2)
+      expect(mockUpdate).not.toHaveBeenCalled()
+    }
+  )
+
   it('preserves a newer successful refresh when discovery is superseded', async () => {
     mockDiscoverServerTools.mockResolvedValueOnce({
       tools: [],

apps/sim/app/api/mcp/servers/[id]/refresh/route.ts

@@ -265,11 +265,13 @@ export const POST = withRouteHandler(
         let connectionStatus = refreshedServer?.connectionStatus ?? 'error'
         let lastError = refreshedServer ? refreshedServer.lastError : discoveryError
         let toolCount = refreshedServer?.toolCount ?? discoveredTools.length
-        const newerSuccessWonRace =
-          connectionStatus === 'connected' &&
+        const newerPublicationWonRace =
           refreshedServer?.lastToolsRefresh != null &&
           (server.lastToolsRefresh == null ||
-            refreshedServer.lastToolsRefresh > server.lastToolsRefresh) &&
+            refreshedServer.lastToolsRefresh > server.lastToolsRefresh)
+        const newerSuccessWonRace =
+          connectionStatus === 'connected' &&
+          newerPublicationWonRace &&
           refreshedServer.lastConnected != null &&
           (server.lastConnected == null || refreshedServer.lastConnected > server.lastConnected)
 
@@ -279,6 +281,7 @@ export const POST = withRouteHandler(
           toolCount = 0
         } else if (
           discoveryError === null &&
+          !newerPublicationWonRace &&
           (discoveryState === 'unavailable' || discoveryState === 'winner-cache')
         ) {
           connectionStatus = 'connected'