Skip to content

Commit 0f43215

Browse files
committed
fix(mcp): keep a legacy server row from blanking the whole server list
The server-list response validated transport/authType/connectionStatus with strict enums, but those columns are free text and can hold legacy values (e.g. transport 'http'/'sse' from before the streamable-http consolidation, including rows copied verbatim by workspace fork). Since the client strict- parses the whole array with retry:false, a single off-enum row blanked the entire workspace's MCP list. - Tolerate off-enum values in the response via Zod .catch() (request bodies stay strict): transport -> streamable-http, authType/connectionStatus -> undefined - Stop the workspace-fork copy from propagating legacy transport values - Reset connection status on a headers->OAuth flip in the create/upsert path, mirroring the update path (kills a false "connected" state) - Drop the positive tool cache on OAuth-pending in bulk discovery, matching the single-server path (no stale tools after re-auth is required)
1 parent bd636d4 commit 0f43215

4 files changed

Lines changed: 43 additions & 7 deletions

File tree

apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,9 @@ export async function copyForkResourceContainers(
395395
createdBy: userId,
396396
url: typeof row.url === 'string' ? rewriteEnv(row.url) : row.url,
397397
headers,
398+
// Normalize legacy `http`/`sse` transports to the only supported value so a
399+
// forked row never carries a transport the API contract would reject.
400+
transport: 'streamable-http',
398401
connectionStatus: 'disconnected',
399402
lastConnected: null,
400403
lastError: null,

apps/sim/lib/api/contracts/mcp.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ const optionalNumberFromNullableSchema = z.preprocess(
2626

2727
const optionalConnectionStatusFromNullableSchema = z.preprocess(
2828
(value) => (value === null ? undefined : value),
29-
z.enum(['connected', 'disconnected', 'error']).optional()
29+
// `connection_status` is a free-text column; tolerate an off-enum value as undefined
30+
// rather than failing the whole list's validation.
31+
z
32+
.enum(['connected', 'disconnected', 'error'])
33+
.optional()
34+
.catch(undefined)
3035
)
3136

3237
const optionalHeadersFromNullableSchema = z.preprocess(
@@ -36,6 +41,16 @@ const optionalHeadersFromNullableSchema = z.preprocess(
3641

3742
export const mcpTransportSchema = z.enum(['streamable-http'])
3843

44+
/**
45+
* Transport as read back from storage. The `transport` column is free text, and
46+
* rows predating the Streamable HTTP consolidation (or copied verbatim by an
47+
* older fork) may still hold legacy `http`/`sse` values. Every server is operated
48+
* over Streamable HTTP regardless, so any non-canonical value normalizes to the
49+
* supported transport — this stops a single legacy row from failing the entire
50+
* server list's response validation.
51+
*/
52+
const mcpTransportResponseSchema = mcpTransportSchema.catch('streamable-http')
53+
3954
export const mcpAuthTypeSchema = z.enum(['none', 'headers', 'oauth'])
4055

4156
const consecutiveFailuresSchema = z.preprocess(
@@ -98,8 +113,10 @@ export const mcpServerSchema = z
98113
workspaceId: z.string(),
99114
name: z.string(),
100115
description: optionalStringFromNullableSchema,
101-
transport: mcpTransportSchema,
102-
authType: mcpAuthTypeSchema.optional(),
116+
transport: mcpTransportResponseSchema,
117+
// Response-side tolerance: `auth_type` is a free-text column, so a value outside
118+
// the enum normalizes to undefined rather than failing the whole list's validation.
119+
authType: mcpAuthTypeSchema.optional().catch(undefined),
103120
url: optionalStringFromNullableSchema,
104121
timeout: optionalNumberFromNullableSchema,
105122
retries: optionalNumberFromNullableSchema,

apps/sim/lib/mcp/orchestration/server-lifecycle.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,10 @@ export async function performCreateMcpServer(
169169
currentEncryptedClientSecret: existingServer.oauthClientSecret,
170170
})
171171
const isRevival = existingServer.deletedAt !== null
172-
const shouldClearOauth = urlChanged || credsChanged || isRevival
172+
const authTypeChanged = existingServer.authType !== resolvedAuthType
173+
// Turning OAuth off orphans its tokens; revoke and delete them, mirroring the update path.
174+
const oauthDisabled = existingServer.authType === 'oauth' && resolvedAuthType !== 'oauth'
175+
const shouldClearOauth = urlChanged || credsChanged || isRevival || oauthDisabled
173176

174177
if (shouldClearOauth) await revokeMcpOauthTokens(serverId)
175178

@@ -191,9 +194,13 @@ export async function performCreateMcpServer(
191194
deletedAt: null,
192195
}
193196
if (resolvedAuthType === 'oauth') {
194-
if (shouldClearOauth) {
197+
// A flip to OAuth (headers→OAuth carries no tokens to clear) or an OAuth URL/creds
198+
// change leaves no valid session: reset to disconnected so the UI shows
199+
// "authorization required" instead of a stale connected state until the flow completes.
200+
if (authTypeChanged || shouldClearOauth) {
195201
updateValues.connectionStatus = 'disconnected'
196202
updateValues.lastConnected = null
203+
updateValues.lastError = null
197204
}
198205
} else {
199206
updateValues.connectionStatus = 'connected'

apps/sim/lib/mcp/service.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -760,11 +760,20 @@ class McpService {
760760
return
761761
}
762762
if (outcome.kind === 'oauth-pending') {
763-
// Mark disconnected so the UI surfaces the re-auth button.
763+
// Mark disconnected so the UI surfaces the re-auth button, and drop the positive
764+
// tool cache so a follow-up force-refresh can't serve tools for a server that now
765+
// needs re-auth (mirrors the single-server discovery path).
764766
logger.info(`[${requestId}] Skipping server ${server.name}: OAuth authorization pending`)
765767
deferredSideEffects.push(
766768
this.markServerOauthPending(server.id, workspaceId, discoveryStartedAt).then(
767-
() => undefined
769+
async (statusApplied) => {
770+
if (!statusApplied) return
771+
await this.cacheAdapter
772+
.delete(serverCacheKey(workspaceId, server.id))
773+
.catch((err) =>
774+
logger.warn(`[${requestId}] Cache delete failed for ${server.name}:`, err)
775+
)
776+
}
768777
)
769778
)
770779
return

0 commit comments

Comments
 (0)