fix(mcp-client): await Cordis startup discovery
This commit is contained in:
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/mcp/mcp-client/README.md
|
||||
README.md: 6bcc195e36d24d7e5ae3462573b30f1b41c963a7
|
||||
README.zh.md: 8886c16c3fe66d283ae2191661118729a89f1aeb
|
||||
README.md: c87255917a2def0aef938af9f2c910b65d5a96d5
|
||||
README.zh.md: 6fd6df39d7c5034795021d41de57f8500cbfd1c5
|
||||
|
||||
@@ -44,6 +44,7 @@ The model sees `mcp__github__create_issue`, `mcp__web__search`, … — the same
|
||||
| `url` | http | yes | MCP server URL |
|
||||
| `headers` | http | no | Extra headers (e.g. auth tokens) |
|
||||
| `toolCallTimeoutMs` | both | no | Timeout per `callTool` invocation (default 60000) |
|
||||
| `failOnStartupError` | both | no | Reject plugin activation when the initial connection or tool discovery fails (default `false`) |
|
||||
|
||||
## Tool naming
|
||||
|
||||
@@ -56,7 +57,7 @@ Every MCP tool has two names: the raw MCP name (sent on the wire in `tools/call`
|
||||
|
||||
## Behavior
|
||||
|
||||
- On connect: plugin activation awaits `listTools()` and registers each tool via `ctx.tools.register()` under its public name before the composition starts its first turn. Initial connection failure is logged and activates with no tools.
|
||||
- On connect: plugin activation awaits `listTools()` and registers each tool via `ctx.tools.register()` under its public name before the composition starts its first turn. Initial connection or discovery failure is always logged; it rejects activation when `failOnStartupError` is true and otherwise activates with no tools.
|
||||
- Listens for `notifications/tools/list_changed` → re-syncs; a failed re-sync keeps the previous generation registered.
|
||||
- Tool execute: `client.callTool({ name: rawName, arguments }, { signal })` with timeout + abort support—the public name is never sent to the server.
|
||||
- Canonical success is `{ content: JsonValue[], structuredContent? }`; complete JSON MCP blocks survive for programmatic callers. A supported advertised `outputSchema` validates `structuredContent`; unsupported schema vocabulary falls back to unconstrained `JsonValue`.
|
||||
|
||||
@@ -44,6 +44,7 @@ MCP 客户端桥接插件:连接外部 [Model Context Protocol](https://modelc
|
||||
| `url` | http | 是 | MCP 服务器 URL |
|
||||
| `headers` | http | 否 | 额外标头(例如认证 token) |
|
||||
| `toolCallTimeoutMs` | 两者 | 否 | 每次 `callTool` 调用的超时(默认 60000) |
|
||||
| `failOnStartupError` | 两者 | 否 | 初始连接或工具发现失败时拒绝插件激活(默认 `false`) |
|
||||
|
||||
## 工具命名
|
||||
|
||||
@@ -56,7 +57,7 @@ MCP 客户端桥接插件:连接外部 [Model Context Protocol](https://modelc
|
||||
|
||||
## 行为
|
||||
|
||||
- 连接时:插件激活会等待 `listTools()`,并在组合开始首个轮次前通过 `ctx.tools.register()` 以公开名称注册每个工具。初始连接失败会记录日志,插件仍会激活但不注册工具。
|
||||
- 连接时:插件激活会等待 `listTools()`,并在组合开始首个轮次前通过 `ctx.tools.register()` 以公开名称注册每个工具。初始连接或发现失败始终会记录日志;`failOnStartupError` 为 true 时拒绝激活,否则插件仍会激活但不注册工具。
|
||||
- 监听 `notifications/tools/list_changed` → 重新同步;同步失败时保留上一世代的注册。
|
||||
- 工具执行:`client.callTool({ name: rawName, arguments }, { signal })`,支持超时 + 中止;公开名称绝不会发给服务器。
|
||||
- 规范成功值是 `{ content: JsonValue[], structuredContent? }`;完整的 JSON MCP 块会保留给编程调用方。受支持且已声明的 `outputSchema` 会验证 `structuredContent`;不受支持的 schema 词汇会回退为不受约束的 `JsonValue`。
|
||||
|
||||
@@ -72,6 +72,8 @@ export interface StdioConfig {
|
||||
cwd: string
|
||||
/** Per-tool-call timeout in milliseconds. */
|
||||
toolCallTimeoutMs: number
|
||||
/** Fail plugin activation when the initial connection or tool discovery fails. */
|
||||
failOnStartupError: boolean
|
||||
}
|
||||
|
||||
/** Config for connecting to an MCP server over Streamable HTTP (SSE). */
|
||||
@@ -90,6 +92,8 @@ export interface StreamableHttpConfig {
|
||||
headers: Record<string, string>
|
||||
/** Per-tool-call timeout in milliseconds. */
|
||||
toolCallTimeoutMs: number
|
||||
/** Fail plugin activation when the initial connection or tool discovery fails. */
|
||||
failOnStartupError: boolean
|
||||
}
|
||||
|
||||
/** Configuration for one stdio or Streamable HTTP MCP server. */
|
||||
@@ -104,6 +108,7 @@ export const Config = z.union([
|
||||
env: z.dict(String).default({}),
|
||||
cwd: z.string().default(''),
|
||||
toolCallTimeoutMs: z.number().default(DEFAULT_TOOL_CALL_TIMEOUT_MS),
|
||||
failOnStartupError: z.boolean().default(false),
|
||||
}),
|
||||
z.object({
|
||||
transport: z.const('streamable-http'),
|
||||
@@ -111,6 +116,7 @@ export const Config = z.union([
|
||||
url: z.string().required(),
|
||||
headers: z.dict(String).default({}),
|
||||
toolCallTimeoutMs: z.number().default(DEFAULT_TOOL_CALL_TIMEOUT_MS),
|
||||
failOnStartupError: z.boolean().default(false),
|
||||
}),
|
||||
]) as unknown as z<Config>
|
||||
|
||||
@@ -118,11 +124,13 @@ export const Config = z.union([
|
||||
|
||||
/**
|
||||
* Connect one MCP server and publish its initial tool generation before activation.
|
||||
* This entry remains explicitly `async`: Cordis treats a prototype-bearing
|
||||
* ordinary function as a constructor, whose returned Promise is not startup work.
|
||||
* @param ctx - plugin context carrying the tool registry.
|
||||
* @param config - resolved transport and server namespace configuration.
|
||||
* @returns startup readiness after connection and initial tool discovery settle.
|
||||
*/
|
||||
export function apply(ctx: Context, config: Config): Promise<void> {
|
||||
export async function apply(ctx: Context, config: Config): Promise<void> {
|
||||
// Reserve the namespace first: a duplicate `serverName` fails THIS instance
|
||||
// at load with an actionable error and leaves the earlier instance intact.
|
||||
ctx.effect(() => {
|
||||
@@ -151,10 +159,10 @@ export function apply(ctx: Context, config: Config): Promise<void> {
|
||||
toolCallTimeoutMs: config.toolCallTimeoutMs,
|
||||
}
|
||||
|
||||
// Connect and set up tools. Errors during connect/first sync are logged,
|
||||
// not thrown (the plugin simply has no tools registered). `ready` resolves
|
||||
// to an accessor for the CURRENT disposer generation, so the effect
|
||||
// disposer below always unregisters the live set, not the first one.
|
||||
// Connect and set up tools. `ready` always settles to an outcome so rollback
|
||||
// can close a partially opened client even when strict startup later rejects.
|
||||
// Its accessor returns the CURRENT disposer generation, so disposal always
|
||||
// unregisters the live set, not the first one.
|
||||
const ready = (async () => {
|
||||
await client.connect(transport)
|
||||
|
||||
@@ -174,17 +182,20 @@ export function apply(ctx: Context, config: Config): Promise<void> {
|
||||
},
|
||||
)
|
||||
|
||||
return () => disposers
|
||||
return { getDisposers: () => disposers }
|
||||
})().catch((error: unknown) => {
|
||||
ctx.logger.error(`mcp-client(${config.serverName}): failed to connect: ${String(error)}`)
|
||||
return () => new Map<string, () => void>()
|
||||
return { getDisposers: () => new Map<string, () => void>(), error }
|
||||
})
|
||||
|
||||
ctx.effect(() => async () => {
|
||||
const live = await ready
|
||||
for (const dispose of live().values()) dispose()
|
||||
const outcome = await ready
|
||||
for (const dispose of outcome.getDisposers().values()) dispose()
|
||||
try { await client.close() } catch { /* transport already gone */ }
|
||||
}, 'mcp-client.connection')
|
||||
|
||||
return ready.then(() => undefined)
|
||||
const outcome = await ready
|
||||
if ('error' in outcome && config.failOnStartupError) {
|
||||
throw new Error(`mcp-client(${config.serverName}): initial connection or tool discovery failed`, { cause: outcome.error })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,6 +82,7 @@ const stdioConfig: Config = {
|
||||
env: {},
|
||||
cwd: '',
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
|
||||
// ---- Tests ----
|
||||
@@ -150,11 +151,30 @@ describe('apply (plugin lifecycle)', () => {
|
||||
expect(ctx.tools.get('remote')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps the Cordis plugin loading until initial discovery publishes its tools', async () => {
|
||||
const connection: PromiseWithResolvers<void> = Promise.withResolvers()
|
||||
mockConnect.mockImplementation(async () => {
|
||||
await connection.promise
|
||||
})
|
||||
const fiber = ctx.plugin({ name: 'mcp-client-lifecycle', inject, apply }, stdioConfig)
|
||||
let activated = false
|
||||
const activation = Promise.resolve(fiber).then(() => { activated = true })
|
||||
|
||||
await vi.waitFor(() => { expect(mockConnect).toHaveBeenCalled() })
|
||||
expect(activated).toBe(false)
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeUndefined()
|
||||
|
||||
connection.resolve()
|
||||
await activation
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
it('rejects a duplicate serverName at load and leaves the first instance intact', async () => {
|
||||
await apply(ctx, stdioConfig)
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
|
||||
expect(() => { void apply(ctx, stdioConfig) }).toThrow(/serverName "srv" is already in use/)
|
||||
await expect(apply(ctx, stdioConfig)).rejects.toThrow(/serverName "srv" is already in use/)
|
||||
// First instance unaffected.
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeDefined()
|
||||
})
|
||||
@@ -204,6 +224,19 @@ describe('apply (plugin lifecycle)', () => {
|
||||
expect(mockClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects activation and still closes the client when startup failure is configured as fatal', async () => {
|
||||
mockConnect.mockRejectedValue(new Error('connection refused'))
|
||||
await expect(apply(ctx, {
|
||||
...stdioConfig,
|
||||
failOnStartupError: true,
|
||||
})).rejects.toThrow('initial connection or tool discovery failed')
|
||||
|
||||
expect(mockListTools).not.toHaveBeenCalled()
|
||||
expect(ctx.tools.get('mcp__srv__remote')).toBeUndefined()
|
||||
await ctx.fiber.dispose()
|
||||
expect(mockClose).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('re-syncs tools on ToolListChanged notification', async () => {
|
||||
await apply(ctx, stdioConfig)
|
||||
|
||||
@@ -275,6 +308,7 @@ describe('apply (plugin lifecycle)', () => {
|
||||
url: 'http://localhost:3000/mcp',
|
||||
headers: { Authorization: 'Bearer x' },
|
||||
toolCallTimeoutMs: 30_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
|
||||
await apply(ctx, httpConfig)
|
||||
|
||||
@@ -75,6 +75,7 @@ describe('fixture server — controlled scenarios', () => {
|
||||
env: {},
|
||||
cwd: packageDir,
|
||||
toolCallTimeoutMs: 15_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
@@ -164,10 +165,11 @@ describe('fixture server — duplicate serverName', () => {
|
||||
env: {},
|
||||
cwd: packageDir,
|
||||
toolCallTimeoutMs: 15_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
await apply(ctx, config)
|
||||
|
||||
expect(() => { void apply(ctx, config) }).toThrow(/serverName "dup" is already in use/)
|
||||
await expect(apply(ctx, config)).rejects.toThrow(/serverName "dup" is already in use/)
|
||||
|
||||
await ctx.fiber.dispose()
|
||||
await sleep(200)
|
||||
@@ -185,6 +187,7 @@ describe('fixture server — disposal', () => {
|
||||
env: {},
|
||||
cwd: packageDir,
|
||||
toolCallTimeoutMs: 15_000,
|
||||
failOnStartupError: false,
|
||||
})
|
||||
|
||||
// Tools are registered before dispose.
|
||||
@@ -210,6 +213,7 @@ describe('server-everything — official test server', () => {
|
||||
env: {},
|
||||
cwd: '',
|
||||
toolCallTimeoutMs: 30_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
@@ -277,6 +281,7 @@ describe('server-filesystem — real filesystem operations', () => {
|
||||
env: {},
|
||||
cwd: '',
|
||||
toolCallTimeoutMs: 30_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
await apply(ctx, config)
|
||||
}, 60_000)
|
||||
@@ -393,6 +398,7 @@ describe('streamable-http — in-process MCP server', () => {
|
||||
url: baseUrl,
|
||||
headers: { Authorization: 'Bearer e2e-test-token' },
|
||||
toolCallTimeoutMs: 15_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
await apply(ctx, config)
|
||||
}, 30_000)
|
||||
|
||||
@@ -713,6 +713,7 @@ describe('createTransport', () => {
|
||||
env: {},
|
||||
cwd: '/tmp',
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
const transport = createTransport(config)
|
||||
expect(transport).toBeDefined()
|
||||
@@ -727,6 +728,7 @@ describe('createTransport', () => {
|
||||
url: 'http://localhost:3000/mcp',
|
||||
headers: {},
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
const transport = createTransport(config)
|
||||
expect(transport).toBeDefined()
|
||||
@@ -741,6 +743,7 @@ describe('createTransport', () => {
|
||||
url: 'http://localhost:3000/mcp',
|
||||
headers: { Authorization: 'Bearer token' },
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
const transport = createTransport(config)
|
||||
expect(transport).toBeDefined()
|
||||
@@ -764,6 +767,7 @@ describe('createTransport', () => {
|
||||
env: { EXTRA: 'injected' },
|
||||
cwd: '',
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
// createTransport internally calls buildChildEnv; we verify by inspecting
|
||||
// the constructed StdioClientTransport. Since we can't inspect private fields
|
||||
@@ -791,6 +795,7 @@ describe('createTransport', () => {
|
||||
env: { CUSTOM: 'value' },
|
||||
cwd: '',
|
||||
toolCallTimeoutMs: 60_000,
|
||||
failOnStartupError: false,
|
||||
}
|
||||
const transport = createTransport(config)
|
||||
expect(transport).toBeDefined()
|
||||
|
||||
Reference in New Issue
Block a user