perf: keep list non-interactive
This commit is contained in:
parent
e6b451aee3
commit
6012708bf3
@ -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 <server>` failures as unavailable instead of timed out.
|
||||
|
||||
### Config
|
||||
|
||||
|
||||
@ -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: <timed out after ${timeoutMs}ms>`);
|
||||
const timedOut = message === 'Timeout' || /\btimed out\b/i.test(message);
|
||||
console.warn(` Tools: ${timedOut ? `<timed out after ${timeoutMs}ms>` : '<unavailable>'}`);
|
||||
console.warn(` Reason: ${message}`);
|
||||
if (advice.category === 'auth' && advice.authCommand) {
|
||||
console.warn(` Next: run '${advice.authCommand}' to finish authentication.`);
|
||||
|
||||
@ -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<Runtime, Map<string, Promise<ToolMetadata[]>>>();
|
||||
@ -11,7 +12,8 @@ const runtimeCache = new WeakMap<Runtime, Map<string, Promise<ToolMetadata[]>>>(
|
||||
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);
|
||||
|
||||
@ -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<void>;
|
||||
@ -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);
|
||||
}
|
||||
|
||||
@ -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: <unavailable>');
|
||||
|
||||
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);
|
||||
|
||||
|
||||
@ -228,4 +228,13 @@ describe('connectWithAuth', () => {
|
||||
expect.any(OAuthAuthorizationNotStartedError)
|
||||
);
|
||||
});
|
||||
|
||||
it('truncates oversized OAuth startup error details', () => {
|
||||
const hugeHtml = `<html>${'x'.repeat(5000)}</html>`;
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user