From b554ab399bd49859bd9a0aaa27ab1bb802318b5c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 28 Mar 2026 21:04:56 +0000 Subject: [PATCH] fix: dedupe keep-alive daemon restarts (#125) (thanks @zm2231) --- CHANGELOG.md | 1 + README.md | 10 +-- src/cli.ts | 27 ++++-- src/index.ts | 3 +- src/runtime.ts | 47 +---------- tests/cli-managed-runtime.test.ts | 43 ---------- tests/runtime-managed.test.ts | 133 ------------------------------ 7 files changed, 28 insertions(+), 236 deletions(-) delete mode 100644 tests/cli-managed-runtime.test.ts delete mode 100644 tests/runtime-managed.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d54e7f..76a6d19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### CLI +- Deduplicate concurrent keep-alive daemon restarts per server so repeated fatal errors only force-close the cached daemon transport once before retrying. (PR #125, thanks @zm2231) - Keep `mcporter call --output json` parseable by emitting valid JSON even when the command falls back to raw output. (PR #128, thanks @armanddp) - Ignore static `Authorization` headers once OAuth is active so imported editor configs cannot override fresh OAuth tokens. (PR #123, thanks @ahonn) - Preserve full JSON/error payloads when `data` is just one field instead of collapsing the response to `data` alone. (PR #106, thanks @AielloChan) diff --git a/README.md b/README.md index 1fa2dc0..be82bb6 100644 --- a/README.md +++ b/README.md @@ -236,9 +236,9 @@ console.log(result); // raw MCP envelope ## Compose Automations with the Runtime ```ts -import { createManagedRuntime } from "mcporter"; +import { createRuntime } from "mcporter"; -const runtime = await createManagedRuntime(); +const runtime = await createRuntime(); const tools = await runtime.listTools("context7"); const result = await runtime.callTool("context7", "resolve-library-id", { @@ -249,16 +249,16 @@ console.log(result); // prints JSON/text automatically because the CLI pretty-pr await runtime.close(); // shuts down transports and OAuth sessions ``` -Reach for `createManagedRuntime()` when you want the same keep-alive daemon behavior as the CLI, plus connection pooling, repeated calls, or advanced options such as explicit timeouts and log streaming. Use `createRuntime()` when you explicitly want direct runtime connections without daemon lifecycle management. Both runtimes reuse transports, refresh OAuth tokens, and only tear everything down when you call `runtime.close()`. +Reach for `createRuntime()` when you need connection pooling, repeated calls, or advanced options such as explicit timeouts and log streaming. The runtime reuses transports, refreshes OAuth tokens, and only tears everything down when you call `runtime.close()`. ## Compose Tools in Code The runtime API is built for agents and scripts, not just humans at a terminal. ```ts -import { createManagedRuntime, createServerProxy } from "mcporter"; +import { createRuntime, createServerProxy } from "mcporter"; -const runtime = await createManagedRuntime(); +const runtime = await createRuntime(); const chrome = createServerProxy(runtime, "chrome-devtools"); const linear = createServerProxy(runtime, "linear"); diff --git a/src/cli.ts b/src/cli.ts index a64c743..ae623fa 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -14,7 +14,10 @@ import { handleList, printListHelp } from './cli/list-command.js'; import { logError, logInfo } from './cli/logger-context.js'; import { DEBUG_HANG, dumpActiveHandles, terminateChildProcesses } from './cli/runtime-debug.js'; import { resolveConfigPath } from './config.js'; -import { createManagedRuntime, createRuntime } from './runtime.js'; +import { DaemonClient } from './daemon/client.js'; +import { createKeepAliveRuntime } from './daemon/runtime-wrapper.js'; +import { isKeepAliveServer } from './lifecycle.js'; +import { createRuntime } from './runtime.js'; export { handleAuth, printAuthHelp } from './cli/auth-command.js'; export { parseCallArguments } from './cli/call-arguments.js'; @@ -107,12 +110,22 @@ export async function runCli(argv: string[]): Promise { return; } - const runtime = await createManagedRuntime({ - ...runtimeOptions, - configPath: configPathResolved, - configExplicit: configResolution.explicit, - rootDir: rootOverride, - }); + const baseRuntime = await createRuntime(runtimeOptionsWithPath); + const keepAliveServers = new Set( + baseRuntime + .getDefinitions() + .filter(isKeepAliveServer) + .map((entry) => entry.name) + ); + const daemonClient = + keepAliveServers.size > 0 + ? new DaemonClient({ + configPath: configResolution.path, + configExplicit: configResolution.explicit, + rootDir: rootOverride, + }) + : null; + const runtime = createKeepAliveRuntime(baseRuntime, { daemonClient, keepAliveServers }); const inference = inferCommandRouting(command, args, runtime.getDefinitions()); if (inference.kind === 'abort') { diff --git a/src/index.ts b/src/index.ts index 2b541d2..622361b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,11 +5,10 @@ export { createCallResult, describeConnectionIssue, wrapCallResult } from './res export type { CallOptions, ListToolsOptions, - ManagedRuntimeOptions, Runtime, RuntimeLogger, ServerToolInfo, } from './runtime.js'; -export { callOnce, createManagedRuntime, createRuntime } from './runtime.js'; +export { callOnce, createRuntime } from './runtime.js'; export type { ServerProxyOptions } from './server-proxy.js'; export { createServerProxy } from './server-proxy.js'; diff --git a/src/runtime.ts b/src/runtime.ts index 5f44e8a..d7a49df 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -1,10 +1,7 @@ import { createRequire } from 'node:module'; import type { CallToolRequest, ListResourcesRequest } from '@modelcontextprotocol/sdk/types.js'; -import { loadServerDefinitions, resolveConfigPath, type ServerDefinition } from './config.js'; -import { DaemonClient } from './daemon/client.js'; -import { createKeepAliveRuntime } from './daemon/runtime-wrapper.js'; -import { isKeepAliveServer } from './lifecycle.js'; +import { loadServerDefinitions, type ServerDefinition } from './config.js'; import { createPrefixedConsoleLogger, type Logger, type LogLevel, resolveLogLevelFromEnv } from './logging.js'; import { closeTransportAndWait } from './runtime-process-utils.js'; import './sdk-patches.js'; @@ -37,10 +34,6 @@ export interface RuntimeOptions { readonly oauthTimeoutMs?: number; } -export interface ManagedRuntimeOptions extends RuntimeOptions { - readonly configExplicit?: boolean; -} - export type RuntimeLogger = Logger; export interface CallOptions { @@ -93,44 +86,6 @@ export async function createRuntime(options: RuntimeOptions = {}): Promise { - const rootDir = options.rootDir ?? process.cwd(); - const configResolution = resolveConfigPath(options.configPath, rootDir); - const configPath = options.configPath ?? configResolution.path; - const baseRuntime = await createRuntime({ - ...options, - configPath: options.servers - ? options.configPath - : (options.configExplicit ?? configResolution.explicit) - ? configPath - : undefined, - rootDir, - }); - - if (options.servers) { - return baseRuntime; - } - - const keepAliveServers = new Set( - baseRuntime - .getDefinitions() - .filter(isKeepAliveServer) - .map((entry) => entry.name) - ); - - if (keepAliveServers.size === 0) { - return baseRuntime; - } - - const daemonClient = new DaemonClient({ - configPath, - configExplicit: options.configExplicit ?? configResolution.explicit, - rootDir, - }); - return createKeepAliveRuntime(baseRuntime, { daemonClient, keepAliveServers }); -} - // callOnce connects to a server, invokes a single tool, and disposes the connection immediately. export async function callOnce(params: { server: string; diff --git a/tests/cli-managed-runtime.test.ts b/tests/cli-managed-runtime.test.ts deleted file mode 100644 index 6db8681..0000000 --- a/tests/cli-managed-runtime.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; - -process.env.MCPORTER_DISABLE_AUTORUN = '1'; -process.env.MCPORTER_NO_FORCE_EXIT = '1'; - -const runtime = { - getDefinitions: vi.fn(() => []), - close: vi.fn(async () => undefined), -}; - -const createManagedRuntimeMock = vi.fn(async () => runtime); -const createRuntimeMock = vi.fn(async () => runtime); -const handleListMock = vi.fn(async () => undefined); - -vi.mock('../src/runtime.js', async () => { - const actual = await vi.importActual('../src/runtime.js'); - return { - ...actual, - createManagedRuntime: createManagedRuntimeMock, - createRuntime: createRuntimeMock, - }; -}); - -vi.mock('../src/cli/list-command.js', async () => { - const actual = await vi.importActual('../src/cli/list-command.js'); - return { - ...actual, - handleList: handleListMock, - }; -}); - -describe('mcporter CLI managed runtime wiring', () => { - it('uses createManagedRuntime for normal CLI commands', async () => { - const { runCli } = await import('../src/cli.js'); - - await runCli(['list']); - - expect(createManagedRuntimeMock).toHaveBeenCalledTimes(1); - expect(createRuntimeMock).not.toHaveBeenCalled(); - expect(handleListMock).toHaveBeenCalledWith(runtime, []); - expect(runtime.close).toHaveBeenCalledTimes(1); - }); -}); diff --git a/tests/runtime-managed.test.ts b/tests/runtime-managed.test.ts deleted file mode 100644 index 26b040b..0000000 --- a/tests/runtime-managed.test.ts +++ /dev/null @@ -1,133 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Runtime } from '../src/runtime.js'; - -const daemonClientFactory = vi.fn(); -const createKeepAliveRuntimeMock = vi.fn(); -const isKeepAliveServerMock = vi.fn( - (definition: { lifecycle?: { mode?: string } }) => definition.lifecycle?.mode === 'keep-alive' -); -const resolveConfigPathMock = vi.fn((configPath?: string, rootDir?: string) => ({ - path: configPath ?? `${rootDir ?? process.cwd()}/mcporter.json`, - explicit: Boolean(configPath), -})); - -const loadServerDefinitionsMock = vi.fn(); - -vi.mock('../src/config.js', () => ({ - loadServerDefinitions: loadServerDefinitionsMock, - resolveConfigPath: resolveConfigPathMock, -})); - -vi.mock('../src/daemon/client.js', () => ({ - DaemonClient: class { - constructor(options: unknown) { - daemonClientFactory(options); - } - }, -})); - -vi.mock('../src/daemon/runtime-wrapper.js', () => ({ - createKeepAliveRuntime: createKeepAliveRuntimeMock, -})); - -vi.mock('../src/lifecycle.js', () => ({ - isKeepAliveServer: isKeepAliveServerMock, -})); - -vi.mock('../src/runtime/transport.js', () => ({ - createClientContext: vi.fn(), -})); - -vi.mock('../src/sdk-patches.js', () => ({})); - -async function loadRuntimeModule() { - return await import('../src/runtime.js'); -} - -describe('createManagedRuntime', () => { - beforeEach(() => { - loadServerDefinitionsMock.mockReset(); - daemonClientFactory.mockReset(); - createKeepAliveRuntimeMock.mockReset(); - isKeepAliveServerMock.mockClear(); - resolveConfigPathMock.mockClear(); - }); - - it('wraps config-backed keep-alive runtimes with the daemon client', async () => { - loadServerDefinitionsMock.mockResolvedValue([ - { - name: 'chrome-devtools', - command: { kind: 'stdio', command: 'node', args: [] }, - lifecycle: { mode: 'keep-alive' }, - }, - { - name: 'context7', - command: { kind: 'http', url: new URL('https://example.com') }, - }, - ]); - const wrappedRuntime = { wrapped: true } as unknown as Runtime; - createKeepAliveRuntimeMock.mockReturnValue(wrappedRuntime); - - const { createManagedRuntime } = await loadRuntimeModule(); - const runtime = await createManagedRuntime({ - configPath: '/tmp/custom.json', - configExplicit: true, - rootDir: '/repo', - }); - - expect(resolveConfigPathMock).toHaveBeenCalledWith('/tmp/custom.json', '/repo'); - expect(loadServerDefinitionsMock).toHaveBeenCalledWith({ - configPath: '/tmp/custom.json', - rootDir: '/repo', - }); - expect(daemonClientFactory).toHaveBeenCalledWith({ - configPath: '/tmp/custom.json', - configExplicit: true, - rootDir: '/repo', - }); - expect(createKeepAliveRuntimeMock).toHaveBeenCalledWith( - expect.objectContaining({ - getDefinitions: expect.any(Function), - }), - expect.objectContaining({ - keepAliveServers: new Set(['chrome-devtools']), - }) - ); - expect(runtime).toBe(wrappedRuntime); - }); - - it('returns a plain runtime when explicit servers are provided', async () => { - const { createManagedRuntime } = await loadRuntimeModule(); - const runtime = await createManagedRuntime({ - servers: [ - { - name: 'chrome-devtools', - command: { kind: 'stdio', command: 'node', args: [] }, - lifecycle: { mode: 'keep-alive' }, - }, - ] as never, - rootDir: '/repo', - }); - - expect(loadServerDefinitionsMock).not.toHaveBeenCalled(); - expect(daemonClientFactory).not.toHaveBeenCalled(); - expect(createKeepAliveRuntimeMock).not.toHaveBeenCalled(); - expect(runtime.listServers()).toEqual(['chrome-devtools']); - }); - - it('returns a plain runtime when there are no keep-alive servers', async () => { - loadServerDefinitionsMock.mockResolvedValue([ - { - name: 'context7', - command: { kind: 'http', url: new URL('https://example.com') }, - }, - ]); - - const { createManagedRuntime } = await loadRuntimeModule(); - const runtime = await createManagedRuntime({ rootDir: '/repo' }); - - expect(daemonClientFactory).not.toHaveBeenCalled(); - expect(createKeepAliveRuntimeMock).not.toHaveBeenCalled(); - expect(runtime.listServers()).toEqual(['context7']); - }); -});