fix(mcp): bugbot review on hardening PR

- Replace `as never` cast in negotiateProtocolVersion with `as readonly string[]`
  on the array — preserves TypeScript narrowing on the comparison while
  satisfying the readonly-tuple `.includes()` constraint properly.
- Document the pg_advisory_xact_lock tradeoff: session-level locks
  (`pg_advisory_lock`) would release the connection earlier, but PgBouncer
  transaction-pooling mode breaks them. xact_lock is the correct choice for
  Sim's deployment; if pool pressure becomes real, the comment notes the
  Redlock escape hatch.

Author: waleedlatif1

apps/sim/app/api/mcp/serve/[serverId]/route.ts

@@ -46,7 +46,10 @@ function negotiateProtocolVersion(rpcParams: unknown): string {
     rpcParams && typeof rpcParams === 'object' && 'protocolVersion' in rpcParams
       ? (rpcParams as { protocolVersion?: unknown }).protocolVersion
       : undefined
-  if (typeof requested === 'string' && SUPPORTED_PROTOCOL_VERSIONS.includes(requested as never)) {
+  if (
+    typeof requested === 'string' &&
+    (SUPPORTED_PROTOCOL_VERSIONS as readonly string[]).includes(requested)
+  ) {
     return requested
   }
   return LATEST_PROTOCOL_VERSION

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

@@ -237,6 +237,14 @@ export async function clearState(rowId: string): Promise<void> {
  *      Node process holds concurrent callers.
  *   2) Postgres advisory transaction lock — blocks across processes; released
  *      automatically when the transaction ends.
+ *
+ * Tradeoff: the connection is held for the duration of `fn()`, which includes
+ * the SDK's OAuth HTTP refresh. Session-level locks (`pg_advisory_lock`) would
+ * release the connection earlier, but they don't survive PgBouncer transaction
+ * pooling — they're scoped to the underlying physical connection, which can be
+ * swapped between statements. `pg_advisory_xact_lock` is the correct choice
+ * here. If pool pressure becomes a real concern at scale, swap this for a
+ * Redis-based distributed lock (Redlock) that doesn't pin a DB connection.
  */
 const refreshLocks = new Map<string, Promise<unknown>>()