From 59e2cbdb90ddfe376dc4341c344e9ac80c82f158 Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 9 Nov 2026 22:54:30 -0500 Subject: [PATCH 1/6] feat(dev-proxy): speak both protocol eras, negotiate the backend's, and start the backend on first use (#1025) Step 2 of #1025, PR 5 of 5: the development loop's proxy. Measured first, as the plan required: a v2 client's `auto` negotiation over the SDK's own StdioClientTransport probes server/discover on a disposable SIBLING process spawned from the same parameters, then starts the real one (the SDK's negotiateStdioViaSibling; a subclass probes in place). The proxy ran main() in full on that sibling, including the unconditional backend.start(), so every probing connect started a backend twice and ran ensurePortFree twice against one port; ten dev-proxy processes had idled for four days under this Claude Code session with two backends between them. The client-facing server is now served through the SDK's stdio entry (tools/dev-proxy/front-server.mjs): one instance per connection in the era the client opens with, the relay target following the pinned instance, the handlers moved out of main(), and an ObservedStdioServerTransport so the proxy exits when the wire closes by itself. The backend starts on the first request that reaches a handler (BackendManager.ensureStarted): the SDK entry answers server/discover itself, so a probe sibling never reaches a handler and never spawns a backend. The first tools/list still waits for the start (#716), and a successful initial start still announces tools/list_changed. The backend Client negotiates its era for an http backend (legacy for a stdio one, where `auto` would probe on a sibling of the backend); dev_server_status reports protocolEra and protocolVersion. Resource subscriptions are translated across eras by backend-subscriptions.mjs: one resources/subscribe per URI to a 2025 backend, re-sent after a restart; ONE subscriptions/listen with the URIs plus the list-changed flags to a 2026 backend, re-opened on set change and after every (re)connect. The backend runs with MCP_DEBUG_SESSION_IDLE_MS=0 unless the environment or an override sets it. Measured after: the same connect lands on modern / 2026-07-28, the sibling spawns nothing, the backend starts on the first tools/list, and the proxy's backend connection is in the modern era. Co-Authored-By: Claude Fable 5.1 --- CLAUDE.md | 4 +- changelog.d/1025-dev-proxy.changed.md | 1 + tests/fixtures/dev-proxy/startup-backend.mjs | 13 +- tests/integration/dev-proxy-eras.test.ts | 110 ++++++++ tests/unit/dev-proxy/backend-env.test.ts | 17 ++ .../dev-proxy/backend-subscriptions.test.ts | 159 ++++++++++++ tests/unit/dev-proxy/front-server.test.ts | 149 +++++++++++ tools/dev-proxy/README.md | 23 ++ tools/dev-proxy/backend-env.d.mts | 8 + tools/dev-proxy/backend-env.mjs | 14 ++ tools/dev-proxy/backend-subscriptions.d.mts | 23 ++ tools/dev-proxy/backend-subscriptions.mjs | 103 ++++++++ tools/dev-proxy/dev-proxy.mjs | 238 ++++++++---------- tools/dev-proxy/front-server.d.mts | 44 ++++ tools/dev-proxy/front-server.mjs | 215 ++++++++++++++++ 15 files changed, 987 insertions(+), 134 deletions(-) create mode 100644 changelog.d/1025-dev-proxy.changed.md create mode 100644 tests/integration/dev-proxy-eras.test.ts create mode 100644 tests/unit/dev-proxy/backend-subscriptions.test.ts create mode 100644 tests/unit/dev-proxy/front-server.test.ts create mode 100644 tools/dev-proxy/backend-subscriptions.d.mts create mode 100644 tools/dev-proxy/backend-subscriptions.mjs create mode 100644 tools/dev-proxy/front-server.d.mts create mode 100644 tools/dev-proxy/front-server.mjs diff --git a/CLAUDE.md b/CLAUDE.md index 2525d1dc..b45394d5 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 an `http` backend, 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..057531af --- /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 an `http` backend the proxy negotiates the era too (a stdio backend stays on 2025), and `dev_server_status` reports `protocolEra`/`protocolVersion`. 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. 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 backend runs with `MCP_DEBUG_SESSION_IDLE_MS=0` unless the environment or an override sets it, and 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/startup-backend.mjs b/tests/fixtures/dev-proxy/startup-backend.mjs index c905fafe..debdef9b 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,13 @@ 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 the + // proxy subscribed on a client's behalf (issue #1025, step 2: the proxy's + // subscription relay across eras). + const touch = request.params.arguments?.touch; + if (typeof touch === 'string') { + 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 +64,9 @@ 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: accepted and forgotten, like the real server's +// per-connection set (the proxy re-sends them after a restart). +server.setRequestHandler('resources/subscribe', async () => ({})); +server.setRequestHandler('resources/unsubscribe', async () => ({})); 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..548c1601 --- /dev/null +++ b/tests/integration/dev-proxy-eras.test.ts @@ -0,0 +1,110 @@ +/** + * 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. + 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; + expect(JSON.parse(text)).toMatchObject({ state: 'stopped', protocolEra: null }); + 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: {} }); + expect(JSON.parse((after.content as Array<{ text: string }>)[0].text)).toMatchObject({ + state: 'running', protocolEra: 'legacy', protocolVersion: '2025-11-25', + }); + }); + + 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); + }); + + 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; + // the proxy had subscribed it through resources/subscribe on the client's behalf. + const touched = await client.callTool({ name: 'fixture_tool', arguments: { touch: 'fixture://status' } }); + expect(touched.isError).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..1ed1d79f 100644 --- a/tests/unit/dev-proxy/backend-env.test.ts +++ b/tests/unit/dev-proxy/backend-env.test.ts @@ -4,6 +4,7 @@ import { resolveBackendPort, resolveBackendTransport, updateBackendEnvOverrides, + withBackendDefaults, } from '../../../tools/dev-proxy/backend-env.mjs'; describe('dev-proxy backend port (issue #689)', () => { @@ -80,3 +81,19 @@ describe('dev-proxy backend environment overrides', () => { expect(result.MCP_EXIT_ON_STDIN_CLOSE).toBe('1'); }); }); + +describe('dev-proxy backend defaults (issue #1025, step 2)', () => { + // One client, and the proxy's own lifetime is the backend's: the per-debug-session + // idle policy (30 min on http) would close a developer's paused session under them. + it('turns the backend idle policy off unless the environment already says otherwise', () => { + expect(withBackendDefaults({ PATH: '/bin' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '0', PATH: '/bin' }); + expect(withBackendDefaults({ MCP_DEBUG_SESSION_IDLE_MS: '60000' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '60000' }); + // An explicitly empty value is the user's too: the server treats it as unset, not ours to rewrite. + expect(withBackendDefaults({ MCP_DEBUG_SESSION_IDLE_MS: '' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '' }); + }); + + it('lets a tool-call override win over the default', () => { + const env = buildBackendEnvironment(withBackendDefaults({}), { MCP_DEBUG_SESSION_IDLE_MS: '5000' }); + expect(env.MCP_DEBUG_SESSION_IDLE_MS).toBe('5000'); + }); +}); 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..b8b187a2 --- /dev/null +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -0,0 +1,159 @@ +/** + * 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) { + const closes: Array> = []; + const client = { + getProtocolEra: vi.fn(() => era), + subscribeResource: vi.fn(async (_params: { uri: string }) => ({})), + unsubscribeResource: vi.fn(async (_params: { uri: string }) => ({})), + 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' }); + }); + }); + + 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'], + })); + }); + + 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/front-server.test.ts b/tests/unit/dev-proxy/front-server.test.ts new file mode 100644 index 00000000..7896832f --- /dev/null +++ b/tests/unit/dev-proxy/front-server.test.ts @@ -0,0 +1,149 @@ +/** + * 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 } 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 () => {}) }; +} + +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(); + }); + + 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()); + }); +}); diff --git a/tools/dev-proxy/README.md b/tools/dev-proxy/README.md index aa4f230c..42a9290d 100644 --- a/tools/dev-proxy/README.md +++ b/tools/dev-proxy/README.md @@ -12,6 +12,24 @@ 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 an `http` backend the proxy negotiates the same way and lands on 2026-07-28; a stdio +backend stays on the 2025 era. `dev_server_status` reports `protocolEra` and `protocolVersion`. +Resource subscriptions are translated across eras: the set the client subscribed to 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. + +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 ``` @@ -158,6 +176,11 @@ preserves the current set, and passing `{}` clears it. Overrides are merged into backend spawn but are not passed to the build command. Proxy-controlled values required for clean shutdown take precedence. +One default sits under everything else: the backend runs with `MCP_DEBUG_SESSION_IDLE_MS=0` +(the per-debug-session idle policy off) unless your environment or an override sets the variable +— one client, and the proxy's lifetime is the backend's, so a 30-minute window would close a +paused session under you. + Each request's overrides are applied when its queued restart begins, after any build succeeds. An omitted `env` inherits the preceding applied settings. Concurrent requests cannot overwrite one another's settings, and each response reports the backend generation that its own restart started. diff --git a/tools/dev-proxy/backend-env.d.mts b/tools/dev-proxy/backend-env.d.mts index 3fca2f2e..26b325e6 100644 --- a/tools/dev-proxy/backend-env.d.mts +++ b/tools/dev-proxy/backend-env.d.mts @@ -39,3 +39,11 @@ export function buildBackendEnvironment( overrides: Record, forced?: Record ): Record; + +/** + * The proxy's backend defaults under the inherited environment, so the user's + * own setting wins: `MCP_DEBUG_SESSION_IDLE_MS=0` unless set (issue #1025). + */ +export function withBackendDefaults( + inherited: Record +): Record; diff --git a/tools/dev-proxy/backend-env.mjs b/tools/dev-proxy/backend-env.mjs index 4a087227..76008864 100644 --- a/tools/dev-proxy/backend-env.mjs +++ b/tools/dev-proxy/backend-env.mjs @@ -80,3 +80,17 @@ export function updateBackendEnvOverrides(current, args) { export function buildBackendEnvironment(inherited, overrides, forced = {}) { return { ...inherited, ...overrides, ...forced }; } + +/** + * The proxy's defaults for its backend, under the inherited environment so the + * user's own setting always wins (issue #1025, step 2): one client, and the + * proxy's lifetime is the backend's, so the per-debug-session idle policy + * (30 min on http) is off unless the environment says otherwise — it would + * close a developer's paused session under them. + * + * @param {Record} inherited + * @returns {Record} + */ +export function withBackendDefaults(inherited) { + return { MCP_DEBUG_SESSION_IDLE_MS: '0', ...inherited }; +} diff --git a/tools/dev-proxy/backend-subscriptions.d.mts b/tools/dev-proxy/backend-subscriptions.d.mts new file mode 100644 index 00000000..19b39ae9 --- /dev/null +++ b/tools/dev-proxy/backend-subscriptions.d.mts @@ -0,0 +1,23 @@ +export interface RelayClient { + getProtocolEra?(): 'legacy' | 'modern'; + subscribeResource(params: { uri: string }): Promise; + unsubscribeResource(params: { uri: string }): Promise; + listen(filter: { + resourceSubscriptions: string[]; + toolsListChanged: boolean; + resourcesListChanged: boolean; + }): Promise<{ close(): Promise }>; +} + +export interface SubscriptionRelay { + uris(): string[]; + subscribe(uri: string): Promise; + unsubscribe(uri: string): Promise; + reopen(): Promise; + 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..749b2fa7 --- /dev/null +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -0,0 +1,103 @@ +/** + * 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). + * + * 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 {(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 = () => {} }) { + /** @type {Set} */ + const uris = new Set(); + /** The modern-era listen currently open, if any. */ + /** @type {{ close(): Promise } | null} */ + let open = null; + + const isModern = (client) => client?.getProtocolEra?.() === 'modern'; + + 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}`); + } + }; + + /** Express the whole set to the current backend, whichever era it speaks. */ + const express = async () => { + const client = getClient(); + if (!client) return; + if (isModern(client)) { + await closeOpen(); + open = await client.listen({ + resourceSubscriptions: [...uris], + toolsListChanged: true, + resourcesListChanged: true, + }); + return; + } + for (const uri of uris) await client.subscribeResource({ uri }); + }; + + return { + uris: () => [...uris], + + /** @param {string} uri */ + async subscribe(uri) { + if (uris.has(uri)) return; + const client = getClient(); + if (client && !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); + if (client) await express(); + }, + + /** @param {string} uri */ + async unsubscribe(uri) { + if (!uris.delete(uri)) return; + const client = getClient(); + if (!client) return; + if (!isModern(client)) { + await client.unsubscribeResource({ uri }); + return; + } + await express(); + }, + + /** After a backend (re)connect: the new backend knows nothing of the set yet. */ + reopen: express, + + /** Before a backend goes away: end the open listen; the set stays for the next one. */ + close: closeOpen, + }; +} diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 2bca4835..6a515c10 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -23,8 +23,7 @@ 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'; @@ -49,11 +48,14 @@ import { resolveBackendPort, resolveBackendTransport, updateBackendEnvOverrides, + withBackendDefaults, } from './backend-env.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 } from './tool-error.mjs'; +import { createFront, ObservedStdioServerTransport } from './front-server.mjs'; +import { createSubscriptionRelay } from './backend-subscriptions.mjs'; // --------------------------------------------------------------------------- // Configuration @@ -183,6 +185,13 @@ 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; + /** 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 ------------------------------------------------ @@ -207,7 +216,7 @@ class BackendManager { _buildBackendEnv({ forceStdinClose = false } = {}) { return buildBackendEnvironment( - process.env, + withBackendDefaults(process.env), this.backendEnvOverrides, forceStdinClose ? { MCP_EXIT_ON_STDIN_CLOSE: '1' } : {} ); @@ -219,6 +228,32 @@ 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) => { + log(`Initial backend start failed: ${err.message}`); + log('Dev tools are still available — use dev_restart_debugger to retry'); + return false; + } + ); + return this.initialStart; + } + async _start() { this._assertOpen(); if (this.state === 'running' || this.state === 'starting') { @@ -483,6 +518,8 @@ 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, @@ -537,7 +574,12 @@ 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 http backend speaks both eras (issue #1025): negotiate. A stdio backend + // stays legacy — `auto` over stdio would probe on a sibling of the backend. + this.mcpClient = new Client( + { name: 'dev-proxy', version: '1.0.0' }, + { versionNegotiation: { mode: this.backendTransport === 'http' ? 'auto' : 'legacy' } } + ); // Relay backend resource notifications (resources/updated, list_changed) // to the front client (issue #218). Registered as the fallback handler so @@ -595,6 +637,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 +664,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 +805,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 +874,58 @@ 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. + const relay = createSubscriptionRelay({ getClient: () => 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..08569f7c --- /dev/null +++ b/tools/dev-proxy/front-server.d.mts @@ -0,0 +1,44 @@ +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; +} + +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..c8afc045 --- /dev/null +++ b/tools/dev-proxy/front-server.mjs @@ -0,0 +1,215 @@ +/** + * 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; + + 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; + previousClose?.(); + }; + current = server; + 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': + await target.sendResourceUpdated({ uri: notification.params?.uri }); + return; + case 'notifications/resources/list_changed': + 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(); + } +} From bdf402ee95f14095fb0250ebe4c512717cdd10ab Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 9 Nov 2026 23:21:38 -0500 Subject: [PATCH 2/6] fix(dev-proxy): track the backend's resources for a 2026 client, serialize the relay, and say what a modern pin drops (#1025) Three findings from the review of PR #1118. A 2026-07-28 front client's subscriptions never reached the backend: its subscriptions/listen is served by the SDK entry and never reaches a handler, and relay.subscribe was only called from the resources/subscribe handler a 2026 client cannot send. The relay's set stayed empty, so no output pings flowed for exactly the client this PR exists for. The fixture hid it by pinging without a subscription. Now, while a modern instance is pinned, the relay tracks every resource the backend lists (trackAll), re-read on each relayed resources/list_changed (refreshKnown) and after every (re)connect; the front's own router delivers each client only what it listened for. The fixture backend keeps a subscribe set and refuses a touch for an unsubscribed URI, so the integration test now pins the chain. Every expression against the backend is serialized: two subscribes arriving back-to-back used to both open a listen and leak the first one (and a 2026 backend emits one copy per live subscription). A refused modern listen no longer remembers its URI. The relay only ever sees a non-stopped backend's client. README: a 2026 client's tools/list_changed is dropped unless its listen opted in (toolsListChanged: true), so after an inventory-changing restart such a client keeps its list until it lists again; the http backend lands on 2026-07-28 only when its dist serves it. Live, through the proxy against dist/index.js http: a pinned 2026 client hears the backend's resources/list_changed on create_debug_session and a resources/updated for the session's output once a Python debuggee prints. Co-Authored-By: Claude Fable 5.1 --- tests/fixtures/dev-proxy/startup-backend.mjs | 26 ++- tests/integration/dev-proxy-eras.test.ts | 11 +- .../dev-proxy/backend-subscriptions.test.ts | 99 ++++++++++- tests/unit/dev-proxy/front-server.test.ts | 74 +++++++- tools/dev-proxy/README.md | 19 ++- tools/dev-proxy/backend-subscriptions.d.mts | 8 + tools/dev-proxy/backend-subscriptions.mjs | 161 +++++++++++++++--- tools/dev-proxy/dev-proxy.mjs | 8 +- tools/dev-proxy/front-server.d.mts | 4 + tools/dev-proxy/front-server.mjs | 43 ++++- 10 files changed, 402 insertions(+), 51 deletions(-) diff --git a/tests/fixtures/dev-proxy/startup-backend.mjs b/tests/fixtures/dev-proxy/startup-backend.mjs index debdef9b..f7aa9606 100644 --- a/tests/fixtures/dev-proxy/startup-backend.mjs +++ b/tests/fixtures/dev-proxy/startup-backend.mjs @@ -42,11 +42,16 @@ 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 the - // proxy subscribed on a client's behalf (issue #1025, step 2: the proxy's - // subscription relay across eras). + // `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) { @@ -64,9 +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: accepted and forgotten, like the real server's -// per-connection set (the proxy re-sends them after a restart). -server.setRequestHandler('resources/subscribe', async () => ({})); -server.setRequestHandler('resources/unsubscribe', async () => ({})); +// 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 index 548c1601..1896cef8 100644 --- a/tests/integration/dev-proxy-eras.test.ts +++ b/tests/integration/dev-proxy-eras.test.ts @@ -67,6 +67,8 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { 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: {} }); @@ -100,10 +102,13 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { await client.listTools(); const subscription = await client.listen({ resourceSubscriptions: ['fixture://status'] }); - // The fixture backend (2025) sends resources/updated for a touched URI; - // the proxy had subscribed it through resources/subscribe on the client's behalf. + // 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).not.toBe(true); + 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-subscriptions.test.ts b/tests/unit/dev-proxy/backend-subscriptions.test.ts index b8b187a2..3a846769 100644 --- a/tests/unit/dev-proxy/backend-subscriptions.test.ts +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -15,12 +15,13 @@ import { createSubscriptionRelay } from '../../../tools/dev-proxy/backend-subscr type Era = 'legacy' | 'modern'; -function fakeClient(era: Era) { +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); @@ -146,6 +147,102 @@ describe('dev-proxy backend subscription relay', () => { })); }); + describe('serialization (review of PR #1118, Important #2)', () => { + 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 new Error('Unknown resource: ' + uri); + 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')); diff --git a/tests/unit/dev-proxy/front-server.test.ts b/tests/unit/dev-proxy/front-server.test.ts index 7896832f..7bd7d1d1 100644 --- a/tests/unit/dev-proxy/front-server.test.ts +++ b/tests/unit/dev-proxy/front-server.test.ts @@ -10,7 +10,7 @@ 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 } from '../../../tools/dev-proxy/front-server.mjs'; +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: {} } }]; @@ -32,7 +32,12 @@ function fakeBackend(overrides: Record = {}) { } function fakeRelay() { - return { subscribe: vi.fn(async () => {}), unsubscribe: vi.fn(async () => {}) }; + 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)', () => { @@ -136,6 +141,59 @@ describe('dev-proxy front behind serveStdio (issue #1025, step 2)', () => { 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' } }); @@ -147,3 +205,15 @@ describe('dev-proxy front behind serveStdio (issue #1025, step 2)', () => { 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/tools/dev-proxy/README.md b/tools/dev-proxy/README.md index 42a9290d..69b95a34 100644 --- a/tools/dev-proxy/README.md +++ b/tools/dev-proxy/README.md @@ -16,12 +16,19 @@ The proxy speaks both MCP protocol eras to its client (issue #1025): it is serve 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 an `http` backend the proxy negotiates the same way and lands on 2026-07-28; a stdio -backend stays on the 2025 era. `dev_server_status` reports `protocolEra` and `protocolVersion`. -Resource subscriptions are translated across eras: the set the client subscribed to 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. +Towards an `http` backend the proxy negotiates the same way and lands on 2026-07-28 when that +backend's dist serves it (an older dist answers the 2025 era; `dev_server_status` says which +under `protocolEra`/`protocolVersion`); a stdio backend stays on the 2025 era. 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. 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 diff --git a/tools/dev-proxy/backend-subscriptions.d.mts b/tools/dev-proxy/backend-subscriptions.d.mts index 19b39ae9..6520ff29 100644 --- a/tools/dev-proxy/backend-subscriptions.d.mts +++ b/tools/dev-proxy/backend-subscriptions.d.mts @@ -2,6 +2,7 @@ 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; @@ -10,10 +11,17 @@ export interface RelayClient { } 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; } diff --git a/tools/dev-proxy/backend-subscriptions.mjs b/tools/dev-proxy/backend-subscriptions.mjs index 749b2fa7..5ab67b39 100644 --- a/tools/dev-proxy/backend-subscriptions.mjs +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -12,6 +12,17 @@ * 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. + * * 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 @@ -23,6 +34,7 @@ * @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 */ @@ -30,13 +42,32 @@ * @param {{ getClient: () => RelayClient | null | undefined, log?: (msg: string) => void }} deps */ export function createSubscriptionRelay({ getClient, log = () => {} }) { - /** @type {Set} */ + /** What front clients asked for by name. @type {Set} */ const uris = new Set(); - /** The modern-era listen currently open, if any. */ - /** @type {{ close(): Promise } | null} */ + /** What the backend lists, while a 2026 front client is pinned. @type {string[]} */ + let known = []; + let trackingAll = false; + /** 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 isModern = (client) => client?.getProtocolEra?.() === 'modern'; + const wanted = () => [...new Set([...uris, ...(trackingAll ? known : [])])]; const closeOpen = async () => { const current = open; @@ -49,55 +80,129 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { } }; - /** Express the whole set to the current backend, whichever era it speaks. */ - const express = async () => { - const client = getClient(); - if (!client) return; + /** 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'); + }; + + /** One tracked URI the backend refuses is logged, not fatal; a named one is the client's answer. */ + const subscribeOne = async (client, uri) => { + try { + await client.subscribeResource({ uri }); + } catch (err) { + if (uris.has(uri)) throw err; + 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: [...uris], + resourceSubscriptions: wanted(), toolsListChanged: true, resourcesListChanged: true, }); return; } - for (const uri of uris) await client.subscribeResource({ uri }); + 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 = []; + }, + /** @param {string} uri */ async subscribe(uri) { if (uris.has(uri)) return; - const client = getClient(); - if (client && !isModern(client)) { - // A refusal (an unknown resource) is the front client's answer: not remembered. - await client.subscribeResource({ 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); - return; - } - uris.add(uri); - if (client) await express(); + try { + await express(client); + } catch (err) { + uris.delete(uri); + throw err; + } + }); }, /** @param {string} uri */ async unsubscribe(uri) { - if (!uris.delete(uri)) return; - const client = getClient(); - if (!client) return; - if (!isModern(client)) { - await client.unsubscribeResource({ uri }); - return; - } - await express(); + if (!uris.has(uri)) return; + 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: express, + 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: closeOpen, + close() { + return serialize(closeOpen); + }, }; } diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 6a515c10..a565025e 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -877,7 +877,13 @@ async function main() { // 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. - const relay = createSubscriptionRelay({ getClient: () => backend.mcpClient, log }); + // 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 diff --git a/tools/dev-proxy/front-server.d.mts b/tools/dev-proxy/front-server.d.mts index 08569f7c..3db50714 100644 --- a/tools/dev-proxy/front-server.d.mts +++ b/tools/dev-proxy/front-server.d.mts @@ -17,6 +17,10 @@ export interface FrontBackend { 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 { diff --git a/tools/dev-proxy/front-server.mjs b/tools/dev-proxy/front-server.mjs index c8afc045..6655a43c 100644 --- a/tools/dev-proxy/front-server.mjs +++ b/tools/dev-proxy/front-server.mjs @@ -42,6 +42,25 @@ import { assertBackendAvailable, backendFailureHint, dedupeMcpErrorPrefix } from 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; @@ -148,10 +167,16 @@ export function createFront({ backend, relay, devTools, handleDevTool, discovery // must not clear a newer pin. const previousClose = server.onclose; server.onclose = () => { - if (current === server) current = null; + if (current === server) { + current = null; + currentEra = null; + } + applyTracking(); previousClose?.(); }; current = server; + currentEra = era; + applyTracking(); log(`Front instance built for the ${era} era`); return server; }; @@ -165,10 +190,22 @@ export function createFront({ backend, relay, devTools, handleDevTool, discovery const target = current; if (!target) return; switch (notification.method) { - case 'notifications/resources/updated': - await target.sendResourceUpdated({ uri: notification.params?.uri }); + 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': From 84d1bad80d7242cffb9596c91cd4e0bbd3832599 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 10 Nov 2026 12:19:12 -0500 Subject: [PATCH 3/6] fix(dev-proxy): negotiate with a stdio backend in place, survive a backend that refuses a subscription, and say when the backend has not started yet (#1025) A second look at three rulings made while building step 2, on JF's review. A stdio backend was pinned to the 2025 era because the SDK's `auto` negotiation probes on a disposable sibling process, which here would be a second backend. The SDK documents the way around that: only its exact base stdio transport takes the sibling path, and a subclass probes on the connection itself. The proxy's backend transport is now such a subclass and negotiates on both transports, so the stdio-backend entries exercise the server's modern stdio leg, with the backend still started once. Measured through the proxy: the current build lands on 2026-07-28; the legacy fixture and the published 0.26.0 npx bundle and Docker image answer the probe with "method not found" and fall back to 2025-11-25 on the same pipe, tool calls working. The subscription relay kept a refused subscription forever and rethrew on it. A backend restart is the proxy's purpose and it ends every debug session, so each subscription a client named before it then points at nothing: the first refusal aborted the re-expression, and every later reopen failed at the same URI. Re-expression no longer stops at a refusal: a URI the client named is dropped and said once, a listed resource the backend will not subscribe (its proxy-log tails) is asked once and remembered until the backend stops listing it. Only a JSON-RPC error answer counts as a refusal; a failed connection still surfaces, and a client's own subscribe still gets its refusal. Smaller: - dev_server_status before the first start says the backend starts on the first request that needs it; "stopped" alone read like a crash. - A request that lands while the proxy is shutting down logs that it is, not "Initial backend start failed" with a hint to retry. - The proxy no longer forces MCP_DEBUG_SESSION_IDLE_MS=0 on its backend: the idle policy is off by default now, so the override was dead. - installShutdownHandlers loses its unused `server` parameter and the stale note that the SDK transport never notices EOF. Co-Authored-By: Claude Fable 5.2 --- CLAUDE.md | 2 +- changelog.d/1025-dev-proxy.changed.md | 2 +- tests/fixtures/dev-proxy/real-backend.mjs | 18 ++++ tests/integration/dev-proxy-eras.test.ts | 35 +++++++- tests/unit/dev-proxy/backend-env.test.ts | 16 ---- .../dev-proxy/backend-subscriptions.test.ts | 90 ++++++++++++++++++- .../unit/dev-proxy/dev-proxy-shutdown.test.ts | 14 --- tests/unit/dev-proxy/tool-error.test.ts | 18 ++++ tools/dev-proxy/README.md | 21 ++--- tools/dev-proxy/backend-env.d.mts | 7 -- tools/dev-proxy/backend-env.mjs | 13 --- tools/dev-proxy/backend-subscriptions.mjs | 41 ++++++++- tools/dev-proxy/dev-proxy.mjs | 37 ++++++-- tools/dev-proxy/shutdown.d.mts | 6 -- tools/dev-proxy/shutdown.mjs | 37 ++------ tools/dev-proxy/tool-error.d.mts | 7 ++ tools/dev-proxy/tool-error.mjs | 21 +++++ 17 files changed, 271 insertions(+), 114 deletions(-) create mode 100644 tests/fixtures/dev-proxy/real-backend.mjs diff --git a/CLAUDE.md b/CLAUDE.md index 46e4130a..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. 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 an `http` backend, 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`). +**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) diff --git a/changelog.d/1025-dev-proxy.changed.md b/changelog.d/1025-dev-proxy.changed.md index 057531af..53e2f054 100644 --- a/changelog.d/1025-dev-proxy.changed.md +++ b/changelog.d/1025-dev-proxy.changed.md @@ -1 +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 an `http` backend the proxy negotiates the era too (a stdio backend stays on 2025), and `dev_server_status` reports `protocolEra`/`protocolVersion`. 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. 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 backend runs with `MCP_DEBUG_SESSION_IDLE_MS=0` unless the environment or an override sets it, and 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. +**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/integration/dev-proxy-eras.test.ts b/tests/integration/dev-proxy-eras.test.ts index 1896cef8..0debdd92 100644 --- a/tests/integration/dev-proxy-eras.test.ts +++ b/tests/integration/dev-proxy-eras.test.ts @@ -73,7 +73,12 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { const status = await client.callTool({ name: 'dev_server_status', arguments: {} }); const text = (status.content as Array<{ text: string }>)[0].text; - expect(JSON.parse(text)).toMatchObject({ state: 'stopped', protocolEra: null }); + // "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(); @@ -82,9 +87,9 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { // 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: {} }); - expect(JSON.parse((after.content as Array<{ text: string }>)[0].text)).toMatchObject({ - state: 'running', protocolEra: 'legacy', protocolVersion: '2025-11-25', - }); + 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('a 2025 client keeps working unchanged', async () => { @@ -95,6 +100,28 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { 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[] = []; diff --git a/tests/unit/dev-proxy/backend-env.test.ts b/tests/unit/dev-proxy/backend-env.test.ts index 1ed1d79f..8794795c 100644 --- a/tests/unit/dev-proxy/backend-env.test.ts +++ b/tests/unit/dev-proxy/backend-env.test.ts @@ -4,7 +4,6 @@ import { resolveBackendPort, resolveBackendTransport, updateBackendEnvOverrides, - withBackendDefaults, } from '../../../tools/dev-proxy/backend-env.mjs'; describe('dev-proxy backend port (issue #689)', () => { @@ -82,18 +81,3 @@ describe('dev-proxy backend environment overrides', () => { }); }); -describe('dev-proxy backend defaults (issue #1025, step 2)', () => { - // One client, and the proxy's own lifetime is the backend's: the per-debug-session - // idle policy (30 min on http) would close a developer's paused session under them. - it('turns the backend idle policy off unless the environment already says otherwise', () => { - expect(withBackendDefaults({ PATH: '/bin' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '0', PATH: '/bin' }); - expect(withBackendDefaults({ MCP_DEBUG_SESSION_IDLE_MS: '60000' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '60000' }); - // An explicitly empty value is the user's too: the server treats it as unset, not ours to rewrite. - expect(withBackendDefaults({ MCP_DEBUG_SESSION_IDLE_MS: '' })).toEqual({ MCP_DEBUG_SESSION_IDLE_MS: '' }); - }); - - it('lets a tool-call override win over the default', () => { - const env = buildBackendEnvironment(withBackendDefaults({}), { MCP_DEBUG_SESSION_IDLE_MS: '5000' }); - expect(env.MCP_DEBUG_SESSION_IDLE_MS).toBe('5000'); - }); -}); diff --git a/tests/unit/dev-proxy/backend-subscriptions.test.ts b/tests/unit/dev-proxy/backend-subscriptions.test.ts index 3a846769..2f1dc24a 100644 --- a/tests/unit/dev-proxy/backend-subscriptions.test.ts +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -63,6 +63,94 @@ describe('dev-proxy backend subscription relay', () => { }); }); + // 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('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'); @@ -212,7 +300,7 @@ describe('dev-proxy backend subscription relay', () => { 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 new Error('Unknown resource: ' + uri); + if (uri.endsWith('proxy-log')) throw Object.assign(new Error('Unknown resource: ' + uri), { code: -32602 }); return {}; }); const log = vi.fn(); 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/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 69b95a34..277b4db3 100644 --- a/tools/dev-proxy/README.md +++ b/tools/dev-proxy/README.md @@ -16,9 +16,12 @@ The proxy speaks both MCP protocol eras to its client (issue #1025): it is serve 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 an `http` backend the proxy negotiates the same way and lands on 2026-07-28 when that -backend's dist serves it (an older dist answers the 2025 era; `dev_server_status` says which -under `protocolEra`/`protocolVersion`); a stdio backend stays on the 2025 era. Resource +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 @@ -26,7 +29,10 @@ loses them. A 2025 client names what it wants through `resources/subscribe`. A 2 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. That router also drops a `tools/list_changed` a 2026 client did not opt into +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. @@ -108,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 @@ -183,11 +189,6 @@ preserves the current set, and passing `{}` clears it. Overrides are merged into backend spawn but are not passed to the build command. Proxy-controlled values required for clean shutdown take precedence. -One default sits under everything else: the backend runs with `MCP_DEBUG_SESSION_IDLE_MS=0` -(the per-debug-session idle policy off) unless your environment or an override sets the variable -— one client, and the proxy's lifetime is the backend's, so a 30-minute window would close a -paused session under you. - Each request's overrides are applied when its queued restart begins, after any build succeeds. An omitted `env` inherits the preceding applied settings. Concurrent requests cannot overwrite one another's settings, and each response reports the backend generation that its own restart started. diff --git a/tools/dev-proxy/backend-env.d.mts b/tools/dev-proxy/backend-env.d.mts index 26b325e6..069b7399 100644 --- a/tools/dev-proxy/backend-env.d.mts +++ b/tools/dev-proxy/backend-env.d.mts @@ -40,10 +40,3 @@ export function buildBackendEnvironment( forced?: Record ): Record; -/** - * The proxy's backend defaults under the inherited environment, so the user's - * own setting wins: `MCP_DEBUG_SESSION_IDLE_MS=0` unless set (issue #1025). - */ -export function withBackendDefaults( - inherited: Record -): Record; diff --git a/tools/dev-proxy/backend-env.mjs b/tools/dev-proxy/backend-env.mjs index 76008864..90bc82d9 100644 --- a/tools/dev-proxy/backend-env.mjs +++ b/tools/dev-proxy/backend-env.mjs @@ -81,16 +81,3 @@ export function buildBackendEnvironment(inherited, overrides, forced = {}) { return { ...inherited, ...overrides, ...forced }; } -/** - * The proxy's defaults for its backend, under the inherited environment so the - * user's own setting always wins (issue #1025, step 2): one client, and the - * proxy's lifetime is the backend's, so the per-debug-session idle policy - * (30 min on http) is off unless the environment says otherwise — it would - * close a developer's paused session under them. - * - * @param {Record} inherited - * @returns {Record} - */ -export function withBackendDefaults(inherited) { - return { MCP_DEBUG_SESSION_IDLE_MS: '0', ...inherited }; -} diff --git a/tools/dev-proxy/backend-subscriptions.mjs b/tools/dev-proxy/backend-subscriptions.mjs index 5ab67b39..7b5e54fe 100644 --- a/tools/dev-proxy/backend-subscriptions.mjs +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -23,6 +23,15 @@ * 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 a JSON-RPC error answer counts as a refusal; + * 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 @@ -47,6 +56,8 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { /** 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} */ @@ -90,15 +101,34 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { 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); + } }; - /** One tracked URI the backend refuses is logged, not fatal; a named one is the client's answer. */ + /** The backend answered no (a JSON-RPC error), as opposed to the connection failing. */ + const isRefusal = (err) => typeof err?.code === 'number'; + + /** + * 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. 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 (uris.has(uri)) throw err; - log(`Backend refused a tracked subscription for ${uri}: ${err?.message ?? err}`); + if (!isRefusal(err)) throw err; + 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}`); + } } }; @@ -127,7 +157,10 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { */ trackAll(enabled) { trackingAll = Boolean(enabled); - if (!trackingAll) known = []; + if (!trackingAll) { + known = []; + refused.clear(); + } }, /** @param {string} uri */ diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index a565025e..59c97522 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -48,12 +48,14 @@ import { resolveBackendPort, resolveBackendTransport, updateBackendEnvOverrides, - withBackendDefaults, } from './backend-env.mjs'; import { LifecycleQueue, queueGenerationCleanup } from './lifecycle-queue.mjs'; import { runBuild } from './build-runner.mjs'; import { ensurePortFree, forceKillPid } from './process-cleanup.mjs'; -import { backendCallOptions } from './tool-error.mjs'; +import { + backendCallOptions, + initialStartFailureLines, +} from './tool-error.mjs'; import { createFront, ObservedStdioServerTransport } from './front-server.mjs'; import { createSubscriptionRelay } from './backend-subscriptions.mjs'; @@ -154,6 +156,18 @@ function parseCommandString(cmdStr) { // BackendManager — manages the mcp-debugger child process lifecycle // --------------------------------------------------------------------------- +/** + * The stdio transport to the backend. It adds nothing; being a subclass is the + * 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 (issue #1025, step 2). + */ +class BackendStdioTransport extends StdioClientTransport {} + class BackendManager { constructor() { /** @type {'stopped' | 'starting' | 'running' | 'restarting'} */ @@ -216,7 +230,7 @@ class BackendManager { _buildBackendEnv({ forceStdinClose = false } = {}) { return buildBackendEnvironment( - withBackendDefaults(process.env), + process.env, this.backendEnvOverrides, forceStdinClose ? { MCP_EXIT_ON_STDIN_CLOSE: '1' } : {} ); @@ -246,8 +260,7 @@ class BackendManager { return true; }, (err) => { - log(`Initial backend start failed: ${err.message}`); - log('Dev tools are still available — use dev_restart_debugger to retry'); + for (const line of initialStartFailureLines(err, this.shutdownController.signal.aborted)) log(line); return false; } ); @@ -523,6 +536,11 @@ class BackendManager { 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.initialStart + ? {} + : { 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).' }), }; } @@ -574,11 +592,12 @@ class BackendManager { async _connectClient(command, args) { const closeState = { expected: false }; this.transportCloseState = closeState; - // The http backend speaks both eras (issue #1025): negotiate. A stdio backend - // stays legacy — `auto` over stdio would probe on a sibling of the backend. + // The backend speaks both eras on both transports (issue #1025): negotiate. + // Over stdio that takes BackendStdioTransport below, 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: this.backendTransport === 'http' ? 'auto' : 'legacy' } } + { versionNegotiation: { mode: 'auto' } } ); // Relay backend resource notifications (resources/updated, list_changed) @@ -595,7 +614,7 @@ class BackendManager { if (this.backendTransport === 'stdio') { // Stdio mode: StdioClientTransport spawns the child - const transport = new StdioClientTransport({ + const transport = new BackendStdioTransport({ command, args, cwd: PROJECT_ROOT, 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', + ]; +} From 627a30954d377b719b72e400f8d53cec46e2f817 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 10 Nov 2026 13:40:28 -0500 Subject: [PATCH 4/6] fix(dev-proxy): end a backend start that is still negotiating when the proxy shuts down; keep a subscription the backend merely failed on (#1025) Found by CI on Linux and by the review of the second-look pass. The previous commit made the stdio backend negotiate its era in place, through a subclassed transport. Dispatched CI then failed on Linux in the rebuild suite: "shuts down with work queued/starting without spawning queued replacements" found the backend process still alive after the proxy had gone. Measured on the SDK client 2.4.0: negotiating in place, the SDK does not watch the signal given to connect(). The connect stays pending past the abort and the child keeps running, where with its base transport the SDK closes the transport itself. So a shutdown that found a start still negotiating never cancelled it, the start's cleanup never ran, and the proxy exited without its backend. (Not a missing pid: transport.pid is set right after connect() in both modes.) The transport now takes the proxy's shutdown signal and closes itself when it aborts, which ends the child and fails the pending negotiation at once, so the start's own failure path cleans up. It lets go of the signal when it closes or when its backend has gone by itself, so restarts do not pile listeners onto a signal that lives as long as the proxy. It moves to its own module so a unit test can hold it on every platform: without the close, connect is still pending four seconds after the abort and the child is alive. From the review: - The relay treated any JSON-RPC error as "that resource is gone". Only InvalidParams (or the older resource-not-found code) says that; an internal error from a live backend now leaves the subscription in place, to be tried again, instead of silently dropping it. - dev_server_status showed its "not started yet" note beside state "running" when the first handled request was a dev restart; the note now follows whether any start has begun. Co-Authored-By: Claude Fable 5.2 --- tests/integration/dev-proxy-eras.test.ts | 15 +++ .../dev-proxy/backend-stdio-transport.test.ts | 93 +++++++++++++++++++ .../dev-proxy/backend-subscriptions.test.ts | 33 +++++++ tools/dev-proxy/backend-stdio-transport.d.mts | 13 +++ tools/dev-proxy/backend-stdio-transport.mjs | 63 +++++++++++++ tools/dev-proxy/backend-subscriptions.mjs | 25 +++-- tools/dev-proxy/dev-proxy.mjs | 33 +++---- 7 files changed, 250 insertions(+), 25 deletions(-) create mode 100644 tests/unit/dev-proxy/backend-stdio-transport.test.ts create mode 100644 tools/dev-proxy/backend-stdio-transport.d.mts create mode 100644 tools/dev-proxy/backend-stdio-transport.mjs diff --git a/tests/integration/dev-proxy-eras.test.ts b/tests/integration/dev-proxy-eras.test.ts index 0debdd92..466d5d97 100644 --- a/tests/integration/dev-proxy-eras.test.ts +++ b/tests/integration/dev-proxy-eras.test.ts @@ -92,6 +92,21 @@ describe('dev-proxy protocol eras (issue #1025, step 2)', () => { 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'); 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..1958e11a --- /dev/null +++ b/tests/unit/dev-proxy/backend-stdio-transport.test.ts @@ -0,0 +1,93 @@ +/** + * 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: negotiating in place, the SDK does not watch + * the signal given to connect() (measured, client 2.4.0), 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 for its base transport. + */ +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 index 2f1dc24a..2e9aed03 100644 --- a/tests/unit/dev-proxy/backend-subscriptions.test.ts +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -129,6 +129,39 @@ describe('dev-proxy backend subscription relay', () => { 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 }); 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..1e766d40 --- /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 (the SDK's in-place negotiation does not + * watch connect()'s signal). + */ +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..7a26d3f3 --- /dev/null +++ b/tools/dev-proxy/backend-stdio-transport.mjs @@ -0,0 +1,63 @@ +/** + * 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. Negotiating in place, the SDK does not + * watch the signal given to connect() (measured, client 2.4.0): a start still + * negotiating when the proxy shuts down hangs until the probe gives up, and the + * backend it spawned outlives the proxy. With its base transport the SDK closes + * the transport itself on that abort; this class does the same, 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.mjs b/tools/dev-proxy/backend-subscriptions.mjs index 7b5e54fe..d870585b 100644 --- a/tools/dev-proxy/backend-subscriptions.mjs +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -28,9 +28,11 @@ * 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 a JSON-RPC error answer counts as a refusal; - * a failed connection still surfaces. (A 2026 backend's listen takes any URI, - * so a dead one named there simply never fires.) + * 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 @@ -107,13 +109,16 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { } }; - /** The backend answered no (a JSON-RPC error), as opposed to the connection failing. */ - const isRefusal = (err) => typeof err?.code === 'number'; + /** 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. A failed connection is thrown. + * 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); @@ -121,7 +126,13 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { try { await client.subscribeResource({ uri }); } catch (err) { - if (!isRefusal(err)) throw 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})`); diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 59c97522..52bf6663 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -21,7 +21,6 @@ * or restart to settle (default: 15000) */ -import { StdioClientTransport } from '@modelcontextprotocol/client/stdio'; import { Client, StreamableHTTPClientTransport } from '@modelcontextprotocol/client'; import { serveStdio } from '@modelcontextprotocol/server/stdio'; import { spawn } from 'child_process'; @@ -49,6 +48,7 @@ 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'; @@ -156,18 +156,6 @@ function parseCommandString(cmdStr) { // BackendManager — manages the mcp-debugger child process lifecycle // --------------------------------------------------------------------------- -/** - * The stdio transport to the backend. It adds nothing; being a subclass is the - * 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 (issue #1025, step 2). - */ -class BackendStdioTransport extends StdioClientTransport {} - class BackendManager { constructor() { /** @type {'stopped' | 'starting' | 'running' | 'restarting'} */ @@ -176,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; @@ -201,6 +189,8 @@ class BackendManager { 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(). */ @@ -274,6 +264,7 @@ class BackendManager { return; } + this.everStarted = true; this.state = 'starting'; this.expectedChildExit = false; const { command, args } = this._computeBackendCommand(); @@ -538,7 +529,7 @@ class BackendManager { 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.initialStart + ...(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).' }), }; @@ -593,8 +584,9 @@ class BackendManager { const closeState = { expected: false }; this.transportCloseState = closeState; // The backend speaks both eras on both transports (issue #1025): negotiate. - // Over stdio that takes BackendStdioTransport below, so the probe runs on - // the backend's own pipe rather than on a second backend process. + // 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' } } @@ -614,13 +606,16 @@ class BackendManager { if (this.backendTransport === 'stdio') { // Stdio mode: StdioClientTransport spawns the child + // The shutdown signal goes to the transport as well as to connect(): the + // in-place 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; @@ -634,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. From 9132a2e0a45f9d0d606104a730e671a0f201c667 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 10 Nov 2026 14:16:11 -0500 Subject: [PATCH 5/6] docs(dev-proxy): the ignored abort is the SDK's auto negotiation, on either path, not the in-place probe as such (#1025) Comments only. The previous commit said the SDK ignores connect()'s signal when it negotiates in place and closes the transport itself for its base class. Measured with all four combinations on client 2.4.0, the variable is the mode: in legacy mode an abort rejects the connect and the transport is closed, base class or subclass; in auto mode the connect stays pending and the child keeps running, on the sibling path and in place alike. The fix is unchanged and right either way; the comments now name the cause correctly. Co-Authored-By: Claude Fable 5.2 --- .../dev-proxy/backend-stdio-transport.test.ts | 15 +++++++++------ tools/dev-proxy/backend-stdio-transport.d.mts | 4 ++-- tools/dev-proxy/backend-stdio-transport.mjs | 18 +++++++++++------- tools/dev-proxy/dev-proxy.mjs | 2 +- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/unit/dev-proxy/backend-stdio-transport.test.ts b/tests/unit/dev-proxy/backend-stdio-transport.test.ts index 1958e11a..ebfcbdf4 100644 --- a/tests/unit/dev-proxy/backend-stdio-transport.test.ts +++ b/tests/unit/dev-proxy/backend-stdio-transport.test.ts @@ -3,12 +3,15 @@ * * 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: negotiating in place, the SDK does not watch - * the signal given to connect() (measured, client 2.4.0), 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 for its base transport. + * 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'; diff --git a/tools/dev-proxy/backend-stdio-transport.d.mts b/tools/dev-proxy/backend-stdio-transport.d.mts index 1e766d40..aefa31b1 100644 --- a/tools/dev-proxy/backend-stdio-transport.d.mts +++ b/tools/dev-proxy/backend-stdio-transport.d.mts @@ -3,8 +3,8 @@ import { StdioClientTransport, type StdioServerParameters } from '@modelcontextp /** * 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 (the SDK's in-place negotiation does not - * watch connect()'s signal). + * 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 }); diff --git a/tools/dev-proxy/backend-stdio-transport.mjs b/tools/dev-proxy/backend-stdio-transport.mjs index 7a26d3f3..d1c7aca3 100644 --- a/tools/dev-proxy/backend-stdio-transport.mjs +++ b/tools/dev-proxy/backend-stdio-transport.mjs @@ -9,13 +9,17 @@ * 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. Negotiating in place, the SDK does not - * watch the signal given to connect() (measured, client 2.4.0): a start still - * negotiating when the proxy shuts down hangs until the probe gives up, and the - * backend it spawned outlives the proxy. With its base transport the SDK closes - * the transport itself on that abort; this class does the same, which ends the - * child and fails the pending negotiation at once, so the start's own failure - * path can clean up. + * 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). diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 52bf6663..a6ea7ea7 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -607,7 +607,7 @@ class BackendManager { if (this.backendTransport === 'stdio') { // Stdio mode: StdioClientTransport spawns the child // The shutdown signal goes to the transport as well as to connect(): the - // in-place negotiation does not watch connect()'s, so without this a start + // 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, From 0dc64d3fdeee77c77697d4f7a23d8cc5f66994c6 Mon Sep 17 00:00:00 2001 From: debugmcpdev Date: Tue, 10 Nov 2026 21:22:37 -0500 Subject: [PATCH 6/6] fix(dev-proxy): serialize opposing subscription requests Check subscription state inside the operation queue so rapid subscribe/unsubscribe requests preserve arrival order. Add regressions for both orders across both protocol eras. --- .../dev-proxy/backend-subscriptions.test.ts | 41 +++++++++++++++++++ tools/dev-proxy/backend-subscriptions.mjs | 2 - 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/unit/dev-proxy/backend-subscriptions.test.ts b/tests/unit/dev-proxy/backend-subscriptions.test.ts index 2e9aed03..dea23cf6 100644 --- a/tests/unit/dev-proxy/backend-subscriptions.test.ts +++ b/tests/unit/dev-proxy/backend-subscriptions.test.ts @@ -269,6 +269,47 @@ describe('dev-proxy backend subscription relay', () => { }); 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> = []; diff --git a/tools/dev-proxy/backend-subscriptions.mjs b/tools/dev-proxy/backend-subscriptions.mjs index d870585b..a8d9f82a 100644 --- a/tools/dev-proxy/backend-subscriptions.mjs +++ b/tools/dev-proxy/backend-subscriptions.mjs @@ -176,7 +176,6 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { /** @param {string} uri */ async subscribe(uri) { - if (uris.has(uri)) return; return serialize(async () => { if (uris.has(uri)) return; const client = getClient(); @@ -202,7 +201,6 @@ export function createSubscriptionRelay({ getClient, log = () => {} }) { /** @param {string} uri */ async unsubscribe(uri) { - if (!uris.has(uri)) return; return serialize(async () => { if (!uris.delete(uri)) return; const client = getClient();