diff --git a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.i18n.yaml b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.i18n.yaml index e0aac526cc..ffc985cc70 100644 --- a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.i18n.yaml @@ -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 .agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.md -2026-08-06-mcp-client-auto-reconnect.md: e371fa13dc7bc5330f658f4d6e864969baf93cf0 -2026-08-06-mcp-client-auto-reconnect.zh.md: eb6e67dabd7c6115b9ecb507029ffa7d45338466 +2026-08-06-mcp-client-auto-reconnect.md: 75e17da716f817306bb30322678d26b0e18a5aff +2026-08-06-mcp-client-auto-reconnect.zh.md: 0ba06d17dca895be8449eb771d34dc096307d176 diff --git a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.md b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.md index e371fa13dc..75e17da716 100644 --- a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.md +++ b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.md @@ -14,7 +14,7 @@ The [MCP client](2026-07-07-mcp-client-plugin.md) connected once at plugin load. **Trigger.** The supervisor arms `client.onclose` per generation. The SDK fires it when the stdio child exits, so a crash is observed without polling. `StreamableHTTPClientTransport` fires `onclose` only for deliberate closes — it owns its internal SSE-stream recovery and surfaces request failures per call — so HTTP servers are effectively outside supervisor restarts; the package README records that limitation. -**Generations without interleaving.** Each attempt builds a fresh transport and `Client` (the SDK binds a Protocol to one transport for life). One per-supervisor queue serializes every `syncTools` call — initial syncs and `list_changed` re-syncs across all generations — and an `isCurrent` fence makes stale generations inert, so no two syncs can interleave the dispose-previous/register-next swap (which would double-dispose one generation and leak another). The queue also closes a pre-existing race where two rapid `list_changed` notifications re-synced concurrently. Failure signals are idempotent per generation: a connect rejection racing its own transport close schedules exactly one retry. A failed attempt cannot enter backoff until both `Client.close()` settles and the transport reports `onclose`, which for stdio proves the child exited; a missing close signal stops reconnection after the SDK's bounded termination window instead of allowing two server processes to overlap. Disposal uses the same bounded close-signal barrier and reports an incomplete shutdown without ever restarting. +**Generations without interleaving.** Each attempt builds a fresh transport and `Client` (the SDK binds a Protocol to one transport for life). One per-supervisor queue serializes every `syncTools` call — initial syncs and `list_changed` re-syncs across all generations — and an `isCurrent` fence makes stale generations inert, so no two syncs can interleave the dispose-previous/register-next swap (which would double-dispose one generation and leak another). The queue also closes a pre-existing race where two rapid `list_changed` notifications re-synced concurrently. The activation attempt, rather than the first queue entrant, explicitly owns strict startup registration: an early `list_changed` notification uses contained re-sync semantics and cannot consume `failOnStartupError`. Failure signals are idempotent per generation: a connect rejection racing its own transport close schedules exactly one retry. A failed attempt cannot enter backoff until both `Client.close()` settles and the transport reports `onclose`, which for stdio proves the child exited; a missing close signal stops reconnection after the SDK's bounded termination window instead of allowing two server processes to overlap. Disposal uses the same bounded close-signal barrier and reports an incomplete shutdown without ever restarting. **Bounded backoff with an outage budget.** Delays double from `initialDelayMs` up to `maxDelayMs`. One outage shares `maxAttempts` consecutive failed attempts; exhaustion unregisters the server's tools, logs at error level, and stops until disposal or reload. A connection that survives past the stability window — `maxDelayMs`, derived rather than a fifth tunable, as the longest configured backoff spacing — resets the budget, so an occasionally-crashing server recovers indefinitely while a crash loop whose connects briefly succeed cannot launder its budget into a restart storm. @@ -38,7 +38,7 @@ The [MCP client](2026-07-07-mcp-client-plugin.md) connected once at plugin load. ## Testing -Unit (`tests/reconnect.spec.ts`, mocked SDK): recovery swaps generations without duplication or leaks and serves post-recovery calls, failed initialization waits for the old generation's close signal and fails closed when that signal never arrives, disposal waits for the same signal with a bounded incomplete-shutdown path, the failure cap unregisters tools and stops, dispose cancels a pending backoff and quiesces an in-flight sync, a close after dispose schedules nothing, disabled mode keeps the v1 behavior, the stability window resets the budget while a crash loop exhausts it, double failure signals schedule one retry, stale generations and handlers are inert, and `resolveReconnectPolicy` rejects each invalid bound. E2E (`tests/mcp-client.e2e.ts`, keyless): the fixture server gained a `crash` tool that replies then exits; real-process tests prove a stdio crash recovers end to end and that unloading the plugin mid-outage stops reconnection promptly. Snapshot: deliberately none, per the original note's rationale — reconnection adds no new presentation shape, and a snapshot composition spawning a crashing server would make replays timing-dependent. +Unit (`tests/reconnect.spec.ts`, mocked SDK): recovery swaps generations without duplication or leaks and serves post-recovery calls, strict startup registration survives a pre-connect `list_changed` notification, failed initialization waits for the old generation's close signal and fails closed when that signal never arrives, disposal waits for the same signal with a bounded incomplete-shutdown path, the failure cap unregisters tools and stops, dispose cancels a pending backoff and quiesces an in-flight sync, a close after dispose schedules nothing, disabled mode keeps the v1 behavior, the stability window resets the budget while a crash loop exhausts it, double failure signals schedule one retry, stale generations and handlers are inert, and `resolveReconnectPolicy` rejects each invalid bound. E2E (`tests/mcp-client.e2e.ts`, keyless): the fixture server gained a `crash` tool that replies then exits; real-process tests prove a stdio crash recovers end to end and that unloading the plugin mid-outage stops reconnection promptly. Snapshot: deliberately none, per the original note's rationale — reconnection adds no new presentation shape, and a snapshot composition spawning a crashing server would make replays timing-dependent. ## Consequences diff --git a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.zh.md b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.zh.md index eb6e67dabd..0ba06d17dc 100644 --- a/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.zh.md +++ b/.agents/notes/implemented/feature/2026-08-06-mcp-client-auto-reconnect.zh.md @@ -14,7 +14,7 @@ Status: implemented **触发条件。** 监督器在每一代上挂载 `client.onclose`。SDK 在 stdio 子进程退出时触发该回调，因此崩溃无需轮询即可感知。`StreamableHTTPClientTransport` 仅在主动关闭时触发 `onclose`——它内部拥有自己的 SSE（Server-Sent Events）流恢复机制，并将请求失败以逐调用方式暴露——因此 HTTP 服务器实际上不在监督器的重启范围内；包 README 记录了该限制。 -**代隔离，无交错。** 每次尝试构建一个全新的 transport 和 `Client`（SDK 将一个 Protocol 绑定到一个 transport 上终身使用）。每个监督器内部有一个队列将所有 `syncTools` 调用串行化——跨所有代的初始同步和 `list_changed` 再同步——`isCurrent` 栅栏使过时的代变为惰性，从而确保不会有两次同步交错执行 dispose 上一代/注册下一代的切换（否则会对同一代执行两次 dispose 并泄漏另一代）。该队列还消除了一个先前存在的竞态：两次快速的 `list_changed` 通知同时触发重新同步。失败信号按代幂等：一次连接拒绝与其自身 transport 关闭竞态时，仅调度恰好一次重试。失败尝试只有在 `Client.close()` 结算且 transport 报告 `onclose` 后才能进入退避；对 stdio 而言，`onclose` 证明子进程已退出；若关闭信号始终未到，则在 SDK 的有界终止窗口结束后停止重连，而不是允许两个服务器进程重叠运行。dispose 使用同一个有界关闭信号屏障；若关停未完成则予以报告，且绝不重启。 +**代隔离，无交错。** 每次尝试构建一个全新的 transport 和 `Client`（SDK 将一个 Protocol 绑定到一个 transport 上终身使用）。每个监督器内部有一个队列将所有 `syncTools` 调用串行化——跨所有代的初始同步和 `list_changed` 再同步——`isCurrent` 栅栏使过时的代变为惰性，从而确保不会有两次同步交错执行 dispose 上一代/注册下一代的切换（否则会对同一代执行两次 dispose 并泄漏另一代）。该队列还消除了一个先前存在的竞态：两次快速的 `list_changed` 通知同时触发重新同步。严格启动注册由激活尝试本身显式拥有，而非由首个入队者拥有；提前到达的 `list_changed` 采用故障隔离的再同步语义，不能消费 `failOnStartupError`。失败信号按代幂等：一次连接拒绝与其自身 transport 关闭竞态时，仅调度恰好一次重试。失败尝试只有在 `Client.close()` 结算且 transport 报告 `onclose` 后才能进入退避；对 stdio 而言，`onclose` 证明子进程已退出；若关闭信号始终未到，则在 SDK 的有界终止窗口结束后停止重连，而不是允许两个服务器进程重叠运行。dispose 使用同一个有界关闭信号屏障；若关停未完成则予以报告，且绝不重启。 **有界退避与故障预算。** 延迟从 `initialDelayMs` 起逐次翻倍，上限为 `maxDelayMs`。一次故障期间共享 `maxAttempts` 次连续失败尝试的预算；耗尽后注销该服务器的工具、以 error 级别记录日志并停止，直到 dispose 或重新加载。连接在存活超过稳定窗口——即 `maxDelayMs`，作为最长退避间隔从配置推导得出而非作为第五个独立调参项——之后重置预算；因此偶尔崩溃的服务器可无限恢复，而连接短暂成功后立即再次崩溃的循环无法将其预算洗白为重启风暴。 @@ -38,7 +38,7 @@ Status: implemented ## 测试 -单元测试（`tests/reconnect.spec.ts`，mock SDK）：恢复在不产生重复或泄漏的前提下切换代并服务恢复后的调用、初始化失败会等待旧代的关闭信号，若该信号始终未到则停止重连、dispose 同样等待同一关闭信号，并在有界等待到期时报告关停未完成、失败上限注销工具并停止、dispose 取消待执行的退避并使进行中的同步完全停稳、dispose 后的关闭不调度任何操作、禁用模式保持 v1 行为、稳定窗口重置预算而崩溃循环耗尽预算、双重失败信号仅调度一次重试、过时的代和处理器为惰性、`resolveReconnectPolicy` 拒绝每个无效边界值。E2E（`tests/mcp-client.e2e.ts`，无需密钥）：fixture 服务器新增了一个 `crash` 工具（先回复再退出）；真实进程测试证明 stdio 崩溃端到端恢复，以及在故障期间卸载插件能立即停止重连。快照：刻意不做，原因与原 Agent Note 相同——重连不引入新的展示形态，而在快照组合中 spawn 崩溃服务器会使回放依赖时序。 +单元测试（`tests/reconnect.spec.ts`，mock SDK）：恢复在不产生重复或泄漏的前提下切换代并服务恢复后的调用、严格启动注册在连接前收到 `list_changed` 通知后仍然生效、初始化失败会等待旧代的关闭信号，若该信号始终未到则停止重连、dispose 同样等待同一关闭信号，并在有界等待到期时报告关停未完成、失败上限注销工具并停止、dispose 取消待执行的退避并使进行中的同步完全停稳、dispose 后的关闭不调度任何操作、禁用模式保持 v1 行为、稳定窗口重置预算而崩溃循环耗尽预算、双重失败信号仅调度一次重试、过时的代和处理器为惰性、`resolveReconnectPolicy` 拒绝每个无效边界值。E2E（`tests/mcp-client.e2e.ts`，无需密钥）：fixture 服务器新增了一个 `crash` 工具（先回复再退出）；真实进程测试证明 stdio 崩溃端到端恢复，以及在故障期间卸载插件能立即停止重连。快照：刻意不做，原因与原 Agent Note 相同——重连不引入新的展示形态，而在快照组合中 spawn 崩溃服务器会使回放依赖时序。 ## 后果 diff --git a/packages/mcp/mcp-client/src/connection.ts b/packages/mcp/mcp-client/src/connection.ts index 2760324a20..3afd05b538 100644 --- a/packages/mcp/mcp-client/src/connection.ts +++ b/packages/mcp/mcp-client/src/connection.ts @@ -133,7 +133,6 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe const startupOpts: ToolBridgeOptions = config.failOnStartupError ? { ...opts, registrationFailure: 'throw' } : opts - let isFirstSync = true let disposed = false /** Current generation: the connecting or connected client; undefined during backoff waits and after final failure. */ @@ -160,9 +159,7 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe * generation and leak another). */ let syncChain: Promise = Promise.resolve() - function enqueueSync(generation: Client): Promise { - const syncOpts = isFirstSync ? startupOpts : opts - isFirstSync = false + function enqueueSync(generation: Client, syncOpts: ToolBridgeOptions = opts): Promise { const run = syncChain.then(async () => { if (!isCurrent(generation)) return disposers = await syncTools(generation, ctx, syncOpts, disposers) @@ -219,7 +216,7 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe ctx.logger.warn(`${label}: connection lost; reconnecting in ${delayMs}ms (attempt ${failedAttempts}/${policy.maxAttempts})`) reconnectTimer = setTimeout(() => { reconnectTimer = undefined - settling = connectGeneration() + settling = connectGeneration(false) }, delayMs) // An armed reconnect timer must never hold the process open on its own. reconnectTimer.unref() @@ -228,10 +225,14 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe /** * One connection attempt: fresh transport + client (the MCP SDK binds a * Protocol to one transport for life), connect, then queue the initial tool - * sync. Every failure funnels through {@link generationDown}; success arms - * the onclose-driven disconnect path. Never rejects. + * sync. The startup flag belongs to the attempt rather than the shared sync + * queue, so an early notification cannot consume strict startup semantics. + * Every failure funnels through {@link generationDown}; success arms the + * onclose-driven disconnect path. Never rejects. + * + * @param startup - Whether this is the plugin's activation attempt. */ - async function connectGeneration(): Promise { + async function connectGeneration(startup: boolean): Promise { const generation = new Client( { name: 'dsh-mcp-client', version: '0.0.1' }, { capabilities: {} }, @@ -272,7 +273,7 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe generationDown(generation) return } - await enqueueSync(generation) + await enqueueSync(generation, startup ? startupOpts : opts) } catch (error) { if (firstAttemptError === undefined) firstAttemptError = error // Disposal clears current ownership before it closes the generation, so @@ -302,7 +303,7 @@ export function startConnection(ctx: Context, config: Config, policy: ResolvedRe } /** The in-flight (or last settled) connection attempt; dispose awaits it for quiescence. */ - let settling = connectGeneration() + let settling = connectGeneration(true) // The ready promise settles when the first attempt finishes (regardless of // success). If the first attempt fails and reconnect is enabled, the diff --git a/packages/mcp/mcp-client/tests/apply.spec.ts b/packages/mcp/mcp-client/tests/apply.spec.ts index b6a9409c63..2d836d710e 100644 --- a/packages/mcp/mcp-client/tests/apply.spec.ts +++ b/packages/mcp/mcp-client/tests/apply.spec.ts @@ -288,6 +288,32 @@ describe('apply (plugin lifecycle)', () => { expect(mockClose).toHaveBeenCalled() }) + it('preserves strict startup registration when list_changed arrives before connect resolves', async () => { + ctx.tools.register({ + name: 'mcp__srv__remote', + description: 'Foreign squatter', + parameters: { type: 'object' }, + output: { + schema: { type: 'string' }, + render: (_args, value) => [{ type: 'text', text: value as string }], + }, + execute: async () => 'foreign', + }) + mockConnect.mockImplementation(async () => { + const handler = mockSetNotificationHandler.mock.calls[0]![1] as () => Promise + await handler() + }) + + await expect(apply(ctx, { + ...stdioConfig, + failOnStartupError: true, + })).rejects.toThrow('initial connection or tool synchronization failed') + + expect(mockListTools).toHaveBeenCalledTimes(2) + expect(ctx.tools.get('mcp__srv__remote')?.description).toBe('Foreign squatter') + await ctx.fiber.dispose() + }) + it('re-syncs tools on ToolListChanged notification', async () => { await apply(ctx, stdioConfig)