diff --git a/CLAUDE.md b/CLAUDE.md index 61487fef..75efa1ad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -520,7 +520,7 @@ The dev proxy (`tools/dev-proxy/dev-proxy.mjs`) is a lightweight MCP proxy that **When to use**: Whenever you are actively developing mcp-debugger — making code changes, adding adapters, or installing new toolchains (Go, etc.) that need to be picked up by the running server. -**How it works**: After code changes, call the `dev_rebuild_and_restart` tool. The proxy kills the backend, runs `npm run build`, spawns a fresh process, and reconnects — all transparently. If the backend crashes, dev tools remain available to bring it back. +**How it works**: After code changes, call the `dev_rebuild_and_restart` tool. The proxy kills the backend, runs `npm run build`, spawns a fresh process, and reconnects — all transparently. If the backend crashes, dev tools remain available to bring it back. The proxy speaks both protocol eras to its client through the SDK's stdio entry (a probing client lands on 2026-07-28), negotiates the era with its backend on either transport (over stdio in place, on the backend's own pipe), translates resource subscriptions across eras and re-expresses them after every restart, and starts the backend on the first request that needs it — never when the process starts, because a v2 client's `auto` probe runs on a disposable sibling proxy process (issue #1025, step 2; `tools/dev-proxy/README.md`). **Configuration** (env vars, all optional): - `DEV_PROXY_PORT` — Backend HTTP port (default: 3001; used by `http` mode) @@ -584,7 +584,7 @@ Each session also exposes its captured output as an MCP resource (`debug://sessi **Dev proxy only** (these 3 tools are injected by the dev proxy process itself, not by the main mcp-debugger server): - `dev_restart_debugger` - Restart the backend (pass `rebuild: true` to build first) - `dev_rebuild_and_restart` - Run `npm run build` then restart the backend -- `dev_server_status` - Check backend state, PID, uptime, transport, project root, port, and display-safe backend environment overrides +- `dev_server_status` - Check backend state, PID, uptime, transport, protocol era and version, project root, port, and display-safe backend environment overrides ### Troubleshooting MCP Connection - **If server shows "Failed to connect"**: diff --git a/changelog.d/1025-dev-proxy.changed.md b/changelog.d/1025-dev-proxy.changed.md new file mode 100644 index 00000000..53e2f054 --- /dev/null +++ b/changelog.d/1025-dev-proxy.changed.md @@ -0,0 +1 @@ +**The dev proxy (`tools/dev-proxy`) speaks both MCP protocol eras and starts its backend on first use.** The client-facing side is served through the SDK's stdio entry, so a probing client — Claude Code's v2 runtime — lands on the current 2026-07-28 revision while an `initialize` client keeps the 2025 era; towards its backend the proxy negotiates the era too, on either transport — over stdio on the backend's own pipe, so the backend still starts once and an older build falls back to the 2025 era — and `dev_server_status` reports `protocolEra`/`protocolVersion`, with a note before the first start that the backend starts on first use. Resource subscriptions are translated across eras (one `subscriptions/listen` to a 2026 backend, one `resources/subscribe` per URI to a 2025 one) and re-expressed after every backend restart, so a `dev_rebuild_and_restart` no longer loses them; a subscription to a resource the restarted backend no longer has is dropped instead of failing the rest. The backend now starts on the first request that needs it rather than at process start: a v2 client's `auto` negotiation over stdio probes `server/discover` on a disposable sibling proxy process, which used to start a backend of its own and race the real proxy for `DEV_PROXY_PORT` (measured: two backends per connect). The proxy exits when the SDK transport closes the wire by itself, not only on stdin EOF. Completes step 2 of #1025 for the development loop. diff --git a/tests/fixtures/dev-proxy/real-backend.mjs b/tests/fixtures/dev-proxy/real-backend.mjs new file mode 100644 index 00000000..b951ab11 --- /dev/null +++ b/tests/fixtures/dev-proxy/real-backend.mjs @@ -0,0 +1,18 @@ +// The real mcp-debugger server as a dev-proxy stdio backend, counting its own +// starts (tests/integration/dev-proxy-eras.test.ts). The legacy fixture beside +// this one speaks only the 2025 era; this is the built server, which speaks +// both, so the proxy's negotiation with a stdio backend can be observed — and +// the event line shows whether the proxy spawned it once or twice. +import { appendFileSync } from 'node:fs'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +const events = process.env.DEV_PROXY_FIXTURE_EVENTS; +if (events) { + appendFileSync(events, `${JSON.stringify({ event: 'backend-start', pid: process.pid })}\n`); +} + +const entry = path.resolve(process.env.DEV_PROXY_ROOT ?? process.cwd(), 'dist/index.js'); +// The entry runs its CLI only as the main module, which it tells from argv. +process.argv = [process.argv[0], entry, 'stdio']; +await import(pathToFileURL(entry).href); diff --git a/tests/fixtures/dev-proxy/startup-backend.mjs b/tests/fixtures/dev-proxy/startup-backend.mjs index c905fafe..f7aa9606 100644 --- a/tests/fixtures/dev-proxy/startup-backend.mjs +++ b/tests/fixtures/dev-proxy/startup-backend.mjs @@ -31,7 +31,7 @@ if (releaseFile) { } } -const server = new Server({ name: 'startup-fixture', version: '1.0.0' }, { capabilities: { tools: {}, resources: {} } }); +const server = new Server({ name: 'startup-fixture', version: '1.0.0' }, { capabilities: { tools: {}, resources: { subscribe: true } } }); server.setRequestHandler('tools/list', async () => ({ tools: [{ name: process.env.DEV_PROXY_FIXTURE_TOOL || 'fixture_tool', @@ -42,6 +42,18 @@ server.setRequestHandler('tools/list', async () => ({ // Answering at all is the point: it proves the proxy forwarded the call rather // than refusing it while the backend was still starting. server.setRequestHandler('tools/call', async (request) => { + // `touch: ` makes this 2025 backend send resources/updated for a URI — + // only one the proxy has subscribed, like the real server's per-connection + // subscribe set (issue #1025, step 2: the proxy's subscription relay across + // eras). An unsubscribed touch is refused, so a test cannot pass on a ping + // nobody asked for. + const touch = request.params.arguments?.touch; + if (typeof touch === 'string') { + if (!subscribed.has(touch)) { + return { content: [{ type: 'text', text: `not subscribed: ${touch}` }], isError: true }; + } + await server.sendResourceUpdated({ uri: touch }); + } if (request.params.arguments?.floodStderr) { // More than a pipe's capacity: answering proves the proxy drained stderr // while a build was running, rather than leaving this write blocked. @@ -57,5 +69,16 @@ server.setRequestHandler('resources/list', async () => ({ server.setRequestHandler('resources/read', async () => ({ contents: [{ uri: 'fixture://status', text: process.env.DEV_PROXY_FIXTURE_TOOL || 'fixture_tool' }], })); +// A 2025 backend's subscriptions: the per-connection set the real server keeps +// (lost with the process, which is why the proxy re-sends them after a restart). +const subscribed = new Set(); +server.setRequestHandler('resources/subscribe', async (request) => { + subscribed.add(request.params.uri); + return {}; +}); +server.setRequestHandler('resources/unsubscribe', async (request) => { + subscribed.delete(request.params.uri); + return {}; +}); process.stdin.on('end', () => process.exit(0)); await server.connect(new StdioServerTransport()); diff --git a/tests/integration/dev-proxy-eras.test.ts b/tests/integration/dev-proxy-eras.test.ts new file mode 100644 index 00000000..466d5d97 --- /dev/null +++ b/tests/integration/dev-proxy-eras.test.ts @@ -0,0 +1,157 @@ +/** + * The dev proxy on both protocol eras (issue #1025, step 2, PR 5), driven as + * Claude Code drives it: a v2 client over the SDK's own StdioClientTransport. + * + * In `auto` mode that client probes server/discover on a DISPOSABLE SIBLING + * process spawned from the same parameters, then starts the real one. Measured + * before this change: the sibling ran the proxy's eager backend start too — + * two backends per connect, both racing ensurePortFree on one port. Now the + * backend starts on the first request that reaches a handler, which a probe + * never is. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { mkdtemp, readFile, rm } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { Client } from '@modelcontextprotocol/client'; +import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; + +const root = path.resolve(__dirname, '../..'); +const devTools = ['dev_restart_debugger', 'dev_rebuild_and_restart', 'dev_server_status']; +const clients: Client[] = []; +const directories: string[] = []; + +afterEach(async () => { + for (const client of clients.splice(0)) await client.close().catch(() => undefined); + for (const dir of directories.splice(0)) await rm(dir, { recursive: true, force: true }); +}); + +type Mode = 'auto' | 'legacy' | { pin: '2026-07-28' }; + +async function connect(mode: Mode, env: Record = {}) { + const dir = await mkdtemp(path.join(os.tmpdir(), 'mcp-proxy-eras-')); + directories.push(dir); + const events = path.join(dir, 'events.ndjson'); + const client = new Client({ name: 'eras-test', version: '1.0.0' }, { versionNegotiation: { mode } }); + clients.push(client); + // The SDK's base transport, exactly as Claude Code uses it: the sibling path + // runs only for the base class (a subclass probes in place). + const transport = new StdioClientTransport({ + command: process.execPath, + args: [path.join(root, 'tools/dev-proxy/dev-proxy.mjs')], + cwd: root, + env: { + ...process.env, + DEV_PROXY_ROOT: root, + DEV_PROXY_BACKEND_TRANSPORT: 'stdio', + DEV_PROXY_BACKEND_CMD: `"${process.execPath}" "${path.join(root, 'tests/fixtures/dev-proxy/startup-backend.mjs')}"`, + DEV_PROXY_FIXTURE_EVENTS: events, + DEV_PROXY_FIXTURE_FAIL: '0', + DEV_PROXY_FIXTURE_RELEASE: '', + DEV_PROXY_FIXTURE_TOOL: 'fixture_tool', + ...env, + }, + stderr: 'pipe', + }); + transport.stderr?.on('data', () => {}); + await client.connect(transport); + const backendStarts = async () => (await readFile(events, 'utf8').catch(() => '')) + .split('\n').filter((line) => line.includes('"backend-start"')).length; + return { client, backendStarts }; +} + +describe('dev-proxy protocol eras (issue #1025, step 2)', () => { + it('an auto-negotiating client lands on 2026-07-28, and only the real connection starts a backend, on its first request', async () => { + const { client, backendStarts } = await connect('auto'); + + expect(client.getProtocolEra()).toBe('modern'); + expect(client.getNegotiatedProtocolVersion()).toBe('2026-07-28'); + // The sibling probe and the real connect have both happened; neither started a backend. + // (An eager sibling start might not have appended its event yet, so this first read is + // a probabilistic detector; the two reads below are the deterministic ones.) + expect(await backendStarts()).toBe(0); + + const status = await client.callTool({ name: 'dev_server_status', arguments: {} }); + const text = (status.content as Array<{ text: string }>)[0].text; + // "stopped" alone reads like a crash; before the first start the status says why. + expect(JSON.parse(text)).toMatchObject({ + state: 'stopped', + protocolEra: null, + note: expect.stringContaining('starts on the first request that needs it'), + }); + expect(await backendStarts()).toBe(0); + + const listed = await client.listTools(); + expect(listed.tools.map((tool) => tool.name)).toEqual(['fixture_tool', ...devTools]); + expect(await backendStarts()).toBe(1); + + // The fixture backend is a 2025 server: the status says which era the backend speaks. + const after = await client.callTool({ name: 'dev_server_status', arguments: {} }); + const running = JSON.parse((after.content as Array<{ text: string }>)[0].text); + expect(running).toMatchObject({ state: 'running', protocolEra: 'legacy', protocolVersion: '2025-11-25' }); + expect(running).not.toHaveProperty('note'); + }); + + it('drops the first-use note once the backend has been started any way at all, a dev restart included', async () => { + const { client, backendStarts } = await connect('legacy'); + + // The first request this proxy handles is a dev tool: it starts the backend + // without going through the deferred first start. + const restarted = await client.callTool({ name: 'dev_restart_debugger', arguments: {} }); + expect(restarted.isError, JSON.stringify(restarted.content)).not.toBe(true); + expect(await backendStarts()).toBe(1); + + const status = await client.callTool({ name: 'dev_server_status', arguments: {} }); + const body = JSON.parse((status.content as Array<{ text: string }>)[0].text); + expect(body.state).toBe('running'); + expect(body).not.toHaveProperty('note'); + }); + + it('a 2025 client keeps working unchanged', async () => { + const { client, backendStarts } = await connect('legacy'); + + expect(client.getProtocolEra()).toBe('legacy'); + expect((await client.listTools()).tools.map((tool) => tool.name)).toEqual(['fixture_tool', ...devTools]); + expect(await backendStarts()).toBe(1); + }); + + // The proxy is itself a v2 client of its backend. Over stdio the SDK's `auto` + // mode probes on a disposable sibling process for its base transport — a second + // backend — so the proxy used to pin a stdio backend to the 2025 era and never + // exercise the server's modern stdio leg. A subclassed transport probes in place + // (documented SDK behaviour): one process, and a legacy server falls back to + // `initialize` on the same pipe, which the first case above pins (the fixture + // is a 2025 server: one start, era legacy). + it('negotiates with a stdio backend in place: the real server lands on 2026-07-28, started once', async () => { + const { client, backendStarts } = await connect('legacy', { + DEV_PROXY_BACKEND_CMD: `"${process.execPath}" "${path.join(root, 'tests/fixtures/dev-proxy/real-backend.mjs')}"`, + }); + + const listed = await client.listTools(); + expect(listed.tools.map((tool) => tool.name)).toEqual(expect.arrayContaining(['create_debug_session', ...devTools])); + + const status = await client.callTool({ name: 'dev_server_status', arguments: {} }); + expect(JSON.parse((status.content as Array<{ text: string }>)[0].text)).toMatchObject({ + state: 'running', backendTransport: 'stdio', protocolEra: 'modern', protocolVersion: '2026-07-28', + }); + expect(await backendStarts()).toBe(1); + }, 60_000); + + it('a 2026-07-28 client hears a backend resources/updated on its listen stream through the relay', async () => { + const { client } = await connect({ pin: '2026-07-28' }); + const updated: string[] = []; + client.setNotificationHandler('notifications/resources/updated', async (n) => { updated.push(n.params.uri); }); + await client.listTools(); + const subscription = await client.listen({ resourceSubscriptions: ['fixture://status'] }); + + // The fixture backend (2025) sends resources/updated for a touched URI only + // if the proxy subscribed it. A 2026 client's listen set is served by the SDK + // entry and never reaches the proxy, so while a 2026 client is pinned the + // proxy subscribes the backend to every resource it lists; the front's own + // router then delivers only what this client listened for. + const touched = await client.callTool({ name: 'fixture_tool', arguments: { touch: 'fixture://status' } }); + expect(touched.isError, JSON.stringify(touched.content)).not.toBe(true); + await expect.poll(() => updated, { timeout: 5000 }).toEqual(['fixture://status']); + await subscription.close(); + }); +}); diff --git a/tests/unit/dev-proxy/backend-env.test.ts b/tests/unit/dev-proxy/backend-env.test.ts index 42fb0327..8794795c 100644 --- a/tests/unit/dev-proxy/backend-env.test.ts +++ b/tests/unit/dev-proxy/backend-env.test.ts @@ -80,3 +80,4 @@ describe('dev-proxy backend environment overrides', () => { expect(result.MCP_EXIT_ON_STDIN_CLOSE).toBe('1'); }); }); + diff --git a/tests/unit/dev-proxy/backend-stdio-transport.test.ts b/tests/unit/dev-proxy/backend-stdio-transport.test.ts new file mode 100644 index 00000000..ebfcbdf4 --- /dev/null +++ b/tests/unit/dev-proxy/backend-stdio-transport.test.ts @@ -0,0 +1,96 @@ +/** + * The dev proxy's stdio transport to its backend (issue #1025, step 2). + * + * It is a subclass of the SDK's transport so that `auto` negotiation probes on + * the backend's own pipe instead of on a second backend process. That has a + * cost the SDK does not cover: under `auto` negotiation the SDK does not watch + * the signal given to connect() while its probe is unanswered (measured, client + * 2.4.0: `legacy` rejects the connect and closes the transport on an abort, + * `auto` leaves the connect pending and the child running, sibling path and + * in place alike). So a start that is still negotiating when the proxy shuts + * down hangs, and the backend it spawned outlives the proxy — CI caught it on + * Linux in the rebuild suite's shutdown cases. The transport therefore closes + * itself on the proxy's shutdown signal, as the SDK does by itself in + * `legacy` mode. + */ +import { getEventListeners } from 'node:events'; +import { afterEach, describe, expect, it } from 'vitest'; +import { Client } from '@modelcontextprotocol/client'; +import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; +import { BackendStdioTransport } from '../../../tools/dev-proxy/backend-stdio-transport.mjs'; + +// A backend that never answers: what a shutdown finds while a start is still negotiating. +const SILENT = { command: process.execPath, args: ['-e', 'setInterval(() => {}, 1000)'] }; + +const isAlive = (pid: number): boolean => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +}; +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +describe('dev-proxy backend stdio transport', () => { + const pids: number[] = []; + const transports: BackendStdioTransport[] = []; + + afterEach(async () => { + for (const transport of transports.splice(0)) await transport.close().catch(() => undefined); + for (const pid of pids.splice(0)) { + try { process.kill(pid); } catch { /* already gone */ } + } + }); + + function negotiating(signal: AbortSignal) { + const transport = new BackendStdioTransport(SILENT, { signal }); + transports.push(transport); + const client = new Client({ name: 'backend-transport-test', version: '0.0.0' }, { versionNegotiation: { mode: 'auto' } }); + const connecting = client.connect(transport, { signal }).then(() => 'connected' as const, () => 'rejected' as const); + const pid = transport.pid!; + pids.push(pid); + return { transport, connecting, pid }; + } + + it('is a subclass of the SDK transport: that is what makes the negotiation probe in place', () => { + const transport = new BackendStdioTransport(SILENT); + + expect(transport).toBeInstanceOf(StdioClientTransport); + expect(Object.getPrototypeOf(transport)).not.toBe(StdioClientTransport.prototype); + }); + + it('ends a start that is still negotiating when the proxy shuts down: the connect settles and the backend is gone', async () => { + const shutdown = new AbortController(); + const { connecting, pid } = negotiating(shutdown.signal); + expect(isAlive(pid)).toBe(true); + + shutdown.abort(); + + // Without the transport's own close the SDK leaves this pending past the abort. + expect(await Promise.race([connecting, delay(4000).then(() => 'still pending' as const)])).toBe('rejected'); + await expect.poll(() => isAlive(pid), { timeout: 8000 }).toBe(false); + }, 20_000); + + it('lets go of the shutdown signal once it is closed, so restarts do not pile listeners onto it', async () => { + const shutdown = new AbortController(); + const { transport } = negotiating(shutdown.signal); + expect(getEventListeners(shutdown.signal, 'abort').length).toBeGreaterThan(0); + const before = getEventListeners(shutdown.signal, 'abort').length; + + await transport.close(); + + expect(getEventListeners(shutdown.signal, 'abort').length).toBe(before - 1); + }, 20_000); + + it('lets go of the signal when its owner says the backend has gone by itself', () => { + const shutdown = new AbortController(); + const transport = new BackendStdioTransport(SILENT, { signal: shutdown.signal }); + expect(getEventListeners(shutdown.signal, 'abort')).toHaveLength(1); + + transport.detachFromSignal(); + transport.detachFromSignal(); + + expect(getEventListeners(shutdown.signal, 'abort')).toHaveLength(0); + }); +}); diff --git a/tests/unit/dev-proxy/backend-subscriptions.test.ts b/tests/unit/dev-proxy/backend-subscriptions.test.ts new file mode 100644 index 00000000..dea23cf6 --- /dev/null +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -0,0 +1,418 @@ +/** + * The dev proxy's backend subscription relay (issue #1025, step 2, PR 5). + * + * The proxy keeps the set of output resources the front wants pings for and + * expresses it to whichever era the backend speaks: a 2025 backend takes one + * resources/subscribe per URI (and loses them all on restart, so a reopen + * re-sends them); a 2026-07-28 backend has no resources/subscribe at all — the + * set becomes ONE subscriptions/listen with the URIs plus the list-changed + * flags, re-opened whenever the set changes and after every backend restart. + * The notifications themselves flow through the client's + * fallbackNotificationHandler exactly as before; the relay never touches them. + */ +import { describe, expect, it, vi } from 'vitest'; +import { createSubscriptionRelay } from '../../../tools/dev-proxy/backend-subscriptions.mjs'; + +type Era = 'legacy' | 'modern'; + +function fakeClient(era: Era, listed: string[] = []) { + const closes: Array> = []; + const client = { + getProtocolEra: vi.fn(() => era), + subscribeResource: vi.fn(async (_params: { uri: string }) => ({})), + unsubscribeResource: vi.fn(async (_params: { uri: string }) => ({})), + listResources: vi.fn(async () => ({ resources: listed.map((uri) => ({ uri, name: uri })) })), + listen: vi.fn(async (filter: unknown) => { + const close = vi.fn(async () => {}); + closes.push(close); + return { honoredFilter: filter, close }; + }), + }; + return { client, closes }; +} + +describe('dev-proxy backend subscription relay', () => { + describe('against a 2025 backend', () => { + it('forwards subscribe and unsubscribe one URI at a time and never opens a listen', async () => { + const { client } = fakeClient('legacy'); + const relay = createSubscriptionRelay({ getClient: () => client }); + + await relay.subscribe('debug://sessions/a/output'); + await relay.subscribe('debug://sessions/b/output'); + await relay.unsubscribe('debug://sessions/a/output'); + + expect(client.subscribeResource.mock.calls.map(([p]) => p.uri)).toEqual([ + 'debug://sessions/a/output', + 'debug://sessions/b/output', + ]); + expect(client.unsubscribeResource).toHaveBeenCalledWith({ uri: 'debug://sessions/a/output' }); + expect(client.listen).not.toHaveBeenCalled(); + expect(relay.uris()).toEqual(['debug://sessions/b/output']); + }); + + it('re-sends every subscription on reopen: a restarted backend has forgotten them', async () => { + const { client } = fakeClient('legacy'); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.subscribe('debug://sessions/a/output'); + client.subscribeResource.mockClear(); + + await relay.reopen(); + + expect(client.subscribeResource).toHaveBeenCalledTimes(1); + expect(client.subscribeResource).toHaveBeenCalledWith({ uri: 'debug://sessions/a/output' }); + }); + }); + + // A backend restart is the proxy's whole purpose, and it kills every debug + // session: each subscription a client named before it now points at nothing. + // A 2025 backend refuses those one by one. The relay used to rethrow the first + // refusal — aborting the re-expression and keeping the dead URI, so every + // later reopen failed at the same place. + describe('re-expressing to a 2025 backend that refuses a URI', () => { + const A = 'debug://sessions/a/output'; + const B = 'debug://sessions/b/output'; + const PROXY_LOG = 'debug://sessions/b/proxy-log'; + const refusal = (uri: string) => Object.assign(new Error(`Unknown resource: ${uri}`), { code: -32602 }); + + it('drops a subscription the client named once the backend refuses it, says so once, and still sends the rest', async () => { + const { client } = fakeClient('legacy'); + const log = vi.fn(); + const relay = createSubscriptionRelay({ getClient: () => client, log }); + await relay.subscribe(A); + await relay.subscribe(B); + client.subscribeResource.mockClear(); + client.subscribeResource.mockImplementation(async ({ uri }: { uri: string }) => { + if (uri === A) throw refusal(uri); + return {}; + }); + + await expect(relay.reopen()).resolves.toBeUndefined(); + await relay.reopen(); + + expect(relay.uris()).toEqual([B]); + // A was asked once and never again; B was re-sent on both reopens. + expect(client.subscribeResource.mock.calls.map(([params]) => params.uri)).toEqual([A, B, B]); + expect(log.mock.calls.filter(([line]) => String(line).includes(A))).toHaveLength(1); + expect(String(log.mock.calls.find(([line]) => String(line).includes(A))![0])).toContain('Dropped'); + }); + + it('asks for a listed resource the backend will not subscribe once, not on every reopen', async () => { + const { client } = fakeClient('legacy', [B, PROXY_LOG]); + const log = vi.fn(); + const relay = createSubscriptionRelay({ getClient: () => client, log }); + relay.trackAll(true); + client.subscribeResource.mockImplementation(async ({ uri }: { uri: string }) => { + if (uri === PROXY_LOG) throw refusal(uri); + return {}; + }); + + await relay.reopen(); + await relay.refreshKnown(); + await relay.reopen(); + + const asked = client.subscribeResource.mock.calls.map(([params]) => params.uri); + expect(asked.filter((uri) => uri === PROXY_LOG)).toHaveLength(1); + expect(log.mock.calls.filter(([line]) => String(line).includes(PROXY_LOG))).toHaveLength(1); + }); + + it('asks again for a refused resource once the backend has stopped and started listing it', async () => { + const { client } = fakeClient('legacy', [PROXY_LOG]); + const relay = createSubscriptionRelay({ getClient: () => client }); + relay.trackAll(true); + client.subscribeResource.mockRejectedValueOnce(refusal(PROXY_LOG)); + await relay.reopen(); + + client.listResources.mockResolvedValueOnce({ resources: [] }); + await relay.refreshKnown(); + await relay.refreshKnown(); + + expect(client.subscribeResource.mock.calls.map(([params]) => params.uri)).toEqual([PROXY_LOG, PROXY_LOG]); + }); + + it('keeps a subscription the backend failed on for another reason, and still sends the rest', async () => { + const { client } = fakeClient('legacy'); + const log = vi.fn(); + const relay = createSubscriptionRelay({ getClient: () => client, log }); + await relay.subscribe(A); + await relay.subscribe(B); + client.subscribeResource.mockClear(); + client.subscribeResource.mockImplementation(async ({ uri }: { uri: string }) => { + if (uri === A) throw Object.assign(new Error('Internal error'), { code: -32603 }); + return {}; + }); + + await expect(relay.reopen()).resolves.toBeUndefined(); + + // An internal error says nothing about whether the resource exists: A stays, to be tried again. + expect(relay.uris()).toEqual([A, B]); + expect(client.subscribeResource.mock.calls.map(([params]) => params.uri)).toEqual([A, B]); + expect(log).toHaveBeenCalledWith(expect.stringContaining(A)); + expect(String(log.mock.calls.find(([line]) => String(line).includes(A))![0])).not.toContain('Dropped'); + }); + + it('asks again for a listed resource the backend failed on for another reason', async () => { + const { client } = fakeClient('legacy', [PROXY_LOG]); + const relay = createSubscriptionRelay({ getClient: () => client }); + relay.trackAll(true); + client.subscribeResource.mockRejectedValueOnce(Object.assign(new Error('Internal error'), { code: -32603 })); + + await relay.reopen(); + await relay.reopen(); + + expect(client.subscribeResource.mock.calls.map(([params]) => params.uri)).toEqual([PROXY_LOG, PROXY_LOG]); + }); + + it('does not mistake a broken connection for a refusal: that failure surfaces and nothing is dropped', async () => { + const { client } = fakeClient('legacy'); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.subscribe(A); + client.subscribeResource.mockRejectedValue(new Error('Not connected')); + + await expect(relay.reopen()).rejects.toThrow('Not connected'); + + expect(relay.uris()).toEqual([A]); + }); + + it('still gives the client its own refusal when it subscribes to a resource that is not there', async () => { + const { client } = fakeClient('legacy'); + const relay = createSubscriptionRelay({ getClient: () => client }); + client.subscribeResource.mockRejectedValue(refusal(A)); + + await expect(relay.subscribe(A)).rejects.toThrow('Unknown resource'); + + expect(relay.uris()).toEqual([]); + }); + }); + + describe('against a 2026-07-28 backend', () => { + it('opens one listen for the set plus the list-changed flags, and re-opens it when the set changes', async () => { + const { client, closes } = fakeClient('modern'); + const relay = createSubscriptionRelay({ getClient: () => client }); + + await relay.subscribe('debug://sessions/a/output'); + expect(client.subscribeResource).not.toHaveBeenCalled(); + expect(client.listen).toHaveBeenCalledTimes(1); + expect(client.listen).toHaveBeenLastCalledWith({ + resourceSubscriptions: ['debug://sessions/a/output'], + toolsListChanged: true, + resourcesListChanged: true, + }); + + await relay.subscribe('debug://sessions/b/output'); + // The previous listen is closed before the next one opens. + expect(closes[0]).toHaveBeenCalledTimes(1); + expect(client.listen).toHaveBeenCalledTimes(2); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output', 'debug://sessions/b/output'], + })); + + await relay.unsubscribe('debug://sessions/a/output'); + expect(closes[1]).toHaveBeenCalledTimes(1); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/b/output'], + })); + }); + + it('keeps a listen open for the list-changed notifications even with no URI', async () => { + const { client } = fakeClient('modern'); + const relay = createSubscriptionRelay({ getClient: () => client }); + + await relay.reopen(); + + expect(client.listen).toHaveBeenCalledWith({ + resourceSubscriptions: [], + toolsListChanged: true, + resourcesListChanged: true, + }); + }); + + it('a duplicate subscribe changes nothing and re-opens nothing', async () => { + const { client } = fakeClient('modern'); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.subscribe('debug://sessions/a/output'); + + await relay.subscribe('debug://sessions/a/output'); + + expect(client.listen).toHaveBeenCalledTimes(1); + }); + + it('close() ends the open listen and leaves the set in place for the next backend', async () => { + const { client, closes } = fakeClient('modern'); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.subscribe('debug://sessions/a/output'); + + await relay.close(); + expect(closes[0]).toHaveBeenCalledTimes(1); + expect(relay.uris()).toEqual(['debug://sessions/a/output']); + + // The next backend (after a restart) gets the whole set in one listen. + await relay.reopen(); + expect(client.listen).toHaveBeenCalledTimes(2); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output'], + })); + }); + }); + + it('with no backend client, remembers the set and expresses it on reopen', async () => { + let client: ReturnType['client'] | null = null; + const relay = createSubscriptionRelay({ getClient: () => client }); + + await relay.subscribe('debug://sessions/a/output'); + expect(relay.uris()).toEqual(['debug://sessions/a/output']); + + client = fakeClient('modern').client; + await relay.reopen(); + expect(client.listen).toHaveBeenCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output'], + })); + }); + + describe('serialization (review of PR #1118, Important #2)', () => { + describe.each(['legacy', 'modern'] as const)('opposing operations against a %s backend', (era) => { + const URI = 'debug://sessions/a/output'; + + it('applies an unsubscribe queued behind a subscribe before the subscribe settles', async () => { + const { client } = fakeClient(era); + const relay = createSubscriptionRelay({ getClient: () => client }); + + // Both requests arrive before the serialized subscribe has changed the set. + await Promise.all([relay.subscribe(URI), relay.unsubscribe(URI)]); + + expect(relay.uris()).toEqual([]); + if (era === 'legacy') { + expect(client.subscribeResource).toHaveBeenCalledWith({ uri: URI }); + expect(client.unsubscribeResource).toHaveBeenCalledWith({ uri: URI }); + } else { + expect(client.listen).toHaveBeenCalledTimes(2); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ resourceSubscriptions: [] })); + } + }); + + it('applies a subscribe queued behind an unsubscribe before the unsubscribe settles', async () => { + const { client } = fakeClient(era); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.subscribe(URI); + client.subscribeResource.mockClear(); + client.listen.mockClear(); + + // The second request must see the removal queued before it, not the old set. + await Promise.all([relay.unsubscribe(URI), relay.subscribe(URI)]); + + expect(relay.uris()).toEqual([URI]); + if (era === 'legacy') { + expect(client.unsubscribeResource).toHaveBeenCalledWith({ uri: URI }); + expect(client.subscribeResource).toHaveBeenCalledWith({ uri: URI }); + } else { + expect(client.listen).toHaveBeenCalledTimes(2); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ resourceSubscriptions: [URI] })); + } + }); + }); + + it('two subscribes arriving before either listen settles leave exactly one listen open', async () => { + const { client, closes } = fakeClient('modern'); + const releases: Array<() => void> = []; + client.listen.mockImplementation((filter: unknown) => new Promise((resolve) => { + releases.push(() => { + const close = vi.fn(async () => {}); + closes.push(close); + resolve({ honoredFilter: filter, close }); + }); + })); + const relay = createSubscriptionRelay({ getClient: () => client }); + + const first = relay.subscribe('debug://sessions/a/output'); + const second = relay.subscribe('debug://sessions/b/output'); + await vi.waitFor(() => expect(client.listen).toHaveBeenCalledTimes(1)); + // The second subscribe waits for the first to settle rather than racing it. + expect(client.listen).toHaveBeenCalledTimes(1); + + releases[0](); + await first; + await vi.waitFor(() => expect(client.listen).toHaveBeenCalledTimes(2)); + releases[1](); + await second; + + // The first listen was closed before the second opened; the second stays open. + expect(closes).toHaveLength(2); + expect(closes[0]).toHaveBeenCalledTimes(1); + expect(closes[1]).not.toHaveBeenCalled(); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output', 'debug://sessions/b/output'], + })); + }); + }); + + describe('tracking everything the backend lists (review of PR #1118, Important #1)', () => { + // A 2026-07-28 front client's listen set is served by the SDK entry and never + // reaches the proxy, so while such a client is pinned the proxy subscribes the + // backend to every resource it lists; the front's router filters per client. + it('expresses the listed resources too while track-all is on, refreshes them on request, and drops them when it is off', async () => { + const { client } = fakeClient('modern', ['debug://sessions/a/output', 'debug://sessions/a/proxy-log']); + const relay = createSubscriptionRelay({ getClient: () => client }); + + relay.trackAll(true); + await relay.reopen(); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output', 'debug://sessions/a/proxy-log'], + })); + + // A new session appeared (resources/list_changed): re-list and re-express. + client.listResources.mockResolvedValue({ resources: [{ uri: 'debug://sessions/a/output', name: 'a' }, { uri: 'debug://sessions/b/output', name: 'b' }] }); + await relay.refreshKnown(); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ + resourceSubscriptions: ['debug://sessions/a/output', 'debug://sessions/b/output'], + })); + + // The modern pin closed: only what a client asked for by name remains. + relay.trackAll(false); + await relay.reopen(); + expect(client.listen).toHaveBeenLastCalledWith(expect.objectContaining({ resourceSubscriptions: [] })); + }); + + it('against a 2025 backend subscribes each listed resource, and a refusal is logged rather than thrown', async () => { + const { client } = fakeClient('legacy', ['debug://sessions/a/output', 'debug://sessions/a/proxy-log']); + client.subscribeResource.mockImplementation(async ({ uri }: { uri: string }) => { + if (uri.endsWith('proxy-log')) throw Object.assign(new Error('Unknown resource: ' + uri), { code: -32602 }); + return {}; + }); + const log = vi.fn(); + const relay = createSubscriptionRelay({ getClient: () => client, log }); + + relay.trackAll(true); + await expect(relay.reopen()).resolves.toBeUndefined(); + + expect(client.subscribeResource.mock.calls.map(([p]) => p.uri)).toEqual(['debug://sessions/a/output', 'debug://sessions/a/proxy-log']); + expect(log).toHaveBeenCalledWith(expect.stringContaining('debug://sessions/a/proxy-log')); + }); + + it('refreshKnown is a no-op while track-all is off', async () => { + const { client } = fakeClient('modern', ['debug://sessions/a/output']); + const relay = createSubscriptionRelay({ getClient: () => client }); + await relay.refreshKnown(); + expect(client.listResources).not.toHaveBeenCalled(); + expect(client.listen).not.toHaveBeenCalled(); + }); + }); + + it('does not remember a URI whose listen the 2026 backend refused', async () => { + const { client } = fakeClient('modern'); + client.listen.mockRejectedValueOnce(new Error('listen refused')); + const relay = createSubscriptionRelay({ getClient: () => client }); + + await expect(relay.subscribe('debug://sessions/x/output')).rejects.toThrow('listen refused'); + expect(relay.uris()).toEqual([]); + }); + + it('reports, not throws, when the backend refuses a subscription', async () => { + const { client } = fakeClient('legacy'); + client.subscribeResource.mockRejectedValueOnce(new Error('Unknown resource: debug://sessions/x/output')); + const log = vi.fn(); + const relay = createSubscriptionRelay({ getClient: () => client, log }); + + await expect(relay.subscribe('debug://sessions/x/output')).rejects.toThrow('Unknown resource'); + // A refused URI is not remembered: the front was told no. + expect(relay.uris()).toEqual([]); + }); +}); diff --git a/tests/unit/dev-proxy/dev-proxy-shutdown.test.ts b/tests/unit/dev-proxy/dev-proxy-shutdown.test.ts index 64ab0db3..92b5c864 100644 --- a/tests/unit/dev-proxy/dev-proxy-shutdown.test.ts +++ b/tests/unit/dev-proxy/dev-proxy-shutdown.test.ts @@ -132,20 +132,6 @@ describe('dev-proxy installShutdownHandlers', () => { expect(log).toHaveBeenCalledWith(expect.stringContaining('kill failed')); }); - it('chains server.onclose instead of replacing it, and shuts down on server close', async () => { - const { stdin, proc, backend, log } = makeDeps(); - const previousOnClose = vi.fn(); - const server: { onclose?: () => void } = { onclose: previousOnClose }; - installShutdownHandlers({ stdin, backend, server, log, proc }); - - expect(server.onclose).not.toBe(previousOnClose); - server.onclose!(); - - await vi.waitFor(() => expect(proc.exit).toHaveBeenCalledWith(0)); - expect(previousOnClose).toHaveBeenCalledTimes(1); - expect(backend.stop).toHaveBeenCalledTimes(1); - }); - it('returns an idempotent shutdown function usable directly', async () => { const { stdin, proc, backend, log } = makeDeps(); const shutdown = installShutdownHandlers({ stdin, backend, log, proc }); diff --git a/tests/unit/dev-proxy/front-server.test.ts b/tests/unit/dev-proxy/front-server.test.ts new file mode 100644 index 00000000..7bd7d1d1 --- /dev/null +++ b/tests/unit/dev-proxy/front-server.test.ts @@ -0,0 +1,219 @@ +/** + * The dev proxy's client-facing server behind the SDK's stdio entry (issue + * #1025, step 2, PR 5), in process over the in-memory transport. `serveStdio` + * pins one front instance per connection in whichever era the client opens; + * the front's handlers forward to the backend, answer the dev tools locally, + * and the relay target (`current()`) follows the pinned instance so backend + * notifications reach the client in the era it speaks. + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { InMemoryTransport } from '@modelcontextprotocol/server'; +import { serveStdio, type StdioServerHandle } from '@modelcontextprotocol/server/stdio'; +import { Client } from '@modelcontextprotocol/client'; +import { createFront, ObservedStdioServerTransport } from '../../../tools/dev-proxy/front-server.mjs'; + +const URI = 'debug://sessions/sess-1/output'; +const DEV_TOOLS = [{ name: 'dev_server_status', description: 'status', inputSchema: { type: 'object', properties: {} } }]; + +function fakeBackend(overrides: Record = {}) { + const mcpClient = { + listTools: vi.fn(async () => ({ tools: [{ name: 'backend_tool', description: 'from the backend', inputSchema: { type: 'object', properties: {} } }] })), + listResources: vi.fn(async () => ({ resources: [{ uri: URI, name: 'out' }] })), + readResource: vi.fn(async (params: { uri: string }) => ({ contents: [{ uri: params.uri, text: 'hello' }] })), + }; + return { + state: 'running', + mcpClient, + ensureStarted: vi.fn(async () => {}), + whenReady: vi.fn(async () => true), + callTool: vi.fn(async (name: string) => ({ content: [{ type: 'text', text: `backend handled ${name}` }] })), + ...overrides, + }; +} + +function fakeRelay() { + return { + subscribe: vi.fn(async () => {}), + unsubscribe: vi.fn(async () => {}), + trackAll: vi.fn(), + refreshKnown: vi.fn(async () => {}), + }; +} + +describe('dev-proxy front behind serveStdio (issue #1025, step 2)', () => { + let handle: StdioServerHandle | undefined; + let client: Client | undefined; + + afterEach(async () => { + await client?.close().catch(() => undefined); + await handle?.close().catch(() => undefined); + handle = undefined; + client = undefined; + }); + + function serve(deps: Partial[0]> = {}) { + const backend = fakeBackend(); + const relay = fakeRelay(); + const handleDevTool = vi.fn(async (name: string) => ({ content: [{ type: 'text', text: `dev ${name}` }] })); + const front = createFront({ backend, relay, devTools: DEV_TOOLS, handleDevTool, log: () => {}, ...deps } as any); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + handle = serveStdio(front.factory, { transport: serverTransport }); + return { front, backend, relay, handleDevTool, clientTransport }; + } + + it('a 2026-07-28 client lands on a modern pin, and tools/list merges the backend tools with the dev tools after a deferred start', async () => { + const { backend, clientTransport } = serve(); + client = new Client({ name: 'modern', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + await client.connect(clientTransport); + + expect(client.getProtocolEra()).toBe('modern'); + // Connecting alone never starts the backend: a disposable probe sibling + // connects exactly like this and must not spawn one. + expect(backend.ensureStarted).not.toHaveBeenCalled(); + + const listed = await client.listTools(); + expect(listed.tools.map((t) => t.name)).toEqual(['backend_tool', 'dev_server_status']); + expect(backend.ensureStarted).toHaveBeenCalledTimes(1); + expect(backend.ensureStarted.mock.invocationCallOrder[0]).toBeLessThan(backend.whenReady.mock.invocationCallOrder[0]); + }); + + it('a 2025 client lands on a legacy pin; its resources/subscribe and unsubscribe reach the relay', async () => { + const { relay, clientTransport } = serve(); + client = new Client({ name: 'legacy', version: '0.0.0' }, { versionNegotiation: { mode: 'legacy' } }); + await client.connect(clientTransport); + + expect(client.getProtocolEra()).toBe('legacy'); + await client.subscribeResource({ uri: URI }); + expect(relay.subscribe).toHaveBeenCalledWith(URI); + await client.unsubscribeResource({ uri: URI }); + expect(relay.unsubscribe).toHaveBeenCalledWith(URI); + }); + + it('answers the dev tools locally and forwards every other call to the backend, starting it first', async () => { + const { backend, handleDevTool, clientTransport } = serve(); + client = new Client({ name: 'legacy', version: '0.0.0' }, { versionNegotiation: { mode: 'legacy' } }); + await client.connect(clientTransport); + + const status = await client.callTool({ name: 'dev_server_status', arguments: {} }); + expect(status.content).toEqual([{ type: 'text', text: 'dev dev_server_status' }]); + expect(handleDevTool).toHaveBeenCalledWith('dev_server_status', {}); + expect(backend.ensureStarted).not.toHaveBeenCalled(); + + const forwarded = await client.callTool({ name: 'backend_tool', arguments: { a: 1 } }); + expect(forwarded.content).toEqual([{ type: 'text', text: 'backend handled backend_tool' }]); + expect(backend.ensureStarted).toHaveBeenCalledTimes(1); + expect(backend.callTool).toHaveBeenCalledWith('backend_tool', { a: 1 }, expect.anything()); + }); + + it('the relay target follows the pinned instance: a legacy client hears a relayed resources/updated', async () => { + const { front, clientTransport } = serve(); + client = new Client({ name: 'legacy', version: '0.0.0' }, { versionNegotiation: { mode: 'legacy' } }); + const updated: string[] = []; + client.setNotificationHandler('notifications/resources/updated', async (n) => { updated.push(n.params.uri); }); + await client.connect(clientTransport); + expect(front.current()).not.toBeNull(); + + await front.notify({ method: 'notifications/resources/updated', params: { uri: URI } }); + await vi.waitFor(() => expect(updated).toEqual([URI])); + }); + + it('a modern client hears a relayed resources/updated on its listen stream, and list changes through the typed senders', async () => { + const { front, clientTransport } = serve(); + client = new Client({ name: 'modern', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + const updated: string[] = []; + const listChanges: string[] = []; + client.setNotificationHandler('notifications/resources/updated', async (n) => { updated.push(n.params.uri); }); + client.setNotificationHandler('notifications/tools/list_changed', async (n) => { listChanges.push(n.method); }); + await client.connect(clientTransport); + const subscription = await client.listen({ resourceSubscriptions: [URI], toolsListChanged: true }); + + await front.notify({ method: 'notifications/resources/updated', params: { uri: URI } }); + await front.sendToolListChanged(); + await vi.waitFor(() => expect(updated).toEqual([URI])); + await vi.waitFor(() => expect(listChanges).toEqual(['notifications/tools/list_changed'])); + await subscription.close(); + }); + + it('a notification with no pinned instance is dropped, not thrown', async () => { + const { front } = serve(); + expect(front.current()).toBeNull(); + await expect(front.notify({ method: 'notifications/resources/list_changed' })).resolves.toBeUndefined(); + await expect(front.sendToolListChanged()).resolves.toBeUndefined(); + }); + + // Review of PR #1118, Important #1: a 2026 client's listen set is served by the + // SDK entry and never reaches a handler, so the proxy cannot learn which URIs + // it wants. While a modern instance is pinned, the relay tracks everything the + // backend lists; the front's own router delivers each client only what it + // listened for. + it('a modern pin turns track-all on, and the pin closing turns it off', async () => { + const { relay, clientTransport } = serve(); + client = new Client({ name: 'modern', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + await client.connect(clientTransport); + expect(relay.trackAll).toHaveBeenLastCalledWith(true); + + await client.close(); + client = undefined; + await vi.waitFor(() => expect(relay.trackAll).toHaveBeenLastCalledWith(false)); + }); + + it('a legacy pin leaves track-all off: its client names what it wants through resources/subscribe', async () => { + const { relay, clientTransport } = serve(); + client = new Client({ name: 'legacy', version: '0.0.0' }, { versionNegotiation: { mode: 'legacy' } }); + await client.connect(clientTransport); + expect(relay.trackAll).not.toHaveBeenCalledWith(true); + }); + + it('a relayed resources/list_changed refreshes the tracked set and still reaches the client', async () => { + const { front, relay, clientTransport } = serve(); + client = new Client({ name: 'modern', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + const changes: string[] = []; + client.setNotificationHandler('notifications/resources/list_changed', async (n) => { changes.push(n.method); }); + await client.connect(clientTransport); + const subscription = await client.listen({ resourcesListChanged: true }); + // The pin itself already refreshed once (a backend may be connected already). + const refreshes = relay.refreshKnown.mock.calls.length; + + await front.notify({ method: 'notifications/resources/list_changed' }); + expect(relay.refreshKnown).toHaveBeenCalledTimes(refreshes + 1); + await vi.waitFor(() => expect(changes).toEqual(['notifications/resources/list_changed'])); + await subscription.close(); + }); + + it('a modern client hears nothing for a URI it did not listen for: the router filters per client', async () => { + const { front, clientTransport } = serve(); + client = new Client({ name: 'modern', version: '0.0.0' }, { versionNegotiation: { mode: { pin: '2026-07-28' } } }); + const updated: string[] = []; + client.setNotificationHandler('notifications/resources/updated', async (n) => { updated.push(n.params.uri); }); + await client.connect(clientTransport); + const subscription = await client.listen({ resourceSubscriptions: [URI] }); + + await front.notify({ method: 'notifications/resources/updated', params: { uri: 'debug://sessions/other/output' } }); + await front.notify({ method: 'notifications/resources/updated', params: { uri: URI } }); + await vi.waitFor(() => expect(updated).toEqual([URI])); + await subscription.close(); + }); + + it('clears the relay target when the pinned instance closes', async () => { + const { front, clientTransport } = serve(); + client = new Client({ name: 'legacy', version: '0.0.0' }, { versionNegotiation: { mode: 'legacy' } }); + await client.connect(clientTransport); + expect(front.current()).not.toBeNull(); + + await client.close(); + client = undefined; + await vi.waitFor(() => expect(front.current()).toBeNull()); + }); +}); + +describe('ObservedStdioServerTransport', () => { + it('reports the wire closing exactly once, after the SDK close has run', async () => { + const seen = vi.fn(); + const transport = new ObservedStdioServerTransport(seen); + + await transport.close(); + await transport.close(); + + expect(seen).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/unit/dev-proxy/tool-error.test.ts b/tests/unit/dev-proxy/tool-error.test.ts index c60a68f0..25bd7163 100644 --- a/tests/unit/dev-proxy/tool-error.test.ts +++ b/tests/unit/dev-proxy/tool-error.test.ts @@ -13,6 +13,7 @@ */ import { describe, it, expect } from 'vitest'; import { + initialStartFailureLines, isBackendUnavailableError, dedupeMcpErrorPrefix, assertBackendAvailable, @@ -186,3 +187,20 @@ describe('dev-proxy backendFailureHint (issue #854)', () => { expect(hint).toMatch(/did not answer this call/); }); }); + +describe('what the proxy logs when its first backend start does not happen (issue #1025)', () => { + it('names the failure and the way to retry it', () => { + expect(initialStartFailureLines(new Error('port 3001 is in use'), false)).toEqual([ + 'Initial backend start failed: port 3001 is in use', + 'Dev tools are still available — use dev_restart_debugger to retry', + ]); + }); + + // A request that lands while the proxy is going away asks for a start that is + // refused on purpose. There is nothing to retry and nobody to retry it. + it('says the proxy is shutting down, with no retry hint, when that is why', () => { + expect(initialStartFailureLines(new Error('Proxy is shutting down'), true)).toEqual([ + 'Not starting the backend: the proxy is shutting down', + ]); + }); +}); diff --git a/tools/dev-proxy/README.md b/tools/dev-proxy/README.md index aa4f230c..277b4db3 100644 --- a/tools/dev-proxy/README.md +++ b/tools/dev-proxy/README.md @@ -12,6 +12,37 @@ session. The proxy keeps the client-facing stdio process stable while it replace The backend is a restartable Streamable HTTP child process by default. A stdio backend is also supported through `DEV_PROXY_BACKEND_TRANSPORT`. +The proxy speaks both MCP protocol eras to its client (issue #1025): it is served through the +SDK's stdio entry, so a client that probes with `server/discover` — Claude Code's v2 runtime does — +lands on the current 2026-07-28 revision (sessionless; resource notifications over +`subscriptions/listen`), and a client that opens with `initialize` gets the 2025 era as before. +Towards its backend the proxy negotiates the same way, on either transport, and lands on +2026-07-28 when that backend serves it (an older build answers the 2025 era; `dev_server_status` +says which under `protocolEra`/`protocolVersion`). Over stdio it probes on the backend's own +pipe — its transport is a subclass of the SDK's, which is what keeps the SDK from probing on a +second backend process — and a 2025 backend falls back to `initialize` on the same pipe +(measured against the published 0.26.0 npx bundle and Docker image). Resource +subscriptions are translated across eras: the set is expressed to the backend in the era it +speaks (one `subscriptions/listen` for a 2026 backend, one `resources/subscribe` per URI for a +2025 one) and re-expressed after every backend restart, so a `dev_rebuild_and_restart` no longer +loses them. A 2025 client names what it wants through `resources/subscribe`. A 2026 client's +listen set is served by the SDK entry and never reaches the proxy, so while such a client is +connected the proxy subscribes the backend to every resource it lists (re-read on each +`resources/list_changed`) and the proxy's own router delivers the client only what it listened +for. A 2025 backend can refuse a URI: after a restart every debug session is gone, so a +subscription a client named before it is dropped (logged once) and the rest are still sent, and +a listed resource the backend will not subscribe (its proxy-log tails) is asked once, not on +every reconnect. That router also drops a `tools/list_changed` a 2026 client did not opt into +(`toolsListChanged: true` on its listen): after a restart that changes the inventory, such a +client keeps its tool list until it lists again. + +The backend starts on the first request that needs it, not when the proxy process starts. A v2 +client's `auto` negotiation over stdio probes `server/discover` on a disposable sibling process +spawned from the same command line; the SDK entry answers that probe itself, so the sibling never +reaches a request handler and never spawns a backend of its own to race the real proxy for +`DEV_PROXY_PORT` (measured before this change: two backends per connect). The first +`tools/list` still waits for the start to settle (`DEV_PROXY_DISCOVERY_WAIT_MS`). + ## Architecture ``` @@ -83,7 +114,7 @@ Once connected, three additional tools are available: |------|-------------| | `dev_restart_debugger` | Kill and restart the backend. Pass `rebuild: true` to build first, and optionally replace backend environment overrides with `env`. | | `dev_rebuild_and_restart` | Run `npm run build` then restart the backend; also accepts replacement backend `env` overrides. | -| `dev_server_status` | Check backend state, PID, uptime, build progress, transport, project root, port, and display-safe environment overrides. | +| `dev_server_status` | Check backend state, PID, uptime, build progress, transport, protocol era and version, project root, port, and display-safe environment overrides. Before the first start it reports `stopped` with a `note` that the backend starts on the first request that needs it. | All regular mcp-debugger tools (create_debug_session, set_breakpoint, etc.) are forwarded transparently to the backend. A forwarded call carries the caller's cancellation signal and is given 660 s before the proxy gives up on it — above the diff --git a/tools/dev-proxy/backend-env.d.mts b/tools/dev-proxy/backend-env.d.mts index 3fca2f2e..069b7399 100644 --- a/tools/dev-proxy/backend-env.d.mts +++ b/tools/dev-proxy/backend-env.d.mts @@ -39,3 +39,4 @@ export function buildBackendEnvironment( overrides: Record, forced?: Record ): Record; + diff --git a/tools/dev-proxy/backend-env.mjs b/tools/dev-proxy/backend-env.mjs index 4a087227..90bc82d9 100644 --- a/tools/dev-proxy/backend-env.mjs +++ b/tools/dev-proxy/backend-env.mjs @@ -80,3 +80,4 @@ export function updateBackendEnvOverrides(current, args) { export function buildBackendEnvironment(inherited, overrides, forced = {}) { return { ...inherited, ...overrides, ...forced }; } + diff --git a/tools/dev-proxy/backend-stdio-transport.d.mts b/tools/dev-proxy/backend-stdio-transport.d.mts new file mode 100644 index 00000000..aefa31b1 --- /dev/null +++ b/tools/dev-proxy/backend-stdio-transport.d.mts @@ -0,0 +1,13 @@ +import { StdioClientTransport, type StdioServerParameters } from '@modelcontextprotocol/client/stdio'; + +/** + * The dev proxy's stdio transport to its backend: a subclass of the SDK's, so + * `auto` negotiation probes on the backend's own pipe, that closes itself when + * the proxy's shutdown signal aborts (under `auto` negotiation the SDK does not + * watch connect()'s signal while its probe is unanswered). + */ +export class BackendStdioTransport extends StdioClientTransport { + constructor(params: StdioServerParameters, options?: { signal?: AbortSignal }); + /** Stop listening to the shutdown signal; `close()` does this too. Idempotent. */ + detachFromSignal(): void; +} diff --git a/tools/dev-proxy/backend-stdio-transport.mjs b/tools/dev-proxy/backend-stdio-transport.mjs new file mode 100644 index 00000000..d1c7aca3 --- /dev/null +++ b/tools/dev-proxy/backend-stdio-transport.mjs @@ -0,0 +1,67 @@ +/** + * The dev proxy's stdio transport to its backend (issue #1025, step 2). + * + * Being a subclass is the first point. Under `auto` negotiation the SDK probes + * server/discover on a disposable SIBLING process spawned from the same + * parameters, but only for its exact base StdioClientTransport — "subclasses + * probe in place", on the connection itself. A sibling here would be a second + * backend; in place, the backend starts once, a 2026-07-28 server pins the + * modern era, and a 2025 server answers the probe with "method not found" and + * gets `initialize` on the same pipe. + * + * The shutdown signal is the second. Under `auto` negotiation the SDK does not + * watch the signal given to connect() while its probe is unanswered — measured + * on client 2.4.0 with all four combinations: in `legacy` mode an abort rejects + * the connect and the SDK closes the transport, base class or subclass; in + * `auto` mode the connect stays pending and the child keeps running, on the + * sibling path and in place alike. So a start still negotiating when the proxy + * shuts down would hang until the probe gives up, and the backend it spawned + * would outlive the proxy (Linux CI caught exactly that). This class does what + * the SDK does in `legacy` mode: it closes itself on that abort, which ends + * the child and fails the pending negotiation at once, so the start's own + * failure path can clean up. + * + * Kept in its own module so unit tests can import it without executing the + * dev-proxy entry point (dev-proxy.mjs runs main() at module top level). + */ +import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; + +export class BackendStdioTransport extends StdioClientTransport { + /** Removes the abort listener; a no-op once it has run. */ + #detach = () => {}; + + /** + * @param {ConstructorParameters[0]} params + * @param {{ signal?: AbortSignal }} [options] `signal`: the proxy's shutdown signal. + */ + constructor(params, { signal } = {}) { + super(params); + if (signal) { + const onAbort = () => { + void this.close().catch(() => {}); + }; + signal.addEventListener('abort', onAbort, { once: true }); + this.#detach = () => { + signal.removeEventListener('abort', onAbort); + this.#detach = () => {}; + }; + } + } + + /** + * Stop listening to the shutdown signal. The proxy starts a transport per + * backend start and the signal lives as long as the proxy, so a transport + * that is done must let go of it: `close()` does, and the owner calls this + * when the backend has gone by itself (the SDK then reports `onclose` + * without `close()` ever running). + */ + detachFromSignal() { + this.#detach(); + } + + /** @override */ + async close() { + this.#detach(); + return super.close(); + } +} diff --git a/tools/dev-proxy/backend-subscriptions.d.mts b/tools/dev-proxy/backend-subscriptions.d.mts new file mode 100644 index 00000000..6520ff29 --- /dev/null +++ b/tools/dev-proxy/backend-subscriptions.d.mts @@ -0,0 +1,31 @@ +export interface RelayClient { + getProtocolEra?(): 'legacy' | 'modern'; + subscribeResource(params: { uri: string }): Promise; + unsubscribeResource(params: { uri: string }): Promise; + listResources(): Promise<{ resources?: Array<{ uri: string }> }>; + listen(filter: { + resourceSubscriptions: string[]; + toolsListChanged: boolean; + resourcesListChanged: boolean; + }): Promise<{ close(): Promise }>; +} + +export interface SubscriptionRelay { + /** What front clients asked for by name. */ + uris(): string[]; + /** Track everything the backend lists (a 2026 front client is pinned), or stop. */ + trackAll(enabled: boolean): void; + subscribe(uri: string): Promise; + unsubscribe(uri: string): Promise; + /** After a backend (re)connect: re-read the list if tracking, express the whole set. */ + reopen(): Promise; + /** The backend's resource list changed: re-read it and express what is new. */ + refreshKnown(): Promise; + /** Before a backend goes away: end the open listen; the set stays. */ + close(): Promise; +} + +export function createSubscriptionRelay(deps: { + getClient: () => RelayClient | null | undefined; + log?: (msg: string) => void; +}): SubscriptionRelay; diff --git a/tools/dev-proxy/backend-subscriptions.mjs b/tools/dev-proxy/backend-subscriptions.mjs new file mode 100644 index 00000000..a8d9f82a --- /dev/null +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -0,0 +1,250 @@ +/** + * The backend subscription relay (issue #1025, step 2, PR 5). + * + * The proxy keeps the set of output resources its client wants pings for and + * expresses it to whichever protocol era the backend speaks: + * + * - a 2025 backend takes one `resources/subscribe` per URI, and forgets them + * all when it restarts, so `reopen()` re-sends the set; + * - a 2026-07-28 backend has no `resources/subscribe` at all — the set becomes + * ONE `subscriptions/listen` carrying the URIs plus the list-changed flags, + * re-opened whenever the set changes and after every backend restart (the + * list-changed flags keep a listen open even with no URI, so a new debug + * session's `resources/list_changed` still reaches the client). + * + * Two sources feed the set. A 2025 front client names what it wants through + * `resources/subscribe` (`subscribe`/`unsubscribe`). A 2026-07-28 front + * client's listen set is served by the SDK's stdio entry and never reaches the + * proxy, so while such a client is pinned (`trackAll(true)`) the relay also + * tracks every resource the backend lists — re-read on each + * `resources/list_changed` (`refreshKnown`) — and the front's own router + * delivers each client only what it listened for. + * + * Every expression against the backend is serialized: two subscribes arriving + * back-to-back must not both open a listen and leak the first one. + * + * A 2025 backend can refuse a URI, and re-expression has to survive that. A + * restart ends every debug session, so each subscription a client named before + * it points at nothing afterwards: a refused named URI is dropped, said once, + * and the rest are still sent. A listed resource the backend will not + * subscribe (its proxy-log tails) is asked once and remembered until the + * backend stops listing it. Only the answer "that resource is not here" + * (InvalidParams, or the older resource-not-found code) counts as a refusal: + * any other error answer leaves the subscription in place to be tried again, + * and a failed connection still surfaces. (A 2026 backend's listen takes any + * URI, so a dead one named there simply never fires.) + * + * The notifications themselves arrive through the client's + * `fallbackNotificationHandler` exactly as before (the v2 client passes + * listen-stream change notifications through to its registered handlers); the + * relay never touches them. + */ + +/** + * @typedef {object} RelayClient + * @property {() => 'legacy' | 'modern'} [getProtocolEra] + * @property {(params: { uri: string }) => Promise} subscribeResource + * @property {(params: { uri: string }) => Promise} unsubscribeResource + * @property {() => Promise<{ resources?: Array<{ uri: string }> }>} listResources + * @property {(filter: { resourceSubscriptions: string[], toolsListChanged: boolean, resourcesListChanged: boolean }) => Promise<{ close(): Promise }>} listen + */ + +/** + * @param {{ getClient: () => RelayClient | null | undefined, log?: (msg: string) => void }} deps + */ +export function createSubscriptionRelay({ getClient, log = () => {} }) { + /** What front clients asked for by name. @type {Set} */ + const uris = new Set(); + /** What the backend lists, while a 2026 front client is pinned. @type {string[]} */ + let known = []; + let trackingAll = false; + /** Listed resources the backend refused to subscribe; asked once each. @type {Set} */ + const refused = new Set(); + /** The modern-era listen currently open, if any. @type {{ close(): Promise } | null} */ + let open = null; + /** Serializes every expression against the backend. @type {Promise} */ + let tail = Promise.resolve(); + + /** @template T @param {() => Promise} work @returns {Promise} */ + const serialize = (work) => { + const run = tail.then(work, work); + tail = run.then(() => undefined, () => undefined); + return run; + }; + + const isModern = (client) => { + try { + return client?.getProtocolEra?.() === 'modern'; + } catch { + return false; + } + }; + + const wanted = () => [...new Set([...uris, ...(trackingAll ? known : [])])]; + + const closeOpen = async () => { + const current = open; + open = null; + if (!current) return; + try { + await current.close(); + } catch (err) { + log(`Ignoring listen close error: ${err?.message ?? err}`); + } + }; + + /** Re-read what the backend lists (only while tracking everything). */ + const relist = async (client) => { + if (!trackingAll) { + known = []; + return; + } + const listed = await client.listResources(); + known = (listed?.resources ?? []) + .map((resource) => resource?.uri) + .filter((uri) => typeof uri === 'string'); + // A refusal is remembered only while the backend still lists the resource. + for (const uri of refused) { + if (!known.includes(uri)) refused.delete(uri); + } + }; + + /** The backend answered (a JSON-RPC error, numeric code), as opposed to the connection failing (the SDK's string codes). */ + const isAnswer = (err) => typeof err?.code === 'number'; + /** "That resource is not here": InvalidParams (-32602), or the resource-not-found code older servers used (-32002). */ + const saysGone = (err) => err?.code === -32602 || err?.code === -32002; + + /** + * Re-express one URI to a 2025 backend. A refusal never stops the others: a + * URI the client named is dropped (the resource is gone), a listed one is + * remembered so it is not asked again. Any other error answer changes + * nothing, and a failed connection is thrown. + */ + const subscribeOne = async (client, uri) => { + const named = uris.has(uri); + if (!named && refused.has(uri)) return; + try { + await client.subscribeResource({ uri }); + } catch (err) { + if (!isAnswer(err)) throw err; + if (!saysGone(err)) { + // The backend is alive and failed for another reason: no evidence the + // resource is gone. Keep it, say so, and try again on the next pass. + log(`Backend could not subscribe ${uri} (${err?.message ?? err}); will retry`); + return; + } + if (named) { + uris.delete(uri); + log(`Dropped the subscription to ${uri}: the backend no longer has it (${err?.message ?? err})`); + } else { + refused.add(uri); + log(`Backend refused a tracked subscription for ${uri}: ${err?.message ?? err}`); + } + } + }; + + /** Express the whole wanted set to the backend in its era. */ + const express = async (client) => { + if (isModern(client)) { + await closeOpen(); + open = await client.listen({ + resourceSubscriptions: wanted(), + toolsListChanged: true, + resourcesListChanged: true, + }); + return; + } + for (const uri of wanted()) await subscribeOne(client, uri); + }; + + return { + uris: () => [...uris], + + /** + * Track everything the backend lists (a 2026 front client is pinned) or stop. + * The next `reopen()`/`refreshKnown()` applies it; `refreshKnown()` right away + * expresses it to a backend that is already connected. + * @param {boolean} enabled + */ + trackAll(enabled) { + trackingAll = Boolean(enabled); + if (!trackingAll) { + known = []; + refused.clear(); + } + }, + + /** @param {string} uri */ + async subscribe(uri) { + return serialize(async () => { + if (uris.has(uri)) return; + const client = getClient(); + if (!client) { + uris.add(uri); + return; + } + if (!isModern(client)) { + // A refusal (an unknown resource) is the front client's answer: not remembered. + await client.subscribeResource({ uri }); + uris.add(uri); + return; + } + uris.add(uri); + try { + await express(client); + } catch (err) { + uris.delete(uri); + throw err; + } + }); + }, + + /** @param {string} uri */ + async unsubscribe(uri) { + return serialize(async () => { + if (!uris.delete(uri)) return; + const client = getClient(); + if (!client) return; + if (!isModern(client)) { + await client.unsubscribeResource({ uri }); + return; + } + await express(client); + }); + }, + + /** After a backend (re)connect: the new backend knows nothing of the set yet. */ + reopen() { + return serialize(async () => { + const client = getClient(); + if (!client) return; + await relist(client); + await express(client); + }); + }, + + /** The backend's resource list changed: re-read it and express what is new. */ + refreshKnown() { + return serialize(async () => { + if (!trackingAll) return; + const client = getClient(); + if (!client) return; + const before = new Set(known); + await relist(client); + if (isModern(client)) { + await express(client); + return; + } + for (const uri of known) { + if (before.has(uri) || uris.has(uri)) continue; + await subscribeOne(client, uri); + } + }); + }, + + /** Before a backend goes away: end the open listen; the set stays for the next one. */ + close() { + return serialize(closeOpen); + }, + }; +} diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 2bca4835..a6ea7ea7 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -21,10 +21,8 @@ * or restart to settle (default: 15000) */ -import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; -import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; -import { Server } from '@modelcontextprotocol/server'; +import { serveStdio } from '@modelcontextprotocol/server/stdio'; import { spawn } from 'child_process'; import { randomUUID } from 'crypto'; import { fileURLToPath } from 'url'; @@ -50,10 +48,16 @@ import { resolveBackendTransport, updateBackendEnvOverrides, } from './backend-env.mjs'; +import { BackendStdioTransport } from './backend-stdio-transport.mjs'; import { LifecycleQueue, queueGenerationCleanup } from './lifecycle-queue.mjs'; import { runBuild } from './build-runner.mjs'; import { ensurePortFree, forceKillPid } from './process-cleanup.mjs'; -import { dedupeMcpErrorPrefix, assertBackendAvailable, backendCallOptions, backendFailureHint } from './tool-error.mjs'; +import { + backendCallOptions, + initialStartFailureLines, +} from './tool-error.mjs'; +import { createFront, ObservedStdioServerTransport } from './front-server.mjs'; +import { createSubscriptionRelay } from './backend-subscriptions.mjs'; // --------------------------------------------------------------------------- // Configuration @@ -160,7 +164,7 @@ class BackendManager { this.child = null; /** @type {Client | null} */ this.mcpClient = null; - /** @type {StdioClientTransport | null} */ + /** @type {BackendStdioTransport | null} */ this.stdioTransport = null; /** Keep ownership even when the SDK clears transport.pid during close(). */ this.stdioPid = null; @@ -183,6 +187,15 @@ class BackendManager { this.buildWaiters = new Set(); /** Unique owner for Docker containers started by this stable proxy process. */ this.dockerOwnerId = `${process.pid}-${randomUUID()}`; + /** The one initial start, once a request has asked for it (issue #1025, step 2). */ + this.initialStart = null; + /** Whether any start has begun, by whichever route (the deferred first start, a dev_* restart). */ + this.everStarted = false; + /** Announces the inventory after a successful initial start; set by main(). */ + this.onInitialStart = null; + /** The client's resource subscriptions, expressed in the backend's era; set by main(). */ + /** @type {{ reopen(): Promise, close(): Promise } | null} */ + this.relay = null; } // ---- Command computation ------------------------------------------------ @@ -219,6 +232,31 @@ class BackendManager { return this.lifecycleQueue.run(() => this._start()); } + /** + * The initial start, deferred to the first request that reaches a handler + * (issue #1025, step 2). A v2 client's `auto` negotiation probes server/discover + * on a disposable sibling process spawned from the same parameters; the SDK's + * stdio entry answers that probe itself, so a sibling never reaches a handler + * and never spawns a backend to race this one for the port. One start per + * process: a failed one is not retried here (dev_restart_debugger is the retry). + * Callers do not await it — `whenReady` bounds their wait (issue #716). + */ + ensureStarted() { + if (this.initialStart) return this.initialStart; + this.initialStart = this.start().then( + async () => { + // Refresh clients whose discovery gave up before the backend was ready. + await this.onInitialStart?.(); + return true; + }, + (err) => { + for (const line of initialStartFailureLines(err, this.shutdownController.signal.aborted)) log(line); + return false; + } + ); + return this.initialStart; + } + async _start() { this._assertOpen(); if (this.state === 'running' || this.state === 'starting') { @@ -226,6 +264,7 @@ class BackendManager { return; } + this.everStarted = true; this.state = 'starting'; this.expectedChildExit = false; const { command, args } = this._computeBackendCommand(); @@ -483,9 +522,16 @@ class BackendManager { buildCmd: BUILD_CMD, buildInProgress: this.buildInProgress, backendTransport: this.backendTransport, + protocolEra: this.state === 'running' && this.mcpClient ? (this.mcpClient.getProtocolEra?.() ?? null) : null, + protocolVersion: this.state === 'running' && this.mcpClient ? (this.mcpClient.getNegotiatedProtocolVersion?.() ?? null) : null, backendCmd: BACKEND_CMD || null, backendEnvOverrides: sanitizedEnv.values, backendEnvRedaction: sanitizedEnv.redaction, + // "stopped" alone reads like a crash. Before the first start it is the + // normal state: the backend starts on the first request that needs it. + ...(this.everStarted + ? {} + : { note: 'The backend has not been started yet: it starts on the first request that needs it (any listing or tool call other than the dev_* tools).' }), }; } @@ -537,7 +583,14 @@ class BackendManager { async _connectClient(command, args) { const closeState = { expected: false }; this.transportCloseState = closeState; - this.mcpClient = new Client({ name: 'dev-proxy', version: '1.0.0' }); + // The backend speaks both eras on both transports (issue #1025): negotiate. + // Over stdio that takes BackendStdioTransport (backend-stdio-transport.mjs), + // so the probe runs on the backend's own pipe rather than on a second + // backend process. + this.mcpClient = new Client( + { name: 'dev-proxy', version: '1.0.0' }, + { versionNegotiation: { mode: 'auto' } } + ); // Relay backend resource notifications (resources/updated, list_changed) // to the front client (issue #218). Registered as the fallback handler so @@ -553,13 +606,16 @@ class BackendManager { if (this.backendTransport === 'stdio') { // Stdio mode: StdioClientTransport spawns the child - const transport = new StdioClientTransport({ + // The shutdown signal goes to the transport as well as to connect(): the + // SDK's `auto` negotiation does not watch connect()'s, so without this a start + // still negotiating at shutdown would hang and orphan its backend. + const transport = new BackendStdioTransport({ command, args, cwd: PROJECT_ROOT, env: this._buildBackendEnv(), stderr: 'pipe', - }); + }, { signal: this.shutdownController.signal }); this.stdioTransport = transport; @@ -573,6 +629,8 @@ class BackendManager { }; transport.onclose = () => { + // The backend is gone; this transport is done with the shutdown signal. + transport.detachFromSignal(); log('Stdio transport closed'); // The SDK reports close only once the child has exited, so its PID is // no longer ours: a later force-kill could hit a process that reused it. @@ -595,6 +653,7 @@ class BackendManager { this.stdioPid = transport.pid; await connecting; log('MCP Client connected to backend via stdio'); + await this._reopenRelay(); } else { // Streamable HTTP mode (the only other value resolveBackendTransport // admits): SDK handles reconnection internally. @@ -621,13 +680,27 @@ class BackendManager { }; await this.mcpClient.connect(transport, { signal: this.shutdownController.signal }); - log('MCP Client connected to backend via Streamable HTTP'); + log(`MCP Client connected to backend via Streamable HTTP (${this.mcpClient.getProtocolEra()} era, ${this.mcpClient.getNegotiatedProtocolVersion()})`); + await this._reopenRelay(); + } + } + + async _reopenRelay() { + try { + await this.relay?.reopen(); + } catch (err) { + log(`Could not re-express the resource subscriptions to the backend: ${err?.message ?? err}`); } } async _disconnectClient() { if (this.mcpClient) { const client = this.mcpClient; + try { + await this.relay?.close(); + } catch (err) { + log(`Ignoring relay close error: ${err?.message ?? err}`); + } if (this.transportCloseState) this.transportCloseState.expected = true; try { await client.close(); @@ -748,7 +821,7 @@ const DEV_TOOLS = [ { name: 'dev_server_status', description: - 'Get the current status of the mcp-debugger backend (state, PID, uptime, buildInProgress, transport, project root, port, and display-safe environment overrides).', + 'Get the current status of the mcp-debugger backend (state, PID, uptime, buildInProgress, transport, protocol era and version, project root, port, and display-safe environment overrides).', inputSchema: { type: 'object', properties: {}, @@ -817,139 +890,64 @@ async function main() { const backend = new BackendManager(); - // Create the MCP Server that Claude Code talks to (via stdio) - const server = new Server( - { name: 'dev-proxy', version: '1.0.0' }, - { capabilities: { tools: { listChanged: true }, resources: { subscribe: true, listChanged: true } } } - ); + // The relay expresses the client's resource subscriptions to the backend in + // whichever era it speaks (issue #1025, step 2); the backend re-expresses the + // set after every (re)connect and ends its listen before going away. + // Not a stopped backend's client: a crash leaves a closed client behind until + // the next start, and the relay must not speak to it. A starting backend's + // client is live — the set is expressed right after it connects. + const relay = createSubscriptionRelay({ + getClient: () => (backend.state === 'stopped' ? null : backend.mcpClient), + log, + }); + backend.relay = relay; + + // The client-facing server, one instance per connection, in the era the client + // opens with (issue #1025, step 2): a probing client (Claude Code's v2 runtime) + // lands on 2026-07-28, an `initialize` client on the 2025 era. The dev tools + // are answered locally; everything else forwards to the backend. + const front = createFront({ + backend, + relay, + devTools: DEV_TOOLS, + handleDevTool: (name, args) => handleDevTool(backend, front, name, args), + discoveryWaitMs: DISCOVERY_WAIT_MS, + log, + }); - // Relay backend resource notifications to the front client (issue #218) + // Relay backend resource notifications to the pinned front instance (issue #218). backend.onResourceNotification = (notification) => { - server.notification(notification).catch((err) => { + front.notify(notification).catch((err) => { log(`Failed to relay ${notification.method}: ${err.message}`); }); }; - - // ListTools: forward live to backend, fall back to dev-tools-only when backend is down. - // The wait keeps the first inventory from being taken before the backend has - // started (issue #716); a cancelled request stops waiting with the client. - server.setRequestHandler('tools/list', async (_request, ctx) => { - await backend.whenReady({ signal: ctx?.mcpReq.signal }); - if (backend.state === 'running' && backend.mcpClient) { - try { - const result = await backend.mcpClient.listTools(); - return { tools: [...(result.tools || []), ...DEV_TOOLS] }; - } catch (err) { - log(`Live tools/list failed: ${err.message}`); - } - } - return { tools: [...DEV_TOOLS] }; - }); - - // CallTool: route dev_* locally, forward everything else to backend - server.setRequestHandler('tools/call', async (request, ctx) => { - const { name, arguments: args } = request.params; - - // Dev tools are always handled locally — status and recovery stay callable - // while a start or restart is still in flight. - if (name.startsWith('dev_')) { - return await handleDevTool(backend, server, name, args); - } - - // Forward to backend - try { - const result = await backend.callTool(name, args || {}, { signal: ctx?.mcpReq.signal }); - return result; - } catch (err) { - // A well-formed JSON-RPC error from a running backend (e.g. -32602 - // InvalidParams) proves the backend is alive — pass it through without - // the restart hint, which would send agents on a false detour (#304). - const body = { error: dedupeMcpErrorPrefix(err.message) }; - // Only a stopped backend wants restarting. Telling an agent to restart - // one that is mid-start queues a second restart that kills it (#716), - // and a running backend that was merely slow is neither (#854). - const hint = backendFailureHint(err, { - state: backend.state, - needsRestart: backend.needsRestart(), - discoveryWaitMs: DISCOVERY_WAIT_MS, - }); - if (hint) { - body.hint = hint; - } - return { - content: [{ type: 'text', text: JSON.stringify(body, null, 2) }], - isError: true, - }; - } - }); - - // Resources: pure passthrough to the backend (issue #218). Note that - // subscriptions live in the backend process, so they are lost when the - // backend is restarted (dev_rebuild_and_restart) — re-subscribe after. - server.setRequestHandler('resources/list', async () => { - if (backend.state === 'running' && backend.mcpClient) { - try { - return await backend.mcpClient.listResources(); - } catch (err) { - log(`Live resources/list failed: ${err.message}`); - } - } - return { resources: [] }; - }); - - server.setRequestHandler('resources/read', async (request, ctx) => { - await backend.whenReady({ signal: ctx?.mcpReq.signal }); - assertBackendAvailable(backend); - return await backend.mcpClient.readResource(request.params); - }); - - server.setRequestHandler('resources/subscribe', async (request, ctx) => { - await backend.whenReady({ signal: ctx?.mcpReq.signal }); - assertBackendAvailable(backend); - return await backend.mcpClient.subscribeResource(request.params); + // A rebuild whose new dist throws at startup leaves the client holding a tool + // list that no longer exists; the initial start announces its inventory too. + backend.onInitialStart = () => notifyToolListChanged(front); + + // Exit when the MCP client goes away — stdin EOF/close/error, the transport + // closing the wire by itself (a broken stdout, an oversized frame), or + // SIGINT/SIGTERM — stopping the backend child on the way out. Installed before + // the wire goes live so a client that dies during a slow backend start still + // triggers shutdown (backend.stop() handles a backend that never started). + // Without this, on Windows both the proxy and its backend outlive a dead + // Claude Code forever (issue #122). + const shutdown = installShutdownHandlers({ stdin: process.stdin, backend, log }); + const transport = new ObservedStdioServerTransport(() => { + void shutdown('stdio transport closed (MCP client gone)'); }); - server.setRequestHandler('resources/unsubscribe', async (request, ctx) => { - await backend.whenReady({ signal: ctx?.mcpReq.signal }); - assertBackendAvailable(backend); - return await backend.mcpClient.unsubscribeResource(request.params); + // The SDK's stdio entry owns the wire and answers server/discover itself. The + // backend is NOT started here: a v2 client's `auto` negotiation probes on a + // disposable sibling process spawned from the same parameters, which used to + // start a backend of its own and race this one for the port (issue #1025, + // step 2). The first request that reaches a handler starts it. + serveStdio(front.factory, { + transport, + onerror: (err) => log(`Front transport error: ${err?.message ?? err}`), }); - // Exit when the MCP client goes away — stdin EOF/close/error, protocol-level - // server close, or SIGINT/SIGTERM — stopping the backend child on the way out. - // Installed before backend.start() so a client that dies during a slow backend - // startup still triggers shutdown (backend.stop() handles the 'starting' state). - // Without this, on Windows both the proxy and its backend outlive a dead - // Claude Code forever (issue #122). Safe to install before server.connect(): - // only 'end'/'close'/'error' go on stdin, none of which make it flow. - installShutdownHandlers({ stdin: process.stdin, backend, server, log }); - - // Queue the automatic start BEFORE the transport goes live, so the first - // tools/list finds a lifecycle operation in flight and waits for it rather - // than racing it to a dev-tools-only inventory (issue #716). - const initialStart = backend.start().then( - () => true, - (err) => { - log(`Initial backend start failed: ${err.message}`); - log('Dev tools are still available — use dev_restart_debugger to retry'); - return false; - } - ); - - // Connect to stdio transport for Claude Code - const transport = new StdioServerTransport(); - await server.connect(transport); - - log(`Proxy server connected to stdio (backend transport: ${BACKEND_TRANSPORT})`); - - if (await initialStart) { - // Refresh clients whose discovery gave up before the backend was ready. - // Swallowing: a client that died during a slow start leaves the transport - // closed, and `notification()` throws 'Not connected'. Letting that escape - // would reject main() and hard-exit before the shutdown handler's queued - // backend.stop() ran, orphaning the child (issue #122). - await notifyToolListChanged(server); - } + log(`Proxy serving stdio in both protocol eras (backend transport: ${BACKEND_TRANSPORT}; the backend starts on first use)`); } main().catch((err) => { diff --git a/tools/dev-proxy/front-server.d.mts b/tools/dev-proxy/front-server.d.mts new file mode 100644 index 00000000..3db50714 --- /dev/null +++ b/tools/dev-proxy/front-server.d.mts @@ -0,0 +1,48 @@ +import type { Server } from '@modelcontextprotocol/server'; +import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; + +export interface FrontBackend { + state: string; + mcpClient: { + listTools(): Promise<{ tools?: unknown[] }>; + listResources(): Promise; + readResource(params: unknown): Promise; + } | null; + ensureStarted?(): unknown; + whenReady(options?: { signal?: AbortSignal }): Promise; + callTool(name: string, args: Record, options?: { signal?: AbortSignal }): Promise; + needsRestart?(): boolean; +} + +export interface FrontRelay { + subscribe(uri: string): Promise; + unsubscribe(uri: string): Promise; + /** Track everything the backend lists while a 2026 client is pinned. */ + trackAll?(enabled: boolean): void; + /** Re-read the backend's list (after a resources/list_changed, or when tracking starts). */ + refreshKnown?(): Promise; +} + +export interface Front { + /** The serveStdio factory: one fresh Server per connection or probe, in the era asked for. */ + factory: (ctx: { era: 'legacy' | 'modern' }) => Server; + /** The newest instance that has not closed: the relay target. */ + current(): Server | null; + /** Relay a backend notification to the pinned instance through its typed senders. */ + notify(notification: { method: string; params?: Record }): Promise; + sendToolListChanged(): Promise; +} + +export function createFront(deps: { + backend: FrontBackend; + relay: FrontRelay; + devTools: unknown[]; + handleDevTool: (name: string, args: Record) => Promise; + discoveryWaitMs?: number; + log?: (msg: string) => void; +}): Front; + +/** The SDK stdio transport that reports every close of the wire once. */ +export class ObservedStdioServerTransport extends StdioServerTransport { + constructor(onWireClosed: () => void); +} diff --git a/tools/dev-proxy/front-server.mjs b/tools/dev-proxy/front-server.mjs new file mode 100644 index 00000000..6655a43c --- /dev/null +++ b/tools/dev-proxy/front-server.mjs @@ -0,0 +1,252 @@ +/** + * The dev proxy's client-facing server (issue #1025, step 2, PR 5). + * + * Served through the SDK's stdio entry (`serveStdio(front.factory, …)`), which + * owns the wire, answers a client's `server/discover` probe itself, and pins one + * instance per connection in whichever protocol era the client opens with: a + * probing client (Claude Code's v2 runtime) lands on 2026-07-28, an `initialize` + * client on the 2025 era. The factory may run twice — once for a probe instance + * a client abandons for `initialize`, once for the pinned one — so the relay + * target (`current()`) is the newest instance that has not closed. + * + * The handlers forward to the backend and answer the dev tools locally. The + * backend is started by the first request that reaches a handler + * (`backend.ensureStarted()`): a disposable probe sibling never reaches one, so + * it never spawns a backend of its own. + */ +import { Server } from '@modelcontextprotocol/server'; +import { StdioServerTransport } from '@modelcontextprotocol/server/stdio'; +import { assertBackendAvailable, backendFailureHint, dedupeMcpErrorPrefix } from './tool-error.mjs'; + +/** + * @typedef {object} FrontBackend + * @property {string} state + * @property {{ listTools(): Promise<{ tools?: unknown[] }>, listResources(): Promise, readResource(params: unknown): Promise } | null} mcpClient + * @property {() => unknown} [ensureStarted] + * @property {(options?: { signal?: AbortSignal }) => Promise} whenReady + * @property {(name: string, args: Record, options?: { signal?: AbortSignal }) => Promise} callTool + * @property {() => boolean} [needsRestart] + * @property {{ pending: number }} [lifecycleQueue] + * @property {boolean} [buildInProgress] + */ + +/** + * @param {object} deps + * @param {FrontBackend} deps.backend + * @param {{ subscribe(uri: string): Promise, unsubscribe(uri: string): Promise }} deps.relay + * @param {unknown[]} deps.devTools + * @param {(name: string, args: Record) => Promise} deps.handleDevTool + * @param {number} [deps.discoveryWaitMs] + * @param {(msg: string) => void} [deps.log] + */ +export function createFront({ backend, relay, devTools, handleDevTool, discoveryWaitMs, log = () => {} }) { + /** @type {Server | null} */ + let current = null; + /** @type {'legacy' | 'modern' | null} */ + let currentEra = null; + + /** + * A 2026 client's listen set is served by the SDK entry and never reaches a + * handler, so while a modern instance is pinned the relay tracks everything + * the backend lists; the front's router delivers each client only what it + * listened for. Re-derived from the pinned instance whenever one is built or + * closes (a discarded probe closing must not switch a legacy pin's mode on). + */ + const applyTracking = () => { + const tracking = currentEra === 'modern'; + relay.trackAll?.(tracking); + if (tracking) { + Promise.resolve(relay.refreshKnown?.()).catch((err) => { + log(`Could not track the backend's resources: ${err?.message ?? err}`); + }); + } + }; + + const signalOf = (ctx) => ctx?.mcpReq?.signal; + + /** The backend's start is on demand; waiting for it is bounded by `whenReady`. */ + const ready = async (ctx) => { + void backend.ensureStarted?.(); + await backend.whenReady({ signal: signalOf(ctx) }); + }; + + const running = () => backend.state === 'running' && backend.mcpClient; + + const factory = ({ era }) => { + const server = new Server( + { name: 'dev-proxy', version: '1.0.0' }, + { capabilities: { tools: { listChanged: true }, resources: { subscribe: true, listChanged: true } } } + ); + + // ListTools: forward live to backend, fall back to dev-tools-only when the backend + // is down. The wait keeps the first inventory from being taken before the backend + // has started (issue #716); a cancelled request stops waiting with the client. + server.setRequestHandler('tools/list', async (_request, ctx) => { + await ready(ctx); + if (running()) { + try { + const result = await backend.mcpClient.listTools(); + return { tools: [...(result.tools || []), ...devTools] }; + } catch (err) { + log(`Live tools/list failed: ${err.message}`); + } + } + return { tools: [...devTools] }; + }); + + // CallTool: route dev_* locally, forward everything else to the backend. + server.setRequestHandler('tools/call', async (request, ctx) => { + const { name, arguments: args } = request.params; + + // Dev tools are always handled locally — status and recovery stay callable + // while a start or restart is still in flight. + if (name.startsWith('dev_')) { + return await handleDevTool(name, args ?? {}); + } + + void backend.ensureStarted?.(); + try { + return await backend.callTool(name, args || {}, { signal: signalOf(ctx) }); + } catch (err) { + // A well-formed JSON-RPC error from a running backend (e.g. -32602 + // InvalidParams) proves the backend is alive — pass it through without + // the restart hint, which would send agents on a false detour (#304). + const body = { error: dedupeMcpErrorPrefix(err.message) }; + // Only a stopped backend wants restarting. Telling an agent to restart + // one that is mid-start queues a second restart that kills it (#716), + // and a running backend that was merely slow is neither (#854). + const hint = backendFailureHint(err, { + state: backend.state, + needsRestart: backend.needsRestart?.() ?? false, + discoveryWaitMs, + }); + if (hint) { + body.hint = hint; + } + return { + content: [{ type: 'text', text: JSON.stringify(body, null, 2) }], + isError: true, + }; + } + }); + + // Resources: passthrough to the backend (issue #218); subscriptions go through + // the relay, which expresses them in the backend's era and re-sends them after + // a restart (issue #1025, step 2). + server.setRequestHandler('resources/list', async (_request, ctx) => { + await ready(ctx); + if (running()) { + try { + return await backend.mcpClient.listResources(); + } catch (err) { + log(`Live resources/list failed: ${err.message}`); + } + } + return { resources: [] }; + }); + + server.setRequestHandler('resources/read', async (request, ctx) => { + await ready(ctx); + assertBackendAvailable(backend); + return await backend.mcpClient.readResource(request.params); + }); + + server.setRequestHandler('resources/subscribe', async (request, ctx) => { + await ready(ctx); + assertBackendAvailable(backend); + await relay.subscribe(request.params.uri); + return {}; + }); + + server.setRequestHandler('resources/unsubscribe', async (request) => { + await relay.unsubscribe(request.params.uri); + return {}; + }); + + // The newest instance is the relay target; a discarded probe closing later + // must not clear a newer pin. + const previousClose = server.onclose; + server.onclose = () => { + if (current === server) { + current = null; + currentEra = null; + } + applyTracking(); + previousClose?.(); + }; + current = server; + currentEra = era; + applyTracking(); + log(`Front instance built for the ${era} era`); + return server; + }; + + /** + * Relay a backend notification to the pinned instance through its typed + * senders, so the SDK routes it in the client's era (a 2026 client's listen + * stream, a 2025 client's session). Dropped, not thrown, with no client pinned. + */ + const notify = async (notification) => { + const target = current; + if (!target) return; + switch (notification.method) { + case 'notifications/resources/updated': { + const uri = notification.params?.uri; + if (typeof uri !== 'string') { + log('Dropping a resources/updated without a uri'); + return; + } + await target.sendResourceUpdated({ uri }); + return; + } + case 'notifications/resources/list_changed': + // A new or removed debug session: the tracked set follows the backend's list. + try { + await relay.refreshKnown?.(); + } catch (err) { + log(`Could not refresh the tracked resources: ${err?.message ?? err}`); + } + await target.sendResourceListChanged(); + return; + case 'notifications/tools/list_changed': + await target.sendToolListChanged(); + return; + default: + await target.notification(notification); + } + }; + + return { + factory, + current: () => current, + notify, + sendToolListChanged: async () => { + await current?.sendToolListChanged(); + }, + }; +} + +/** + * The SDK's stdio transport closes itself — on stdin's EOF, but also on a stdout + * error (the client's end of the pipe is gone) or an oversized frame, with stdin + * still open — and tells only its own `onclose`, which `serveStdio` owns. This + * subclass lets the proxy hear every close of the wire, whatever ended it, so + * the process never lives on with a dead instance (issue #1025, step 2). + */ +export class ObservedStdioServerTransport extends StdioServerTransport { + #notified = false; + #onWireClosed; + + /** @param {() => void} onWireClosed */ + constructor(onWireClosed) { + super(); + this.#onWireClosed = onWireClosed; + } + + async close() { + await super.close(); + if (this.#notified) return; + this.#notified = true; + this.#onWireClosed(); + } +} diff --git a/tools/dev-proxy/shutdown.d.mts b/tools/dev-proxy/shutdown.d.mts index ebc565f2..6b2a2089 100644 --- a/tools/dev-proxy/shutdown.d.mts +++ b/tools/dev-proxy/shutdown.d.mts @@ -45,16 +45,10 @@ export interface StoppableBackend { stop(): Promise; } -/** The MCP `Server` whose `onclose` is chained — never `transport.onclose`. */ -export interface CloseableServer { - onclose?: (() => void) | undefined; -} - /** Dependencies of {@link installShutdownHandlers}. */ export interface ShutdownDeps { stdin: ShutdownStdin; backend: StoppableBackend; - server?: CloseableServer; log?: (msg: string) => void; /** Injectable for tests; defaults to `process`. */ proc?: ShutdownProcess; diff --git a/tools/dev-proxy/shutdown.mjs b/tools/dev-proxy/shutdown.mjs index 3fdbd1b7..c45f8b70 100644 --- a/tools/dev-proxy/shutdown.mjs +++ b/tools/dev-proxy/shutdown.mjs @@ -2,16 +2,16 @@ * Shutdown wiring for dev-proxy — exit when the MCP client (Claude Code) goes away. * * Why this exists (issue #122): on Windows, a dying parent process never delivers - * SIGINT/SIGTERM to its children, and the MCP SDK's StdioServerTransport only - * attaches 'data'/'error' listeners to stdin — it never notices EOF, and its - * onclose fires only on programmatic close(). So a dev-proxy whose client died - * kept the event loop alive forever, and its backend child along with it. + * SIGINT/SIGTERM to its children, so a dev-proxy whose client died kept the + * event loop alive forever, and its backend child along with it. * * The reliable cross-platform signal is the proxy's own stdin: when the client * process dies, the OS closes the pipe and stdin emits 'end' (or 'error' for a - * broken pipe) followed by 'close'. Any of those — plus protocol-level server - * close and SIGINT/SIGTERM — means "client is gone": stop the backend child, - * then exit. + * broken pipe) followed by 'close'. Any of those — plus SIGINT/SIGTERM — means + * "client is gone": stop the backend child, then exit. The stdio wire closing + * by itself (a stdout error, an oversized frame) is the proxy's other signal; + * the entry point hears that through its observed transport and calls the + * shutdown function this module returns (issue #1025). * * Kept in its own module so unit tests can import it without executing the * dev-proxy entry point (dev-proxy.mjs runs main() at module top level). @@ -50,16 +50,11 @@ export function isIntentionalTransportAbort(err) { * * @param {object} deps * @param {NodeJS.ReadableStream} deps.stdin - * The proxy's inbound stdin from the MCP client. Safe to listen on: the SDK's - * StdioServerTransport only attaches 'data'/'error' listeners, and its 'data' - * listener keeps the stream flowing so 'end' fires at EOF. + * The proxy's inbound stdin from the MCP client. Safe to listen on beside the + * SDK's StdioServerTransport: Node delivers each event to every listener. * @param {{ stop(): Promise }} deps.backend * Backend manager to stop before exit. stop() handles both transport * modes (http/stdio) including a backend that is still starting. - * @param {{ onclose?: (() => void) | undefined }} [deps.server] - * MCP Server whose onclose is chained (not replaced). Note: hook the SERVER, - * never transport.onclose — Protocol.connect() owns transport.onclose and - * overwriting it breaks the SDK's close chain. * @param {(msg: string) => void} [deps.log] * @param {Pick} [deps.proc] Injectable for tests. * @param {number} [deps.stopTimeoutMs] @@ -69,7 +64,6 @@ export function isIntentionalTransportAbort(err) { export function installShutdownHandlers({ stdin, backend, - server, log = () => {}, proc = process, stopTimeoutMs = STOP_TIMEOUT_MS, @@ -116,19 +110,6 @@ export function installShutdownHandlers({ void shutdown(`stdin error (${err?.message ?? err}) — treating as client disconnect`) ); - // Secondary trigger: protocol-level close (client sent a proper shutdown). - if (server) { - const previousOnClose = server.onclose; - server.onclose = () => { - try { - previousOnClose?.(); - } catch (err) { - log(`Ignoring server onclose handler error: ${err?.message ?? err}`); - } - void shutdown('MCP server connection closed'); - }; - } - // Signals route through the same idempotent path (parity with prior behavior). proc.on('SIGINT', () => void shutdown('SIGINT received')); proc.on('SIGTERM', () => void shutdown('SIGTERM received')); diff --git a/tools/dev-proxy/tool-error.d.mts b/tools/dev-proxy/tool-error.d.mts index 03cce5ae..3e13e2a2 100644 --- a/tools/dev-proxy/tool-error.d.mts +++ b/tools/dev-proxy/tool-error.d.mts @@ -65,3 +65,10 @@ export function backendFailureHint( err: unknown, backend: { state: string; needsRestart: boolean; discoveryWaitMs: number } ): string | undefined; + +/** + * The log lines for a deferred first backend start that did not happen: the + * failure and how to retry it, or — while the proxy is shutting down — only + * that it is. + */ +export function initialStartFailureLines(err: unknown, shuttingDown: boolean): string[]; diff --git a/tools/dev-proxy/tool-error.mjs b/tools/dev-proxy/tool-error.mjs index eda6500f..06cc2b7f 100644 --- a/tools/dev-proxy/tool-error.mjs +++ b/tools/dev-proxy/tool-error.mjs @@ -144,3 +144,24 @@ export function assertBackendAvailable(backend) { ); } } + +/** + * What the proxy logs when its deferred first backend start does not happen. + * A request that lands while the proxy is shutting down asks for a start that + * is refused on purpose: that is said as it is, without the retry hint a real + * failure gets — there is nothing to retry and nobody left to retry it. + * + * @param {unknown} err + * @param {boolean} shuttingDown + * @returns {string[]} + */ +export function initialStartFailureLines(err, shuttingDown) { + if (shuttingDown) { + return ['Not starting the backend: the proxy is shutting down']; + } + const message = err instanceof Error ? err.message : String(err); + return [ + `Initial backend start failed: ${message}`, + 'Dev tools are still available — use dev_restart_debugger to retry', + ]; +}