chore(mcp): remove unused cache CAS methods and simplify discovery helpers

- Delete the unused setIfCurrentMutation/deleteIfCurrentMutation cache
  methods (interface + memory + redis adapters) and their two Redis Lua
  scripts; production only uses applyMutationIfCurrent. Tests rewritten onto
  applyMutationIfCurrent (ordering coverage preserved) or dropped where they
  only exercised the removed Lua.
- Drop the vestigial fallback param from getDiscoveryFailureMessage (all
  callers coerced to 'Connection failed').
- Route isDynamicClientRegistrationUnsupported through getErrorMessage to
  match the module's error-inspection convention.

Author: waleedlatif1

apps/sim/app/api/mcp/oauth/start/route.ts

@@ -2,7 +2,7 @@ import { OAuthError, ServerError } from '@modelcontextprotocol/sdk/server/auth/e
 import { db } from '@sim/db'
 import { mcpServers } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { toError } from '@sim/utils/errors'
+import { getErrorMessage, toError } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
@@ -29,10 +29,9 @@ const DCR_UNSUPPORTED_MESSAGE =
   "This server doesn't support OAuth client registration. Configure a token instead."
 
 function isDynamicClientRegistrationUnsupported(error: unknown): boolean {
-  return (
-    error instanceof Error &&
-    error.message.toLowerCase().includes('does not support dynamic client registration')
-  )
+  return getErrorMessage(error, '')
+    .toLowerCase()
+    .includes('does not support dynamic client registration')
 }
 
 export function surfaceOauthError(error: unknown): string {

apps/sim/lib/mcp/service.test.ts

@@ -53,24 +53,6 @@ const {
       cacheMutations.set(scopeKey, mutationId)
       return mutationId
     }),
-    setIfCurrentMutation: vi.fn(
-      async (
-        scopeKey: string,
-        mutationId: number,
-        key: string,
-        tools: unknown[],
-        ttlMs: number
-      ) => {
-        if (cacheMutations.get(scopeKey) !== mutationId) return false
-        cacheStore.set(key, { tools, expiry: Date.now() + ttlMs })
-        return true
-      }
-    ),
-    deleteIfCurrentMutation: vi.fn(async (scopeKey: string, mutationId: number, key: string) => {
-      if (cacheMutations.get(scopeKey) !== mutationId) return false
-      cacheStore.delete(key)
-      return true
-    }),
     applyMutationIfCurrent: vi.fn(
       async (
         scopeKey: string,

apps/sim/lib/mcp/service.ts

@@ -169,11 +169,7 @@ function isOauthAuthorizationError(error: unknown, authType: McpServerConfig['au
   )
 }
 
-function getDiscoveryFailureMessage(
-  error: unknown,
-  authType: McpServerConfig['authType'],
-  fallback: string
-): string {
+function getDiscoveryFailureMessage(error: unknown, authType: McpServerConfig['authType']): string {
   if (authType !== 'oauth' && error instanceof UnauthorizedError) {
     return 'Authentication failed'
   }
@@ -197,7 +193,7 @@ function getDiscoveryFailureMessage(
   ) {
     return 'Unable to reach the MCP server'
   }
-  return fallback === 'Unknown error' ? 'Connection failed' : fallback
+  return 'Connection failed'
 }
 
 function isTimeoutError(error: unknown): boolean {
@@ -898,7 +894,7 @@ class McpService {
             }
             return {
               kind: 'error',
-              message: getDiscoveryFailureMessage(error, config.authType, 'Unknown error'),
+              message: getDiscoveryFailureMessage(error, config.authType),
               originalError: error,
               config,
               mutation,
@@ -1137,7 +1133,7 @@ class McpService {
               config,
               mutation,
               error,
-              getDiscoveryFailureMessage(error, config.authType, 'Connection failed')
+              getDiscoveryFailureMessage(error, config.authType)
             )
           }
         }
@@ -1200,7 +1196,7 @@ class McpService {
             status: 'error',
             toolCount: 0,
             lastSeen: undefined,
-            error: getDiscoveryFailureMessage(error, config.authType, 'Connection failed'),
+            error: getDiscoveryFailureMessage(error, config.authType),
           })
         }
       }

apps/sim/lib/mcp/storage/adapter.ts

