fix: prevent partial tool leaks and non-blocking dispose

- syncTools: on paginated listTools failure, unregister any tools already
  registered in the current sync before rethrowing (prevents orphans)
- Effect disposer: call client.close() directly without awaiting startup
  completion — aborts a hanging connect promptly on HMR/dispose
This commit is contained in:
lintianle
2026-07-08 13:23:46 +08:00
parent fdd7d1a91c
commit 0c8f2f7daf
2 changed files with 34 additions and 25 deletions

View File

@@ -140,8 +140,9 @@ export function apply(ctx: Context, config: Config): void {
} }
// Connect and set up tools. Errors during connect are logged, not thrown // Connect and set up tools. Errors during connect are logged, not thrown
// (the plugin simply has no tools registered). // (the plugin simply has no tools registered). The IIFE is fire-and-forget;
const ready = (async () => { // disposal closes the client directly without waiting for startup.
void (async () => {
await client.connect(transport) await client.connect(transport)
await resync() await resync()
@@ -156,9 +157,10 @@ export function apply(ctx: Context, config: Config): void {
ctx.logger.error(`mcp-client: failed to connect: ${String(error)}`) ctx.logger.error(`mcp-client: failed to connect: ${String(error)}`)
}) })
// Fiber disposal: close the client (triggers onclose → tools unregistered). // Fiber disposal: close the client immediately (triggers onclose → tools
// unregistered). No `await ready` — if connect is still pending, close aborts
// it promptly rather than blocking until the SDK request times out.
ctx.effect(() => async () => { ctx.effect(() => async () => {
await ready try { await client.close() } catch { /* transport already gone or never connected */ }
try { await client.close() } catch { /* transport already gone */ }
}, 'mcp-client.connection') }, 'mcp-client.connection')
} }

View File

@@ -43,27 +43,34 @@ export async function syncTools(
const disposers: ToolDisposers = new Map() const disposers: ToolDisposers = new Map()
let cursor: string | undefined try {
do { let cursor: string | undefined
const response = await client.listTools(cursor ? { cursor } : undefined) do {
for (const tool of response.tools) { const response = await client.listTools(cursor ? { cursor } : undefined)
const registeredName = opts.toolPrefix + tool.name for (const tool of response.tools) {
const definition: ToolDefinition = { const registeredName = opts.toolPrefix + tool.name
name: registeredName, const definition: ToolDefinition = {
description: tool.description ?? '', name: registeredName,
parameters: tool.inputSchema, description: tool.description ?? '',
execute: createExecutor(client, tool.name, opts), parameters: tool.inputSchema,
execute: createExecutor(client, tool.name, opts),
}
try {
const dispose = ctx.tools.register(definition)
disposers.set(registeredName, dispose)
} catch {
// Name conflict — another tool with this name is already registered.
ctx.logger.warn(`mcp-client: skipping tool "${registeredName}" (name conflict)`)
}
} }
try { cursor = response.nextCursor
const dispose = ctx.tools.register(definition) } while (cursor)
disposers.set(registeredName, dispose) } catch (error: unknown) {
} catch { // Partial failure (e.g. a later page of listTools failed): unregister any
// Name conflict — another tool with this name is already registered. // tools already registered in this sync to avoid orphaning them.
ctx.logger.warn(`mcp-client: skipping tool "${registeredName}" (name conflict)`) for (const dispose of disposers.values()) dispose()
} throw error
} }
cursor = response.nextCursor
} while (cursor)
return disposers return disposers
} }