Skip to content

Commit 1815e21

Browse files
committed
fix(mcp): drop the success-path status reconciliation heuristic
The client-side serversList invalidation on a successful probe assumed the probe updated the stored status, but a server-side cache hit returns tools without touching status — so in the failed-cache-delete edge it fired a pointless refetch. The benefit (instant vs the 60s serversList stale-time for status recovery) doesn't justify the incorrect assumption; rely on the existing 60s stale-time + SSE push for status recovery instead. Keeps the corrected keep/drop comment.
1 parent 55ba015 commit 1815e21

2 files changed

Lines changed: 1 addition & 41 deletions

File tree

apps/sim/hooks/queries/mcp.test.tsx

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -176,34 +176,6 @@ describe('useMcpToolsQuery', () => {
176176
unmount()
177177
})
178178

179-
it('refreshes the server list when a previously-failed server discovers successfully', async () => {
180-
let listCalls = 0
181-
mockRequestJson.mockImplementation(async (contract) => {
182-
if (contract === listMcpServersContract) {
183-
listCalls++
184-
return {
185-
success: true,
186-
data: {
187-
servers: [server('s1', { authType: 'headers', connectionStatus: 'disconnected' })],
188-
},
189-
}
190-
}
191-
if (contract === discoverMcpToolsContract) {
192-
return { success: true, data: { tools: [{ name: 'tool-a', serverId: 's1' }] } }
193-
}
194-
throw new Error('Unexpected MCP request')
195-
})
196-
197-
const { unmount } = renderHookWithClient(() => useMcpToolsQuery(WORKSPACE_ID))
198-
await flush()
199-
200-
// A successful probe against a server the cache still shows 'disconnected' refreshes the
201-
// list (server-side the status is now 'connected') so the row clears its red state.
202-
expect(listCalls).toBeGreaterThanOrEqual(2)
203-
204-
unmount()
205-
})
206-
207179
it('refreshes the server list after a connected OAuth discovery fails', async () => {
208180
let serverListRequests = 0
209181
mockRequestJson.mockImplementation(async (contract) => {

apps/sim/hooks/queries/mcp.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -172,19 +172,7 @@ export function useMcpToolsQuery(workspaceId: string) {
172172
queryKey: mcpKeys.serverToolsList(workspaceId, serverId),
173173
queryFn: async ({ signal }: { signal?: AbortSignal }) => {
174174
try {
175-
const tools = await fetchMcpTools(workspaceId, false, signal, serverId)
176-
// A successful probe flips the stored status to `connected` server-side; if the
177-
// cached list still shows this server failed, refresh it so the row clears its red
178-
// state (and its tools stop being dropped) instead of waiting out the list stale-time.
179-
const cached = queryClient.getQueryData<McpServer[]>(mcpKeys.serversList(workspaceId))
180-
const status = cached?.find((s) => s.id === serverId)?.connectionStatus
181-
if (status && status !== 'connected') {
182-
queryClient.invalidateQueries(
183-
{ queryKey: mcpKeys.serversList(workspaceId) },
184-
{ cancelRefetch: false }
185-
)
186-
}
187-
return tools
175+
return await fetchMcpTools(workspaceId, false, signal, serverId)
188176
} catch (error) {
189177
await queryClient.invalidateQueries(
190178
{ queryKey: mcpKeys.serversList(workspaceId) },

0 commit comments

Comments
 (0)