chore(mcp): trim verbose comments + reuse SDK Tool type in McpTool

- McpTool now extends `Pick<Tool, 'name' | 'description'>` from
  @modelcontextprotocol/sdk so name/description fields stay in sync with
  the SDK; serverId/serverName remain Sim-specific additions.
- Drop file-header restatements ("MCP Types - for connecting to external
  MCP servers"), one-line wrapper docstrings ("Get connection status"),
  and narrative comment blocks that just restate visible code.
- Keep only comments that document non-obvious "why" — OAuth refresh-lock
  tradeoff, in-flight dedup key composition, SDK Tool.inputSchema typing,
  preregistered-client semantics, postMessage handshake contract.

Author: waleedlatif1

apps/sim/hooks/queries/mcp.ts

@@ -57,9 +57,7 @@ export const mcpKeys = {
 
 export type { McpServer }
 
-/**
- * Input for creating/updating an MCP server (distinct from McpServerConfig in types.ts)
- */
+/** Wire shape for create/update; distinct from runtime McpServerConfig. */
 export interface McpServerInput {
   name: string
   transport: McpTransport
@@ -265,11 +263,7 @@ export function useCreateMcpServer() {
   })
 }
 
-/**
- * Result of `useStartMcpOauth`. When `popup` is set, the caller should wait
- * for it to close (or for the `mcp-oauth` postMessage) before clearing any
- * "connecting" UI state.
- */
+/** On `redirect`, the caller must wait for `popup.closed` or the `mcp-oauth` postMessage. */
 export type StartMcpOauthMutationResult =
   | { status: 'redirect'; popup: Window }
   | { status: 'already_authorized' }
@@ -464,13 +458,7 @@ const sseConnections: Map<string, SseEntry> =
   ((globalThis as Record<string, unknown>)[SSE_KEY] as Map<string, SseEntry>) ??
   ((globalThis as Record<string, unknown>)[SSE_KEY] = new Map<string, SseEntry>())
 
-/**
- * Subscribe to MCP tool-change SSE events for a workspace.
- * On each `tools_changed` event, invalidates the relevant React Query caches
- * so the UI refreshes automatically.
- *
- * Invalidates both external MCP server keys and workflow MCP server keys.
- */
+/** Subscribes to `tools_changed` SSE events and invalidates the affected query keys. */
 export function useMcpToolsEvents(workspaceId: string) {
   const queryClient = useQueryClient()
 
@@ -598,17 +586,11 @@ export function useMcpServerTest() {
   }
 }
 
-/**
- * Fetch allowed MCP domains (admin-configured allowlist)
- */
 async function fetchAllowedMcpDomains(signal?: AbortSignal): Promise<string[] | null> {
   const data = await requestJson(getAllowedMcpDomainsContract, { signal })
   return data.allowedMcpDomains ?? null
 }
 
-/**
- * Hook to fetch allowed MCP domains
- */
 export function useAllowedMcpDomains() {
   return useQuery<string[] | null>({
     queryKey: mcpKeys.allowedDomains(),

apps/sim/lib/mcp/client.ts

@@ -1,13 +1,3 @@
-/**
- * MCP (Model Context Protocol) Client
- *
- * Implements the client side of MCP protocol with support for:
- * - Streamable HTTP transport (MCP 2025-06-18)
- * - Tool execution and discovery
- * - Session management and protocol version negotiation
- * - Custom security/consent layer
- */
-
 import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
 import { Client } from '@modelcontextprotocol/sdk/client/index.js'
 import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
@@ -94,11 +84,6 @@ export class McpClient {
     )
   }
 
-  /**
-   * Initialize connection to MCP server.
-   * If an `onToolsChanged` callback was provided, registers a notification handler
-   * for `notifications/tools/list_changed` after connecting.
-   */
   async connect(): Promise<void> {
     logger.info(`Connecting to MCP server: ${this.config.name} (${this.config.transport})`)
 
@@ -135,9 +120,6 @@ export class McpClient {
     }
   }
 
-  /**
-   * Disconnect from MCP server
-   */
   async disconnect(): Promise<void> {
     logger.info(`Disconnecting from MCP server: ${this.config.name}`)
 
@@ -152,16 +134,10 @@ export class McpClient {
     logger.info(`Disconnected from MCP server: ${this.config.name}`)
   }
 
-  /**
-   * Get current connection status
-   */
   getStatus(): McpConnectionStatus {
     return { ...this.connectionStatus }
   }
 
-  /**
-   * List all available tools from the server
-   */
   async listTools(): Promise<McpTool[]> {
     if (!this.isConnected) {
       throw new McpConnectionError('Not connected to server', this.config.name)
@@ -190,9 +166,6 @@ export class McpClient {
     }
   }
 
-  /**
-   * Execute a tool on the MCP server
-   */
   async callTool(toolCall: McpToolCall): Promise<McpToolResult> {
     if (!this.isConnected) {
       throw new McpConnectionError('Not connected to server', this.config.name)
@@ -237,10 +210,6 @@ export class McpClient {
     }
   }
 
-  /**
-   * Ping the server to check if it's still alive and responsive
-   * Per MCP spec: servers should respond to ping requests
-   */
   async ping(): Promise<{ _meta?: Record<string, any> }> {
     if (!this.isConnected) {
       throw new McpConnectionError('Not connected to server', this.config.name)
@@ -257,18 +226,11 @@ export class McpClient {
     }
   }
 
-  /**
-   * Check if the server declared `capabilities.tools.listChanged: true` during initialization.
-   */
   hasListChangedCapability(): boolean {
     return !!this.client.getServerCapabilities()?.tools?.listChanged
   }
 
-  /**
-   * Register a callback to be invoked when the underlying transport closes.
-   * Used by the connection manager for reconnection logic.
-   * Chains with the SDK's internal onclose handler so it still performs its cleanup.
-   */
+  /** Chains with the SDK's internal onclose handler so its cleanup still runs. */
   onClose(callback: () => void): void {
     const existingHandler = this.transport.onclose
     this.transport.onclose = () => {
@@ -277,26 +239,17 @@ export class McpClient {
     }
   }
 
-  /**
-   * Get server configuration
-   */
   getConfig(): McpServerConfig {
     return { ...this.config }
   }
 
-  /**
-   * Get version information for this client
-   */
   static getVersionInfo(): McpVersionInfo {
     return {
       supported: [...McpClient.SUPPORTED_VERSIONS],
       preferred: McpClient.SUPPORTED_VERSIONS[0],
     }
   }
 
-  /**
-   * Get the negotiated protocol version for this connection
-   */
   getNegotiatedVersion(): string | undefined {
     const serverVersion = this.client.getServerVersion()
     return typeof serverVersion === 'string' ? serverVersion : undefined
@@ -306,9 +259,6 @@ export class McpClient {
     return this.transport.sessionId
   }
 
-  /**
-   * Request user consent for tool execution
-   */
   async requestConsent(consentRequest: McpConsentRequest): Promise<McpConsentResponse> {
     if (!this.securityPolicy.requireConsent) {
       return { granted: true, auditId: `audit-${Date.now()}` }

apps/sim/lib/mcp/oauth/provider.ts

@@ -41,10 +41,7 @@ export interface PreregisteredClient {
 interface SimMcpOauthProviderInit {
   row: McpOauthRow
   scope?: string
-  /**
-   * Optional user-supplied client credentials. When provided, the SDK skips
-   * Dynamic Client Registration and uses these for the auth/token exchange.
-   */
+  /** When set, the SDK skips Dynamic Client Registration and uses these credentials directly. */
   preregistered?: PreregisteredClient
 }
 

apps/sim/lib/mcp/service.ts

@@ -1,7 +1,3 @@
-/**
- * MCP Service - Clean stateless service for MCP operations
- */
-
 import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
 import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
 import { db } from '@sim/db'
@@ -100,19 +96,12 @@ class McpService {
     }
   }
 
-  /**
-   * Dispose of the service and cleanup resources
-   */
   dispose(): void {
     this.unsubscribeConnectionManager?.()
     this.cacheAdapter.dispose()
     logger.info('MCP Service disposed')
   }
 
-  /**
-   * Resolve environment variables in server config.
-   * Uses shared utility with strict mode (throws on missing vars).
-   */
   private async resolveConfigEnvVars(
     config: McpServerConfig,
     userId: string,
@@ -126,9 +115,6 @@ class McpService {
     return { config: resolvedConfig, resolvedIP }
   }
 
-  /**
-   * Get server configuration from database
-   */
   private async getServerConfig(
     serverId: string,
     workspaceId: string
@@ -171,9 +157,6 @@ class McpService {
     }
   }
 
-  /**
-   * Get all enabled servers for a workspace
-   */
   private async getWorkspaceServers(workspaceId: string): Promise<McpServerConfig[]> {
     const whereConditions = [
       eq(mcpServers.workspaceId, workspaceId),
@@ -205,9 +188,6 @@ class McpService {
       .filter((config) => isMcpDomainAllowed(config.url))
   }
 
-  /**
-   * Create and connect to an MCP client
-   */
   private async createClient(
     config: McpServerConfig,
     resolvedIP: string | null,
@@ -262,10 +242,6 @@ class McpService {
     })
   }
 
-  /**
-   * Execute a tool on a specific server with retry logic for session errors.
-   * Retries once on session-related errors (400, 404, session ID issues).
-   */
   async executeTool(
     userId: string,
     serverId: string,
@@ -320,22 +296,14 @@ class McpService {
     throw new Error(`Failed to execute tool ${toolCall.name} after ${maxRetries} attempts`)
   }
 
-  /**
-   * Detects an expired or unknown `Mcp-Session-Id` so the caller can retry.
-   * Per MCP spec, the server returns HTTP 404 for an unknown session id and
-   * may return 400 when the session header is malformed; the SDK surfaces
-   * both as `StreamableHTTPError` with a typed numeric `code` field.
-   */
+  /** MCP spec: server returns 404 for unknown session id, 400 for malformed header. */
   private isSessionError(error: unknown): boolean {
     if (error instanceof StreamableHTTPError) {
       return error.code === 404 || error.code === 400
     }
     return false
   }
 
-  /**
-   * Update server connection status after discovery attempt
-   */
   private async updateServerStatus(
     serverId: string,
     workspaceId: string,
@@ -448,9 +416,6 @@ class McpService {
     }
   }
 
-  /**
-   * Discover tools from all workspace servers
-   */
   async discoverTools(
     userId: string,
     workspaceId: string,
@@ -744,9 +709,6 @@ class McpService {
     throw new Error(`Failed to discover tools from server ${serverId} after ${maxRetries} attempts`)
   }
 
-  /**
-   * Get server summaries for a user
-   */
   async getServerSummaries(userId: string, workspaceId: string): Promise<McpServerSummary[]> {
     const requestId = generateRequestId()