From 6012708bf3f558ef8bd160ec3cecb6c0c631badc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 9 May 2026 12:04:25 +0100 Subject: [PATCH] perf: keep list non-interactive --- CHANGELOG.md | 2 ++ src/cli/list-command.ts | 21 ++++++++++++++++++--- src/cli/tool-cache.ts | 12 +++++++++--- src/runtime/oauth.ts | 19 ++++++++++++++++++- tests/cli-list-classification.test.ts | 26 +++++++++++++++++++++----- tests/runtime-oauth-connect.test.ts | 9 +++++++++ tests/tool-cache.test.ts | 15 +++++++++++++++ 7 files changed, 92 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c27a68f..60d81ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ - Skip the redundant daemon `status` preflight for warm keep-alive access, cutting one socket round-trip from each routed list/call/resource request while preserving stale-config and dead-daemon recovery. - Route explicit default keep-alive calls like `chrome-devtools.list_pages` through a daemon-only fast path, avoiding full runtime startup on warm calls. - Further reduce warm keep-alive call startup by avoiding runtime/config schema imports on CLI boot and using a narrower daemon call path for simple explicit calls. +- Keep single-server `mcporter list` non-interactive by reusing cached OAuth without launching new auth flows, and clamp oversized OAuth startup errors so HTML responses do not flood stdout/stderr. +- Label non-timeout `mcporter list ` failures as unavailable instead of timed out. ### Config diff --git a/src/cli/list-command.ts b/src/cli/list-command.ts index e9aa79d..9a89da9 100644 --- a/src/cli/list-command.ts +++ b/src/cli/list-command.ts @@ -207,7 +207,14 @@ export async function handleList( if (flags.format === 'json') { try { const metadataEntries = filterToolMetadata( - await withTimeout(loadToolMetadata(runtime, target, { includeSchema: true }), timeoutMs), + await withTimeout( + loadToolMetadata(runtime, target, { + includeSchema: true, + autoAuthorize: false, + allowCachedAuth: true, + }), + timeoutMs + ), requestedTool ); await persistPreparedEphemeralServer(runtime, prepared); @@ -263,7 +270,14 @@ export async function handleList( try { // Always request schemas so we can render CLI-style parameter hints without re-querying per tool. const metadataEntries = filterToolMetadata( - await withTimeout(loadToolMetadata(runtime, target, { includeSchema: true }), timeoutMs), + await withTimeout( + loadToolMetadata(runtime, target, { + includeSchema: true, + autoAuthorize: false, + allowCachedAuth: true, + }), + timeoutMs + ), requestedTool ); await persistPreparedEphemeralServer(runtime, prepared); @@ -333,7 +347,8 @@ export async function handleList( const message = error instanceof Error ? error.message : 'Failed to load tool list.'; const authCommand = buildAuthCommandHint(definition); const advice = classifyListError(error, definition.name, timeoutMs, { authCommand }); - console.warn(` Tools: `); + const timedOut = message === 'Timeout' || /\btimed out\b/i.test(message); + console.warn(` Tools: ${timedOut ? `` : ''}`); console.warn(` Reason: ${message}`); if (advice.category === 'auth' && advice.authCommand) { console.warn(` Next: run '${advice.authCommand}' to finish authentication.`); diff --git a/src/cli/tool-cache.ts b/src/cli/tool-cache.ts index e749a45..6d12dbf 100644 --- a/src/cli/tool-cache.ts +++ b/src/cli/tool-cache.ts @@ -1,9 +1,10 @@ -import type { Runtime } from '../runtime.js'; +import type { ListToolsOptions, Runtime } from '../runtime.js'; import { buildToolMetadata, type ToolMetadata } from './generate/tools.js'; interface LoadToolMetadataOptions { includeSchema?: boolean; autoAuthorize?: boolean; + allowCachedAuth?: boolean; } const runtimeCache = new WeakMap>>(); @@ -11,7 +12,8 @@ const runtimeCache = new WeakMap>>( function cacheKey(serverName: string, options: LoadToolMetadataOptions): string { const includeSchema = options.includeSchema !== false; const autoAuthorize = options.autoAuthorize !== false; - return `${serverName}::schema:${includeSchema ? '1' : '0'}::auth:${autoAuthorize ? '1' : '0'}`; + const allowCachedAuth = options.allowCachedAuth === true; + return `${serverName}::schema:${includeSchema ? '1' : '0'}::auth:${autoAuthorize ? '1' : '0'}::cached-auth:${allowCachedAuth ? '1' : '0'}`; } export async function loadToolMetadata( @@ -31,8 +33,12 @@ export async function loadToolMetadata( } const includeSchema = options.includeSchema !== false; const autoAuthorize = options.autoAuthorize !== false; + const listOptions: ListToolsOptions = + options.allowCachedAuth === undefined + ? { includeSchema, autoAuthorize } + : { includeSchema, autoAuthorize, allowCachedAuth: options.allowCachedAuth }; const promise = runtime - .listTools(serverName, { includeSchema, autoAuthorize }) + .listTools(serverName, listOptions) .then((tools) => tools.map((tool) => buildToolMetadata(tool))) .catch((error) => { cache?.delete(key); diff --git a/src/runtime/oauth.ts b/src/runtime/oauth.ts index 495f0f9..f3567fc 100644 --- a/src/runtime/oauth.ts +++ b/src/runtime/oauth.ts @@ -7,6 +7,7 @@ import { isUnauthorizedError } from '../runtime-oauth-support.js'; export const DEFAULT_OAUTH_CODE_TIMEOUT_MS = 300_000; const OAUTH_FLOW_ERROR = Symbol('oauth-flow-error'); const POST_AUTH_CONNECT_ERROR = Symbol('post-auth-connect-error'); +const MAX_OAUTH_ERROR_DETAIL_LENGTH = 1_200; export interface OAuthCapableTransport extends Transport { close(): Promise; @@ -43,7 +44,8 @@ export class OAuthAuthorizationNotStartedError extends Error { public readonly serverName: string; constructor(serverName: string, cause?: unknown) { - const detail = cause instanceof Error && cause.message ? ` Last error: ${cause.message}` : ''; + const causeMessage = formatOAuthErrorDetail(cause); + const detail = causeMessage ? ` Last error: ${causeMessage}` : ''; super( `OAuth authorization for '${serverName}' did not produce an authorization URL; aborting instead of waiting for a browser callback.${detail}` ); @@ -52,6 +54,21 @@ export class OAuthAuthorizationNotStartedError extends Error { } } +function formatOAuthErrorDetail(cause: unknown): string { + if (!(cause instanceof Error) || !cause.message) { + return ''; + } + return truncateOAuthErrorDetail(cause.message); +} + +function truncateOAuthErrorDetail(message: string): string { + if (message.length <= MAX_OAUTH_ERROR_DETAIL_LENGTH) { + return message; + } + const truncated = message.length - MAX_OAUTH_ERROR_DETAIL_LENGTH; + return `${message.slice(0, MAX_OAUTH_ERROR_DETAIL_LENGTH)}... [truncated ${truncated} chars]`; +} + export function markOAuthFlowError(error: unknown): unknown { return markError(error, OAUTH_FLOW_ERROR); } diff --git a/tests/cli-list-classification.test.ts b/tests/cli-list-classification.test.ts index 8134fe9..bbd4bc9 100644 --- a/tests/cli-list-classification.test.ts +++ b/tests/cli-list-classification.test.ts @@ -113,6 +113,7 @@ describe('CLI list classification and routing', () => { (call[0]?.toString() ?? '').includes("Next: run 'mcporter auth https://mcp.supabase.com/mcp'") ); expect(hinted).toBe(true); + expect(warnSpy.mock.calls.map((call) => call.join(' '))).toContain(' Tools: '); warnSpy.mockRestore(); logSpy.mockRestore(); @@ -182,7 +183,10 @@ describe('CLI list classification and routing', () => { await handleList(runtime, ['https://mcp.vercel.com']); - expect(listTools).toHaveBeenCalledWith('vercel', expect.anything()); + expect(listTools).toHaveBeenCalledWith( + 'vercel', + expect.objectContaining({ includeSchema: true, autoAuthorize: false, allowCachedAuth: true }) + ); expect(registerDefinition).not.toHaveBeenCalled(); }); @@ -205,7 +209,10 @@ describe('CLI list classification and routing', () => { await handleList(runtime, ['https://www.shadcn.io/api/mcp.getComponents']); - expect(listTools).toHaveBeenCalledWith('shadcn', expect.anything()); + expect(listTools).toHaveBeenCalledWith( + 'shadcn', + expect.objectContaining({ includeSchema: true, autoAuthorize: false, allowCachedAuth: true }) + ); expect(registerDefinition).not.toHaveBeenCalled(); }); @@ -227,7 +234,10 @@ describe('CLI list classification and routing', () => { await handleList(runtime, ['shadcn.io/api/mcp.getComponents']); - expect(listTools).toHaveBeenCalledWith('shadcn', expect.anything()); + expect(listTools).toHaveBeenCalledWith( + 'shadcn', + expect.objectContaining({ includeSchema: true, autoAuthorize: false, allowCachedAuth: true }) + ); }); it('enables cached OAuth when listing all servers', async () => { @@ -279,7 +289,10 @@ describe('CLI list classification and routing', () => { expect(registerDefinition).toHaveBeenCalled(); expect(definitions.get('mcp-example-com-mcp')).toBeDefined(); - expect(listTools).toHaveBeenCalledWith('mcp-example-com-mcp', expect.objectContaining({ includeSchema: true })); + expect(listTools).toHaveBeenCalledWith( + 'mcp-example-com-mcp', + expect.objectContaining({ includeSchema: true, autoAuthorize: false, allowCachedAuth: true }) + ); logSpy.mockRestore(); }); @@ -305,7 +318,10 @@ describe('CLI list classification and routing', () => { await handleList(runtime, ['linera']); expect(getDefinition).toHaveBeenCalledTimes(2); - expect(listTools).toHaveBeenCalledWith('linear', expect.objectContaining({ includeSchema: true })); + expect(listTools).toHaveBeenCalledWith( + 'linear', + expect.objectContaining({ includeSchema: true, autoAuthorize: false, allowCachedAuth: true }) + ); const messages = logSpy.mock.calls.map((call) => call.join(' ')); expect(messages.some((line) => line.includes('Auto-corrected server name to linear'))).toBe(true); diff --git a/tests/runtime-oauth-connect.test.ts b/tests/runtime-oauth-connect.test.ts index 7fed07f..cc554f7 100644 --- a/tests/runtime-oauth-connect.test.ts +++ b/tests/runtime-oauth-connect.test.ts @@ -228,4 +228,13 @@ describe('connectWithAuth', () => { expect.any(OAuthAuthorizationNotStartedError) ); }); + + it('truncates oversized OAuth startup error details', () => { + const hugeHtml = `${'x'.repeat(5000)}`; + const error = new OAuthAuthorizationNotStartedError('shadcn', new Error(`HTTP 404 raw body: ${hugeHtml}`)); + + expect(error.message.length).toBeLessThan(1800); + expect(error.message).toContain('[truncated '); + expect(error.message).not.toContain(hugeHtml); + }); }); diff --git a/tests/tool-cache.test.ts b/tests/tool-cache.test.ts index c7389e7..b4c7e8a 100644 --- a/tests/tool-cache.test.ts +++ b/tests/tool-cache.test.ts @@ -39,4 +39,19 @@ describe('loadToolMetadata', () => { await loadToolMetadata(runtime, 'integration', { includeSchema: false }); expect(listTools).toHaveBeenCalledTimes(2); }); + + it('passes cached OAuth preference to the runtime', async () => { + const listTools = vi.fn(async () => [demoTool]); + const runtime = createRuntimeStub(listTools); + await loadToolMetadata(runtime, 'integration', { + includeSchema: true, + autoAuthorize: false, + allowCachedAuth: true, + }); + expect(listTools).toHaveBeenCalledWith('integration', { + includeSchema: true, + autoAuthorize: false, + allowCachedAuth: true, + }); + }); });