cleanup(mcp): final review fixes

- Remove redundant auth.clearOAuthState after auth.remove in removeAuth
- Remove dead status assignment in create() (assigned then immediately
  returned a new identical object)
- Fix mcpNameToState leak on successful/error OAuth callback completion
- Extract mcpConfig lookup once in tools() loop (was looked up twice)
This commit is contained in:
Kit Langton
2026-03-25 10:14:30 -04:00
parent 3188679396
commit 07c1cb03fc
2 changed files with 13 additions and 15 deletions

View File

@@ -381,16 +381,9 @@ export namespace MCP {
error,
})
})
status = {
status: "failed",
error: "Failed to get tools",
}
return {
mcpClient: undefined,
status: {
status: "failed" as const,
error: "Failed to get tools",
},
status: { status: "failed" as const, error: "Failed to get tools" },
}
}
@@ -610,6 +603,9 @@ export namespace MCP {
connectedClients,
([clientName, client]) =>
Effect.gen(function* () {
const mcpConfig = config[clientName]
const entry = mcpConfig && isMcpConfigured(mcpConfig) ? mcpConfig : undefined
const toolsResult = yield* Effect.tryPromise({
try: () => client.listTools(),
catch: (e: any) => {
@@ -624,10 +620,8 @@ export namespace MCP {
}).pipe(Effect.option)
if (Option.isNone(toolsResult)) {
// Fork background reconnection — tools come back next turn
const mcpConfig = config[clientName]
if (mcpConfig && isMcpConfigured(mcpConfig)) {
yield* createAndStore(clientName, mcpConfig).pipe(
if (entry) {
yield* createAndStore(clientName, entry).pipe(
Effect.catch(() => Effect.void),
Effect.forkIn(scope),
)
@@ -635,8 +629,6 @@ export namespace MCP {
return
}
const mcpConfig = config[clientName]
const entry = isMcpConfigured(mcpConfig) ? mcpConfig : undefined
const timeout = entry?.timeout ?? defaultTimeout
for (const mcpTool of toolsResult.value.tools) {
const sanitizedClientName = clientName.replace(/[^a-zA-Z0-9_-]/g, "_")
@@ -844,7 +836,6 @@ export namespace MCP {
yield* auth.remove(mcpName)
McpOAuthCallback.cancelPending(mcpName)
pendingOAuthTransports.delete(mcpName)
yield* auth.clearOAuthState(mcpName)
log.info("removed oauth credentials", { mcpName })
})

View File

@@ -101,6 +101,9 @@ export namespace McpOAuthCallback {
const pending = pendingAuths.get(state)!
clearTimeout(pending.timeout)
pendingAuths.delete(state)
for (const [name, s] of mcpNameToState) {
if (s === state) { mcpNameToState.delete(name); break }
}
pending.reject(new Error(errorMsg))
}
return new Response(HTML_ERROR(errorMsg), {
@@ -129,6 +132,10 @@ export namespace McpOAuthCallback {
clearTimeout(pending.timeout)
pendingAuths.delete(state)
// Clean up reverse index
for (const [name, s] of mcpNameToState) {
if (s === state) { mcpNameToState.delete(name); break }
}
pending.resolve(code)
return new Response(HTML_SUCCESS, {