@@ -22,14 +22,6 @@ export interface McpCacheStorageAdapter {
    * same value orders database publication for an end-to-end consistent state.
    */
   beginMutation(scopeKey: string): Promise<number>
-  setIfCurrentMutation(
-    scopeKey: string,
-    mutationId: number,
-    key: string,
-    tools: McpTool[],
-    ttlMs: number
-  ): Promise<boolean>
-  deleteIfCurrentMutation(scopeKey: string, mutationId: number, key: string): Promise<boolean>
   /** Atomically applies one server's complete cache state if this mutation still owns it. */
   applyMutationIfCurrent(
     scopeKey: string,

apps/sim/lib/mcp/storage/memory-cache.test.ts

@@ -175,16 +175,20 @@ describe('MemoryMcpCache', () => {
       expect(newer).toBeGreaterThan(older)
 
       expect(
-        await cache.setIfCurrentMutation(
+        await cache.applyMutationIfCurrent(
           'server-1',
           newer,
-          'server-1:tools',
-          [createTool('new-tool')],
-          60000
+          { key: 'server-1:tools', tools: [createTool('new-tool')], ttlMs: 60000 },
+          []
         )
       ).toBe(true)
       expect(
-        await cache.setIfCurrentMutation('server-1', older, 'server-1:failure', [], 60000)
+        await cache.applyMutationIfCurrent(
+          'server-1',
+          older,
+          { key: 'server-1:failure', tools: [], ttlMs: 60000 },
+          []
+        )
       ).toBe(false)
 
       expect((await cache.get('server-1:tools'))?.tools).toEqual([createTool('new-tool')])
@@ -218,12 +222,11 @@ describe('MemoryMcpCache', () => {
       await cache.clear()
 
       expect(
-        await cache.setIfCurrentMutation(
+        await cache.applyMutationIfCurrent(
           'server-1',
           mutation,
-          'server-1:tools',
-          [createTool('stale-tool')],
-          60000
+          { key: 'server-1:tools', tools: [createTool('stale-tool')], ttlMs: 60000 },
+          []
         )
       ).toBe(false)
       expect(await cache.get('server-1:tools')).toBeNull()

apps/sim/lib/mcp/storage/memory-cache.ts

@@ -97,28 +97,6 @@ export class MemoryMcpCache implements McpCacheStorageAdapter {
     return mutationId
   }
 
-  async setIfCurrentMutation(
-    scopeKey: string,
-    mutationId: number,
-    key: string,
-    tools: McpTool[],
-    ttlMs: number
-  ): Promise<boolean> {
-    if (this.mutationVersions.get(scopeKey) !== mutationId) return false
-    await this.set(key, tools, ttlMs)
-    return true
-  }
-
-  async deleteIfCurrentMutation(
-    scopeKey: string,
-    mutationId: number,
-    key: string
-  ): Promise<boolean> {
-    if (this.mutationVersions.get(scopeKey) !== mutationId) return false
-    await this.delete(key)
-    return true
-  }
-
   async applyMutationIfCurrent(
     scopeKey: string,
     mutationId: number,

apps/sim/lib/mcp/storage/redis-cache.test.ts

@@ -55,50 +55,6 @@ describe('RedisMcpCache ordered mutations', () => {
     )
   })
 
-  it('reports whether a tool cache write still owns the current mutation', async () => {
-    redis.eval.mockResolvedValueOnce(1).mockResolvedValueOnce(0)
-
-    await expect(
-      cache.setIfCurrentMutation('workspace:w:server:s', 7, 'workspace:w:server:s', [tool], 60_000)
-    ).resolves.toBe(true)
-    await expect(
-      cache.setIfCurrentMutation(
-        'workspace:w:server:s',
-        6,
-        'workspace:w:server:s:failure',
-        [],
-        60_000
-      )
-    ).resolves.toBe(false)
-
-    expect(redis.eval).toHaveBeenNthCalledWith(
-      1,
-      expect.stringContaining("redis.call('SET'"),
-      2,
-      'mcp:tools-mutation:workspace:w:server:s',
-      'mcp:tools:workspace:w:server:s',
-      '7',
-      expect.stringContaining('new-tool'),
-      '60000'
-    )
-  })
-
-  it('guards cache deletion with the same mutation id', async () => {
-    redis.eval.mockResolvedValueOnce(0)
-
-    await expect(
-      cache.deleteIfCurrentMutation('workspace:w:server:s', 6, 'workspace:w:server:s:failure')
-    ).resolves.toBe(false)
-
-    expect(redis.eval).toHaveBeenCalledWith(
-      expect.stringContaining("redis.call('DEL'"),
-      2,
-      'mcp:tools-mutation:workspace:w:server:s',
-      'mcp:tools:workspace:w:server:s:failure',
-      '6'
-    )
-  })
-
   it('atomically replaces the complete cache state for the current mutation', async () => {
     redis.eval.mockResolvedValueOnce(1)
 

apps/sim/lib/mcp/storage/redis-cache.ts

@@ -18,22 +18,6 @@ redis.call('SET', KEYS[1], tostring(mutationId), 'PX', ARGV[1])
 return mutationId
 `
 
-const SET_IF_CURRENT_MUTATION = `
-if redis.call('GET', KEYS[1]) ~= ARGV[1] then
-  return 0
-end
-redis.call('SET', KEYS[2], ARGV[2], 'PX', ARGV[3])
-return 1
-`
-
-const DELETE_IF_CURRENT_MUTATION = `
-if redis.call('GET', KEYS[1]) ~= ARGV[1] then
-  return 0
-end
-redis.call('DEL', KEYS[2])
-return 1
-`
-
 const APPLY_MUTATION_IF_CURRENT = `
 if redis.call('GET', KEYS[1]) ~= ARGV[1] then
   return 0
@@ -124,54 +108,6 @@ export class RedisMcpCache implements McpCacheStorageAdapter {
     }
   }
 
-  async setIfCurrentMutation(
-    scopeKey: string,
-    mutationId: number,
-    key: string,
-    tools: McpTool[],
-    ttlMs: number
-  ): Promise<boolean> {
-    try {
-      const entry: McpCacheEntry = {
-        tools,
-        expiry: Date.now() + ttlMs,
-      }
-      const result = await this.redis.eval(
-        SET_IF_CURRENT_MUTATION,
-        2,
-        this.getMutationKey(scopeKey),
-        this.getKey(key),
-        String(mutationId),
-        JSON.stringify(entry),
-        String(ttlMs)
-      )
-      return result === 1
-    } catch (error) {
-      logger.error('Redis conditional cache set error:', error)
-      throw error
-    }
-  }
-
-  async deleteIfCurrentMutation(
-    scopeKey: string,
-    mutationId: number,
-    key: string
-  ): Promise<boolean> {
-    try {
-      const result = await this.redis.eval(
-        DELETE_IF_CURRENT_MUTATION,
-        2,
-        this.getMutationKey(scopeKey),
-        this.getKey(key),
-        String(mutationId)
-      )
-      return result === 1
-    } catch (error) {
-      logger.error('Redis conditional cache delete error:', error)
-      throw error
-    }
-  }
-
   async applyMutationIfCurrent(
     scopeKey: string,
     mutationId: number,