fix: persist ad-hoc oauth promotion
This commit is contained in:
parent
b09ce5b20d
commit
c2c256db7e
@ -13,6 +13,7 @@
|
||||
- Print OAuth manual-completion URLs at the default warning log level so headless users can copy them. (PR #143 / issue #139, thanks @stainlu)
|
||||
- Support repeatable `--header KEY=value` flags for ad-hoc HTTP servers and persisted ad-hoc entries. (Issue #117)
|
||||
- Let generated CLIs use `--raw` without also passing required flags, and parse array flags containing JSON object items. (Issues #102 and #103)
|
||||
- Preserve `auth: "oauth"` when an ad-hoc HTTP server is OAuth-promoted and saved with `--persist`. (Issue #82)
|
||||
|
||||
### Config
|
||||
|
||||
|
||||
@ -149,6 +149,7 @@ Use `--scope home|project` with `mcporter config add` to pick the write target e
|
||||
- Names default to slugified hostnames or executable/script combos. Supply `--name` to improve reuse; mcporter uses that slug for OAuth caches even before persistence.
|
||||
- `--allow-http` is mandatory for cleartext endpoints so we never downgrade transport silently.
|
||||
- Add `--persist <path>` (defaulting to `config/mcporter.json` when omitted) to copy the ad-hoc definition into config. We reuse the same serializer as the import pipeline, so copying from Cursor → local config produces identical structure and preserves custom env/header fields.
|
||||
- When an ad-hoc HTTP server returns an OAuth challenge during `list`, `call`, or `auth`, the persisted entry is rewritten with `auth: "oauth"` so later commands use the cached OAuth path instead of retrying unauthenticated HTTP.
|
||||
- `--env KEY=VAL` entries merge with existing `env` dictionaries if you later persist the same server; nothing is lost when you alternate between CLI flags and JSON edits.
|
||||
- `--header KEY=VAL` entries merge into the persisted HTTP `headers` object when used with `--persist`; values support the same `$env:VAR`, `${VAR}`, and `${VAR:-fallback}` placeholders as config-file headers.
|
||||
|
||||
|
||||
@ -5,7 +5,7 @@ import { clearOAuthCaches } from '../oauth-persistence.js';
|
||||
import type { createRuntime } from '../runtime.js';
|
||||
import type { EphemeralServerSpec } from './adhoc-server.js';
|
||||
import { extractEphemeralServerFlags } from './ephemeral-flags.js';
|
||||
import { prepareEphemeralServerTarget } from './ephemeral-target.js';
|
||||
import { persistPreparedEphemeralServer, prepareEphemeralServerTarget } from './ephemeral-target.js';
|
||||
import { looksLikeHttpUrl } from './http-utils.js';
|
||||
import { buildConnectionIssueEnvelope } from './json-output.js';
|
||||
import { logInfo, logWarn } from './logger-context.js';
|
||||
@ -53,8 +53,12 @@ export async function handleAuth(runtime: Runtime, args: string[]): Promise<void
|
||||
|
||||
if (definition.command.kind === 'stdio' && definition.oauthCommand) {
|
||||
logInfo(`Starting auth helper for '${target}' (stdio). Leave this running until the browser flow completes.`);
|
||||
await runStdioAuth(definition);
|
||||
logInfo(`Auth helper for '${target}' finished. You can now call tools.`);
|
||||
try {
|
||||
await runStdioAuth(definition);
|
||||
logInfo(`Auth helper for '${target}' finished. You can now call tools.`);
|
||||
} finally {
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@ -62,9 +66,11 @@ export async function handleAuth(runtime: Runtime, args: string[]): Promise<void
|
||||
try {
|
||||
logInfo(`Initiating OAuth flow for '${target}'...`);
|
||||
const tools = await runtime.listTools(target, { autoAuthorize: true });
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
logInfo(`Authorization complete. ${tools.length} tool${tools.length === 1 ? '' : 's'} available.`);
|
||||
return;
|
||||
} catch (error) {
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
if (attempt === 0 && shouldRetryAuthError(error)) {
|
||||
logWarn('Server signaled OAuth after the initial attempt. Retrying with browser flow...');
|
||||
continue;
|
||||
|
||||
@ -7,7 +7,11 @@ import {
|
||||
CALL_HELP_EXAMPLE_LINES,
|
||||
CALL_HELP_RUNTIME_FLAG_LINES,
|
||||
} from './call-help.js';
|
||||
import { prepareEphemeralServerTarget } from './ephemeral-target.js';
|
||||
import {
|
||||
persistPreparedEphemeralServer,
|
||||
prepareEphemeralServerTarget,
|
||||
type PrepareEphemeralServerTargetResult,
|
||||
} from './ephemeral-target.js';
|
||||
import { looksLikeHttpUrl, normalizeHttpUrlCandidate } from './http-utils.js';
|
||||
import type { IdentifierResolution } from './identifier-helpers.js';
|
||||
import {
|
||||
@ -36,25 +40,31 @@ interface PreparedCallRequest extends ResolvedCallTarget {
|
||||
parsed: CallArgsParseResult;
|
||||
hydratedArgs: Record<string, unknown>;
|
||||
timeoutMs: number;
|
||||
ephemeralTarget?: PrepareEphemeralServerTargetResult;
|
||||
}
|
||||
|
||||
export async function handleCall(runtime: Runtime, args: string[]): Promise<void> {
|
||||
const prepared = await prepareCallRequest(runtime, args);
|
||||
if (!prepared) {
|
||||
return;
|
||||
}
|
||||
let prepared: PreparedCallRequest | undefined;
|
||||
try {
|
||||
prepared = await prepareCallRequest(runtime, args);
|
||||
if (!prepared) {
|
||||
return;
|
||||
}
|
||||
|
||||
const invocation = await invokePreparedCall(runtime, prepared);
|
||||
if (!invocation) {
|
||||
return;
|
||||
}
|
||||
const invocation = await invokePreparedCall(runtime, prepared);
|
||||
if (!invocation) {
|
||||
return;
|
||||
}
|
||||
|
||||
renderCallResult(invocation.result, prepared.parsed);
|
||||
renderCallResult(invocation.result, prepared.parsed);
|
||||
} finally {
|
||||
await persistPreparedEphemeralServer(runtime, prepared?.ephemeralTarget);
|
||||
}
|
||||
}
|
||||
|
||||
async function prepareCallRequest(runtime: Runtime, args: string[]): Promise<PreparedCallRequest | undefined> {
|
||||
const parsed = parseCallArguments(args);
|
||||
await normalizeParsedCallArguments(runtime, parsed);
|
||||
const ephemeralTarget = await normalizeParsedCallArguments(runtime, parsed);
|
||||
const { server, tool } = await resolveServerAndTool(runtime, parsed);
|
||||
|
||||
if (await maybeDescribeServer(runtime, server, tool, parsed.output)) {
|
||||
@ -72,10 +82,13 @@ async function prepareCallRequest(runtime: Runtime, args: string[]): Promise<Pre
|
||||
parsed.schemaArrayCoercionCandidates,
|
||||
timeoutMs
|
||||
);
|
||||
return { parsed, server, tool, hydratedArgs: schemaAwareArgs, timeoutMs };
|
||||
return { parsed, server, tool, hydratedArgs: schemaAwareArgs, timeoutMs, ephemeralTarget };
|
||||
}
|
||||
|
||||
async function normalizeParsedCallArguments(runtime: Runtime, parsed: CallArgsParseResult): Promise<void> {
|
||||
async function normalizeParsedCallArguments(
|
||||
runtime: Runtime,
|
||||
parsed: CallArgsParseResult
|
||||
): Promise<PrepareEphemeralServerTargetResult> {
|
||||
let ephemeralSpec = parsed.ephemeral ? { ...parsed.ephemeral } : undefined;
|
||||
const nameHints: string[] = [];
|
||||
const absorbUrlCandidate = (value: string | undefined): string | undefined => {
|
||||
@ -122,6 +135,7 @@ async function normalizeParsedCallArguments(runtime: Runtime, parsed: CallArgsPa
|
||||
if (!parsed.selector) {
|
||||
parsed.selector = prepared.target;
|
||||
}
|
||||
return prepared;
|
||||
}
|
||||
|
||||
async function resolveServerAndTool(runtime: Runtime, parsed: CallArgsParseResult): Promise<ResolvedCallTarget> {
|
||||
|
||||
@ -17,9 +17,10 @@ interface PrepareEphemeralServerTargetOptions {
|
||||
reuseFromSpec?: boolean;
|
||||
}
|
||||
|
||||
interface PrepareEphemeralServerTargetResult {
|
||||
export interface PrepareEphemeralServerTargetResult {
|
||||
target?: string;
|
||||
resolution?: EphemeralServerResolution;
|
||||
persistPath?: string;
|
||||
}
|
||||
|
||||
export async function prepareEphemeralServerTarget(
|
||||
@ -85,7 +86,33 @@ export async function prepareEphemeralServerTarget(
|
||||
await persistEphemeralServer(resolution, spec.persistPath);
|
||||
}
|
||||
const resolvedTarget = target ?? resolution.name;
|
||||
return { target: resolvedTarget, resolution };
|
||||
return { target: resolvedTarget, resolution, persistPath: spec.persistPath };
|
||||
}
|
||||
|
||||
export async function persistPreparedEphemeralServer(
|
||||
runtime: Runtime,
|
||||
prepared: PrepareEphemeralServerTargetResult | undefined
|
||||
): Promise<void> {
|
||||
if (!prepared?.resolution || !prepared.persistPath) {
|
||||
return;
|
||||
}
|
||||
let currentDefinition;
|
||||
try {
|
||||
currentDefinition = runtime.getDefinition(prepared.resolution.name);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
const persistedEntry = { ...prepared.resolution.persistedEntry };
|
||||
if (currentDefinition.auth === 'oauth') {
|
||||
persistedEntry.auth = 'oauth';
|
||||
}
|
||||
await persistEphemeralServer(
|
||||
{
|
||||
...prepared.resolution,
|
||||
persistedEntry,
|
||||
},
|
||||
prepared.persistPath
|
||||
);
|
||||
}
|
||||
|
||||
function applyNameHints(spec: EphemeralServerSpec | undefined, hints: string[] | undefined): void {
|
||||
|
||||
@ -4,7 +4,7 @@ import { MCPORTER_VERSION } from '../runtime.js';
|
||||
import { setStdioLogMode } from '../sdk-patches.js';
|
||||
import type { EphemeralServerSpec } from './adhoc-server.js';
|
||||
import { extractEphemeralServerFlags } from './ephemeral-flags.js';
|
||||
import { prepareEphemeralServerTarget } from './ephemeral-target.js';
|
||||
import { persistPreparedEphemeralServer, prepareEphemeralServerTarget } from './ephemeral-target.js';
|
||||
import { splitHttpToolSelector } from './http-utils.js';
|
||||
import { chooseClosestIdentifier, renderIdentifierResolutionMessages } from './identifier-helpers.js';
|
||||
import { formatExampleBlock } from './list-detail-helpers.js';
|
||||
@ -253,6 +253,7 @@ export async function handleList(
|
||||
if (flags.format === 'json') {
|
||||
try {
|
||||
const metadataEntries = await withTimeout(loadToolMetadata(runtime, target, { includeSchema: true }), timeoutMs);
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
const durationMs = Date.now() - startedAt;
|
||||
const payload = {
|
||||
mode: 'server',
|
||||
@ -274,6 +275,7 @@ export async function handleList(
|
||||
console.log(JSON.stringify(payload, null, 2));
|
||||
return;
|
||||
} catch (error) {
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
const durationMs = Date.now() - startedAt;
|
||||
const authCommand = buildAuthCommandHint(definition);
|
||||
const advice = classifyListError(error, definition.name, timeoutMs, { authCommand });
|
||||
@ -298,6 +300,7 @@ export async function handleList(
|
||||
try {
|
||||
// Always request schemas so we can render CLI-style parameter hints without re-querying per tool.
|
||||
const metadataEntries = await withTimeout(loadToolMetadata(runtime, target, { includeSchema: true }), timeoutMs);
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
const durationMs = Date.now() - startedAt;
|
||||
const summaryLine = printSingleServerHeader(
|
||||
definition,
|
||||
@ -338,6 +341,7 @@ export async function handleList(
|
||||
console.log('');
|
||||
return;
|
||||
} catch (error) {
|
||||
await persistPreparedEphemeralServer(runtime, prepared);
|
||||
const durationMs = Date.now() - startedAt;
|
||||
printSingleServerHeader(definition, undefined, durationMs, transportSummary, sourcePath);
|
||||
const message = error instanceof Error ? error.message : 'Failed to load tool list.';
|
||||
|
||||
@ -1,3 +1,6 @@
|
||||
import fs from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import type { ServerDefinition } from '../src/config.js';
|
||||
import { cliModulePromise, linearDefinition } from './fixtures/cli-list-fixtures.js';
|
||||
@ -115,6 +118,51 @@ describe('CLI list classification and routing', () => {
|
||||
logSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('persists OAuth promotion for ad-hoc HTTP servers', async () => {
|
||||
const { handleList } = await cliModulePromise;
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-persist-oauth-'));
|
||||
const persistPath = path.join(tempDir, 'mcporter.json');
|
||||
const definitions = new Map<string, ServerDefinition>();
|
||||
const runtime = {
|
||||
registerDefinition: vi.fn((definition: ServerDefinition) => {
|
||||
definitions.set(definition.name, definition);
|
||||
}),
|
||||
getDefinition: vi.fn((name: string) => {
|
||||
const entry = definitions.get(name);
|
||||
if (!entry) {
|
||||
throw new Error(`Unknown MCP server '${name}'.`);
|
||||
}
|
||||
return entry;
|
||||
}),
|
||||
getDefinitions: () => Array.from(definitions.values()),
|
||||
listTools: vi.fn(async (name: string) => {
|
||||
const entry = definitions.get(name);
|
||||
if (!entry) {
|
||||
throw new Error(`Unknown MCP server '${name}'.`);
|
||||
}
|
||||
definitions.set(name, { ...entry, auth: 'oauth' });
|
||||
return [{ name: 'ok' }];
|
||||
}),
|
||||
} as unknown as Awaited<ReturnType<(typeof import('../src/runtime.js'))['createRuntime']>>;
|
||||
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
await handleList(runtime, ['https://mcp.granola.ai/mcp', '--persist', persistPath]);
|
||||
|
||||
const parsed = JSON.parse(await fs.readFile(persistPath, 'utf8')) as {
|
||||
mcpServers: Record<string, { auth?: string; baseUrl?: string }>;
|
||||
};
|
||||
expect(parsed.mcpServers['mcp-granola-ai-mcp']).toMatchObject({
|
||||
baseUrl: 'https://mcp.granola.ai/mcp',
|
||||
auth: 'oauth',
|
||||
});
|
||||
} finally {
|
||||
logSpy.mockRestore();
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('reuses configured servers when listing by URL', async () => {
|
||||
const { handleList } = await cliModulePromise;
|
||||
const definition: ServerDefinition = {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user