From af2f60eecf485d6fa4b6e5b71ce589d784e85f50 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 22:58:07 +0000 Subject: [PATCH] refactor: streamline call parsing/output pipeline --- src/cli/call-arguments.ts | 179 +++++++++++++--------- src/cli/call-command.ts | 85 ++++++++--- src/cli/image-output.ts | 62 ++++++++ src/cli/output-utils.ts | 173 +++++++-------------- src/result-utils.ts | 267 ++++++++++++++++----------------- tests/call-arguments.test.ts | 32 ++-- tests/cli-image-output.test.ts | 65 ++++++++ tests/cli-output-utils.test.ts | 137 +++++++++-------- 8 files changed, 582 insertions(+), 418 deletions(-) create mode 100644 src/cli/image-output.ts create mode 100644 tests/cli-image-output.test.ts diff --git a/src/cli/call-arguments.ts b/src/cli/call-arguments.ts index 93183fa..b50ab3e 100644 --- a/src/cli/call-arguments.ts +++ b/src/cli/call-arguments.ts @@ -23,9 +23,35 @@ export interface CallArgsParseResult { type CoercionMode = 'default' | 'raw-strings' | 'none'; +interface FlagParseState { + coercionMode: CoercionMode; +} + +interface FlagHandlerContext { + args: string[]; + index: number; + result: CallArgsParseResult; + state: FlagParseState; +} + +type FlagHandler = (context: FlagHandlerContext) => number; + +const FLAG_HANDLERS: Record = { + '--server': handleServerFlag, + '--mcp': handleServerFlag, + '--tool': handleToolFlag, + '--timeout': handleTimeoutFlag, + '--tail-log': handleTailLogFlag, + '--save-images': handleSaveImagesFlag, + '--yes': handleNoopFlag, + '--raw-strings': handleRawStringsFlag, + '--no-coerce': handleNoCoerceFlag, + '--args': handleArgsFlag, +}; + export function parseCallArguments(args: string[]): CallArgsParseResult { const result: CallArgsParseResult = { args: {}, tailLog: false, output: 'auto' }; - let coercionMode: CoercionMode = 'default'; + const flagState: FlagParseState = { coercionMode: 'default' }; const ephemeral = extractEphemeralServerFlags(args); result.ephemeral = ephemeral; result.output = consumeOutputFormat(args, { @@ -39,76 +65,9 @@ export function parseCallArguments(args: string[]): CallArgsParseResult { index += 1; continue; } - if (token === '--server' || token === '--mcp') { - const value = args[index + 1]; - if (!value) { - throw new Error(`Flag '${token}' requires a value.`); - } - result.server = value; - index += 2; - continue; - } - if (token === '--tool') { - const value = args[index + 1]; - if (!value) { - throw new Error(`Flag '${token}' requires a value.`); - } - result.tool = value; - index += 2; - continue; - } - if (token === '--timeout') { - result.timeoutMs = consumeTimeoutFlag(args, index, { - flagName: '--timeout', - missingValueMessage: '--timeout requires a value (milliseconds).', - }); - continue; - } - if (token === '--tail-log') { - result.tailLog = true; - index += 1; - continue; - } - if (token === '--save-images') { - const value = args[index + 1]; - if (!value) { - throw new Error('--save-images requires a directory path.'); - } - result.saveImagesDir = value; - index += 2; - continue; - } - if (token === '--yes') { - index += 1; - continue; - } - if (token === '--raw-strings') { - coercionMode = 'raw-strings'; - result.rawStrings = true; - index += 1; - continue; - } - if (token === '--no-coerce') { - coercionMode = 'none'; - result.rawStrings = true; - index += 1; - continue; - } - if (token === '--args') { - const value = args[index + 1]; - if (!value) { - throw new Error('--args requires a JSON value.'); - } - try { - const decoded = JSON.parse(value); - if (decoded === null || typeof decoded !== 'object' || Array.isArray(decoded)) { - throw new Error('--args must be a JSON object.'); - } - Object.assign(result.args, decoded); - } catch (error) { - throw new Error(`Unable to parse --args: ${(error as Error).message}`); - } - index += 2; + const flagHandler = FLAG_HANDLERS[token]; + if (flagHandler) { + index = flagHandler({ args, index, result, state: flagState }); continue; } positional.push(token); @@ -194,12 +153,12 @@ export function parseCallArguments(args: string[]): CallArgsParseResult { } const parsed = parseKeyValueToken(token, positional[index + 1]); if (!parsed) { - trailingPositional.push(coerceValue(token, coercionMode)); + trailingPositional.push(coerceValue(token, flagState.coercionMode)); index += 1; continue; } index += parsed.consumed; - const value = coerceValue(parsed.rawValue, coercionMode); + const value = coerceValue(parsed.rawValue, flagState.coercionMode); if (parsed.key === 'tool' && !result.tool) { if (typeof value !== 'string') { throw new Error("Argument 'tool' must be a string value."); @@ -222,6 +181,80 @@ export function parseCallArguments(args: string[]): CallArgsParseResult { return result; } +function handleServerFlag(context: FlagHandlerContext): number { + const token = context.args[context.index] ?? '--server'; + context.result.server = consumeFlagValue(context.args, context.index, token); + return context.index + 2; +} + +function handleToolFlag(context: FlagHandlerContext): number { + context.result.tool = consumeFlagValue(context.args, context.index, '--tool'); + return context.index + 2; +} + +function handleTimeoutFlag(context: FlagHandlerContext): number { + context.result.timeoutMs = consumeTimeoutFlag(context.args, context.index, { + flagName: '--timeout', + missingValueMessage: '--timeout requires a value (milliseconds).', + }); + // consumeTimeoutFlag removes the flag/value pair in-place; stay on the same index. + return context.index; +} + +function handleTailLogFlag(context: FlagHandlerContext): number { + context.result.tailLog = true; + return context.index + 1; +} + +function handleSaveImagesFlag(context: FlagHandlerContext): number { + context.result.saveImagesDir = consumeFlagValue( + context.args, + context.index, + '--save-images', + '--save-images requires a directory path.' + ); + return context.index + 2; +} + +function handleNoopFlag(context: FlagHandlerContext): number { + return context.index + 1; +} + +function handleRawStringsFlag(context: FlagHandlerContext): number { + context.state.coercionMode = 'raw-strings'; + context.result.rawStrings = true; + return context.index + 1; +} + +function handleNoCoerceFlag(context: FlagHandlerContext): number { + context.state.coercionMode = 'none'; + context.result.rawStrings = true; + return context.index + 1; +} + +function handleArgsFlag(context: FlagHandlerContext): number { + const raw = consumeFlagValue(context.args, context.index, '--args', '--args requires a JSON value.'); + let decoded: unknown; + try { + decoded = JSON.parse(raw); + } catch (error) { + throw new Error(`Unable to parse --args: ${(error as Error).message}`); + } + if (decoded === null || typeof decoded !== 'object' || Array.isArray(decoded)) { + throw new Error('Unable to parse --args: --args must be a JSON object.'); + } + Object.assign(context.result.args, decoded); + return context.index + 2; +} + +function consumeFlagValue(args: string[], index: number, token: string, missingValueMessage?: string): string { + const value = args[index + 1]; + if (value) { + return value; + } + throw new Error(missingValueMessage ?? `Flag '${token}' requires a value.`); +} + interface ParsedKeyValueToken { key: string; rawValue: string; diff --git a/src/cli/call-command.ts b/src/cli/call-command.ts index 7af4f7d..946f913 100644 --- a/src/cli/call-command.ts +++ b/src/cli/call-command.ts @@ -9,22 +9,59 @@ import { normalizeIdentifier, renderIdentifierResolutionMessages, } from './identifier-helpers.js'; +import { saveCallImagesIfRequested } from './image-output.js'; import { buildConnectionIssueEnvelope } from './json-output.js'; import { handleList } from './list-command.js'; import type { OutputFormat } from './output-utils.js'; -import { printCallOutput, saveCallImagesIfRequested, tailLogIfRequested } from './output-utils.js'; +import { printCallOutput, tailLogIfRequested } from './output-utils.js'; import { dumpActiveHandles } from './runtime-debug.js'; import { dimText, redText, yellowText } from './terminal.js'; import { resolveCallTimeout, withTimeout } from './timeouts.js'; import { loadToolMetadata } from './tool-cache.js'; -export async function handleCall( - runtime: Awaited>, - args: string[] -): Promise { - const parsed = parseCallArguments(args); - let ephemeralSpec = parsed.ephemeral ? { ...parsed.ephemeral } : undefined; +type Runtime = Awaited>; +interface ResolvedCallTarget { + server: string; + tool: string; +} + +interface PreparedCallRequest extends ResolvedCallTarget { + parsed: CallArgsParseResult; + hydratedArgs: Record; + timeoutMs: number; +} + +export async function handleCall(runtime: Runtime, args: string[]): Promise { + const prepared = await prepareCallRequest(runtime, args); + if (!prepared) { + return; + } + + const invocation = await invokePreparedCall(runtime, prepared); + if (!invocation) { + return; + } + + renderCallResult(invocation.result, prepared.parsed); +} + +async function prepareCallRequest(runtime: Runtime, args: string[]): Promise { + const parsed = parseCallArguments(args); + await normalizeParsedCallArguments(runtime, parsed); + const { server, tool } = await resolveServerAndTool(runtime, parsed); + + if (await maybeDescribeServer(runtime, server, tool, parsed.output)) { + return undefined; + } + + const timeoutMs = resolveCallTimeout(parsed.timeoutMs); + const hydratedArgs = await hydratePositionalArguments(runtime, server, tool, parsed.args, parsed.positionalArgs); + return { parsed, server, tool, hydratedArgs, timeoutMs }; +} + +async function normalizeParsedCallArguments(runtime: Runtime, parsed: CallArgsParseResult): Promise { + let ephemeralSpec = parsed.ephemeral ? { ...parsed.ephemeral } : undefined; const nameHints: string[] = []; const absorbUrlCandidate = (value: string | undefined): string | undefined => { if (!value) { @@ -70,7 +107,9 @@ export async function handleCall( if (!parsed.selector) { parsed.selector = prepared.target; } +} +async function resolveServerAndTool(runtime: Runtime, parsed: CallArgsParseResult): Promise { const target = resolveCallTarget(parsed, { allowMissingTool: true }); const server = target.server; let tool = target.tool; @@ -83,28 +122,36 @@ export async function handleCall( throw new Error('Missing tool name. Provide it via . or --tool.'); } } + return { server, tool }; +} - if (await maybeDescribeServer(runtime, server, tool, parsed.output)) { - return; - } - - const timeoutMs = resolveCallTimeout(parsed.timeoutMs); - const hydratedArgs = await hydratePositionalArguments(runtime, server, tool, parsed.args, parsed.positionalArgs); +async function invokePreparedCall( + runtime: Runtime, + prepared: PreparedCallRequest +): Promise<{ result: unknown; resolvedTool: string } | undefined> { let invocation: { result: unknown; resolvedTool: string }; try { - invocation = await invokeWithAutoCorrection(runtime, server, tool, hydratedArgs, timeoutMs); + invocation = await invokeWithAutoCorrection( + runtime, + prepared.server, + prepared.tool, + prepared.hydratedArgs, + prepared.timeoutMs + ); } catch (error) { - const issue = maybeReportConnectionIssue(server, tool, error); - if (parsed.output === 'json' || parsed.output === 'raw') { - const payload = buildConnectionIssueEnvelope({ server, tool, error, issue }); + const issue = maybeReportConnectionIssue(prepared.server, prepared.tool, error); + if (prepared.parsed.output === 'json' || prepared.parsed.output === 'raw') { + const payload = buildConnectionIssueEnvelope({ server: prepared.server, tool: prepared.tool, error, issue }); console.log(JSON.stringify(payload, null, 2)); process.exitCode = 1; - return; + return undefined; } throw error; } - const { result } = invocation; + return invocation; +} +function renderCallResult(result: unknown, parsed: CallArgsParseResult): void { const { callResult: wrapped } = wrapCallResult(result); printCallOutput(wrapped, result, parsed.output); saveCallImagesIfRequested(wrapped, parsed.saveImagesDir); diff --git a/src/cli/image-output.ts b/src/cli/image-output.ts new file mode 100644 index 0000000..8542fa5 --- /dev/null +++ b/src/cli/image-output.ts @@ -0,0 +1,62 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import type { CallResult } from '../result-utils.js'; + +export function saveCallImagesIfRequested(wrapped: CallResult, outputDir: string | undefined): void { + if (!outputDir) { + return; + } + const images = wrapped.images(); + if (!images || images.length === 0) { + return; + } + + fs.mkdirSync(outputDir, { recursive: true }); + const writtenPaths: string[] = []; + const timestamp = Date.now(); + for (const [index, image] of images.entries()) { + const extension = extensionFromMimeType(image.mimeType); + const baseName = `mcp-image-${timestamp}-${index + 1}`; + const outputPath = resolveImageOutputPath(outputDir, baseName, extension); + fs.writeFileSync(outputPath, Buffer.from(image.data, 'base64')); + writtenPaths.push(outputPath); + } + + for (const outputPath of writtenPaths) { + console.error(`[mcporter] Saved image: ${outputPath}`); + } +} + +function extensionFromMimeType(mimeType: string | undefined): string { + if (!mimeType) { + return '.bin'; + } + const normalized = mimeType.toLowerCase().split(';', 1)[0]?.trim() ?? ''; + const mapping: Record = { + 'image/png': '.png', + 'image/jpeg': '.jpg', + 'image/jpg': '.jpg', + 'image/webp': '.webp', + 'image/gif': '.gif', + 'image/svg+xml': '.svg', + 'image/bmp': '.bmp', + 'image/tiff': '.tiff', + 'image/x-icon': '.ico', + }; + return mapping[normalized] ?? '.bin'; +} + +function resolveImageOutputPath(outputDir: string, baseName: string, extension: string): string { + let candidate = path.join(outputDir, `${baseName}${extension}`); + if (!fs.existsSync(candidate)) { + return candidate; + } + let suffix = 2; + while (true) { + candidate = path.join(outputDir, `${baseName}-${suffix}${extension}`); + if (!fs.existsSync(candidate)) { + return candidate; + } + suffix += 1; + } +} diff --git a/src/cli/output-utils.ts b/src/cli/output-utils.ts index 4563e5f..5398365 100644 --- a/src/cli/output-utils.ts +++ b/src/cli/output-utils.ts @@ -1,80 +1,30 @@ import fs from 'node:fs'; -import path from 'node:path'; import { inspect } from 'node:util'; -import type { CallResult, ImageContent } from '../result-utils.js'; +import type { CallResult } from '../result-utils.js'; import { logWarn } from './logger-context.js'; export type OutputFormat = 'auto' | 'text' | 'markdown' | 'json' | 'raw'; const RAW_INSPECT_DEPTH = 8; +type RenderableKind = 'json' | 'markdown' | 'text' | 'raw'; + +interface RenderableOutput { + kind: RenderableKind; + value: unknown; +} + +const PREFERRED_OUTPUT_BY_FORMAT: Record = { + auto: ['json', 'markdown', 'text', 'raw'], + text: ['text', 'markdown', 'json', 'raw'], + markdown: ['markdown', 'text', 'json', 'raw'], + json: ['json', 'raw'], + raw: ['raw'], +}; + export function printCallOutput(wrapped: CallResult, raw: T, format: OutputFormat): void { - switch (format) { - case 'raw': { - printRaw(raw); - return; - } - case 'json': { - const jsonValue = wrapped.json(); - if (jsonValue !== null && attemptPrintJson(jsonValue)) { - return; - } - printRaw(raw); - return; - } - case 'markdown': { - const markdown = wrapped.markdown(); - if (typeof markdown === 'string') { - console.log(markdown); - return; - } - const text = wrapped.text(); - if (typeof text === 'string') { - console.log(text); - return; - } - const jsonValue = wrapped.json(); - if (jsonValue !== null && attemptPrintJson(jsonValue)) { - return; - } - printRaw(raw); - return; - } - case 'text': { - const text = wrapped.text(); - if (typeof text === 'string') { - console.log(text); - return; - } - const markdown = wrapped.markdown(); - if (typeof markdown === 'string') { - console.log(markdown); - return; - } - const jsonValue = wrapped.json(); - if (jsonValue !== null && attemptPrintJson(jsonValue)) { - return; - } - printRaw(raw); - return; - } - default: { - const jsonValue = wrapped.json(); - if (jsonValue !== null && attemptPrintJson(jsonValue)) { - return; - } - const markdown = wrapped.markdown(); - if (typeof markdown === 'string') { - console.log(markdown); - return; - } - const text = wrapped.text(); - if (typeof text === 'string') { - console.log(text); - return; - } - printRaw(raw); - } - } + const preferredKinds = PREFERRED_OUTPUT_BY_FORMAT[format]; + const renderable = resolveRenderableOutput(wrapped, raw, preferredKinds); + emitRenderableOutput(renderable); } export function tailLogIfRequested(result: unknown, enabled: boolean): void { @@ -121,61 +71,52 @@ export function tailLogIfRequested(result: unknown, enabled: boolean): void { } } -export function saveCallImagesIfRequested(wrapped: CallResult, outputDir: string | undefined): void { - if (!outputDir) { - return; - } - const images = wrapped.images(); - if (!images || images.length === 0) { - return; - } - const resolvedDir = path.resolve(outputDir); - try { - fs.mkdirSync(resolvedDir, { recursive: true }); - } catch (error) { - logWarn(`Unable to create image output directory ${resolvedDir}: ${(error as Error).message}`); - return; - } - writeImages(images, resolvedDir); -} - -function writeImages(images: ImageContent[], outputDir: string): void { - for (let i = 0; i < images.length; i++) { - const img = images[i]; - if (!img) { +function resolveRenderableOutput( + wrapped: CallResult, + raw: T, + preferredKinds: RenderableKind[] +): RenderableOutput { + for (const kind of preferredKinds) { + if (kind === 'json') { + const jsonValue = wrapped.json(); + if (jsonValue !== null) { + return { kind, value: jsonValue }; + } continue; } - const ext = extensionFromMimeType(img.mimeType); - const outputPath = resolveImageOutputPath(outputDir, i + 1, ext); - try { - const buffer = Buffer.from(img.data, 'base64'); - fs.writeFileSync(outputPath, buffer); - console.error(`[mcporter] Saved image: ${outputPath} (${buffer.length} bytes, ${img.mimeType})`); - } catch (writeError) { - logWarn(`Failed to save image ${i + 1} (${img.mimeType}): ${(writeError as Error).message}`); + if (kind === 'markdown') { + const markdown = wrapped.markdown(); + if (typeof markdown === 'string') { + return { kind, value: markdown }; + } + continue; + } + if (kind === 'text') { + const text = wrapped.text(); + if (typeof text === 'string') { + return { kind, value: text }; + } + continue; + } + if (kind === 'raw') { + return { kind, value: raw }; } } + return { kind: 'raw', value: raw }; } -function extensionFromMimeType(mimeType: string): string { - const subtype = mimeType.split('/')[1]?.split(';')[0]?.trim().toLowerCase(); - if (subtype && /^[a-z0-9.+-]+$/.test(subtype)) { - return subtype; - } - return 'png'; -} - -function resolveImageOutputPath(outputDir: string, imageIndex: number, extension: string): string { - const baseName = `image-${imageIndex}`; - let attempt = 0; - while (true) { - const suffix = attempt === 0 ? '' : `-${attempt}`; - const candidate = path.join(outputDir, `${baseName}${suffix}.${extension}`); - if (!fs.existsSync(candidate)) { - return candidate; +function emitRenderableOutput(renderable: RenderableOutput): void { + if (renderable.kind === 'json') { + if (!attemptPrintJson(renderable.value)) { + printRaw(renderable.value); } - attempt += 1; + return; } + if (renderable.kind === 'markdown' || renderable.kind === 'text') { + console.log(String(renderable.value)); + return; + } + printRaw(renderable.value); } function attemptPrintJson(value: unknown): boolean { diff --git a/src/result-utils.ts b/src/result-utils.ts index 7cc6d66..32d35e8 100644 --- a/src/result-utils.ts +++ b/src/result-utils.ts @@ -15,52 +15,47 @@ export interface CallResult { structuredContent(): unknown; } -// extractContentArray pulls the `content` array from MCP response envelopes. -function extractContentArray(raw: unknown): unknown[] | null { - if (!raw || typeof raw !== 'object') { - return null; - } - - const obj = raw as Record; - - // Check for content array at top level - if ('content' in obj && Array.isArray(obj.content)) { - return obj.content as unknown[]; - } - - // Check for content array nested inside 'raw' wrapper - if ('raw' in obj && obj.raw && typeof obj.raw === 'object') { - const nested = obj.raw as Record; - if ('content' in nested && Array.isArray(nested.content)) { - return nested.content as unknown[]; - } - } - - return null; +interface ExtractedEnvelope { + content: unknown[] | null; + structuredContent: unknown; } -// extractStructuredContent returns the structuredContent field when present. -function extractStructuredContent(raw: unknown): unknown { +interface CollectedCallContent { + content: unknown[] | null; + structuredContent: unknown; + textEntries: string[]; + markdownEntries: string[]; + jsonCandidates: unknown[]; + images: ImageContent[]; +} + +function extractEnvelope(raw: unknown): ExtractedEnvelope { if (!raw || typeof raw !== 'object') { - return null; + return { content: null, structuredContent: null }; } const obj = raw as Record; + let content: unknown[] | null = null; + let structuredContent: unknown = null; - // Check for structuredContent at top level + if ('content' in obj && Array.isArray(obj.content)) { + content = obj.content as unknown[]; + } if ('structuredContent' in obj) { - return obj.structuredContent; + structuredContent = obj.structuredContent; } - // Check for structuredContent nested inside 'raw' wrapper if ('raw' in obj && obj.raw && typeof obj.raw === 'object') { const nested = obj.raw as Record; - if ('structuredContent' in nested) { - return nested.structuredContent; + if (!content && 'content' in nested && Array.isArray(nested.content)) { + content = nested.content as unknown[]; + } + if (structuredContent === null && 'structuredContent' in nested) { + structuredContent = nested.structuredContent; } } - return null; + return { content, structuredContent }; } // asString converts known content/value shapes into plain strings. @@ -75,42 +70,92 @@ function asString(value: unknown): string | null { return null; } -// collectImages extracts all image content blocks. -function collectImages(content: unknown[]): ImageContent[] | null { +function collectCallContent(raw: unknown): CollectedCallContent { + const envelope = extractEnvelope(raw); + const textEntries: string[] = []; + const markdownEntries: string[] = []; + const jsonCandidates: unknown[] = []; const images: ImageContent[] = []; - for (const entry of content) { - if (entry && typeof entry === 'object' && 'type' in entry) { - const typedEntry = entry as Record; - if (typedEntry.type === 'image') { - const data = typedEntry.data; - const mimeType = typedEntry.mimeType ?? 'image/png'; - if (typeof data === 'string' && typeof mimeType === 'string') { - images.push({ data, mimeType }); - } + + if (!envelope.content) { + return { + content: envelope.content, + structuredContent: envelope.structuredContent, + textEntries, + markdownEntries, + jsonCandidates, + images, + }; + } + + for (const entry of envelope.content) { + if (typeof entry === 'string') { + const parsed = tryParseJson(entry); + if (parsed !== null) { + jsonCandidates.push(parsed); } + continue; + } + if (!entry || typeof entry !== 'object' || !('type' in entry)) { + continue; + } + + const typedEntry = entry as Record; + if (typedEntry.type === 'json') { + const parsed = tryParseJson(entry); + if (parsed !== null) { + jsonCandidates.push(parsed); + } + continue; + } + if (typedEntry.type === 'image') { + const data = typedEntry.data; + const mimeType = typedEntry.mimeType ?? 'image/png'; + if (typeof data === 'string' && typeof mimeType === 'string') { + images.push({ data, mimeType }); + } + continue; + } + if (typedEntry.type !== 'text' && typedEntry.type !== 'markdown') { + continue; + } + + const text = asString(entry); + if (!text) { + continue; + } + textEntries.push(text); + if (typedEntry.type === 'markdown') { + markdownEntries.push(text); + } + const parsed = tryParseJson(text); + if (parsed !== null) { + jsonCandidates.push(parsed); } } - return images.length > 0 ? images : null; + + return { + content: envelope.content, + structuredContent: envelope.structuredContent, + textEntries, + markdownEntries, + jsonCandidates, + images, + }; } -// collectText flattens all text/markdown entries into a joined string. -function collectText(content: unknown[], joiner: string): string | null { - const pieces: string[] = []; - for (const entry of content) { - if (entry && typeof entry === 'object' && 'type' in entry) { - const type = (entry as Record).type; - if (type === 'text' || type === 'markdown') { - const text = asString(entry); - if (text) { - pieces.push(text); - } - } - } +function collectText(entries: string[], joiner: string): string | null { + if (entries.length === 0) { + return null; } - if (pieces.length > 0) { - return pieces.join(joiner); + return entries.join(joiner); +} + +function collectImages(images: ImageContent[]): ImageContent[] | null { + if (images.length === 0) { + return null; } - return null; + return images; } // tryParseJson pulls JSON payloads out of structured responses or raw strings. @@ -138,6 +183,15 @@ function tryParseJson(value: unknown): unknown { // createCallResult wraps a tool response with helpers for common content types. export function createCallResult(raw: T): CallResult { + let cachedContent: CollectedCallContent | undefined; + const getCollectedContent = (): CollectedCallContent => { + if (cachedContent) { + return cachedContent; + } + cachedContent = collectCallContent(raw); + return cachedContent; + }; + return { raw, text(joiner = '\n') { @@ -148,95 +202,42 @@ export function createCallResult(raw: T): CallResult { return raw; } - const content = extractContentArray(raw); - if (content) { - const collected = collectText(content, joiner); - if (collected) { - return collected; - } + const collected = getCollectedContent(); + const combinedText = collectText(collected.textEntries, joiner); + if (combinedText) { + return combinedText; } - - const structured = extractStructuredContent(raw); - const asStr = asString(structured); - return asStr ?? null; + return asString(collected.structuredContent); }, markdown(joiner = '\n') { - const structured = extractStructuredContent(raw); + const collected = getCollectedContent(); + const structured = collected.structuredContent; if (structured && typeof structured === 'object') { const markdown = (structured as Record).markdown; if (typeof markdown === 'string') { return markdown; } } - - const content = extractContentArray(raw); - if (!content) { - return null; - } - const markdownEntries = content.filter( - (entry) => entry && typeof entry === 'object' && (entry as Record).type === 'markdown' - ); - if (markdownEntries.length === 0) { - return null; - } - return markdownEntries - .map((entry) => asString(entry) ?? '') - .filter(Boolean) - .join(joiner); + return collectText(collected.markdownEntries, joiner); }, json() { - const structured = extractStructuredContent(raw); - const parsedStructured = tryParseJson(structured); + const collected = getCollectedContent(); + const parsedStructured = tryParseJson(collected.structuredContent); if (parsedStructured !== null) { return parsedStructured as J; } - - const content = extractContentArray(raw); - if (content) { - const collected: unknown[] = []; - for (const entry of content) { - if (entry && typeof entry === 'object') { - const typedEntry = entry as Record; - if (typedEntry.type === 'json') { - const parsed = tryParseJson(entry); - if (parsed !== null) { - collected.push(parsed); - } - continue; - } - if (typedEntry.type === 'text' || typedEntry.type === 'markdown') { - const text = asString(entry); - if (typeof text === 'string') { - const parsedText = tryParseJson(text); - if (parsedText !== null) { - collected.push(parsedText); - } - } - continue; - } - } - if (typeof entry === 'string') { - const parsed = tryParseJson(entry); - if (parsed !== null) { - collected.push(parsed); - } - } - } - if (collected.length === 1) { - return collected[0] as J; - } - if (collected.length > 1) { - return collected as J; - } + if (collected.jsonCandidates.length === 1) { + return collected.jsonCandidates[0] as J; + } + if (collected.jsonCandidates.length > 1) { + return collected.jsonCandidates as J; } - if (typeof raw === 'string') { const parsedRaw = tryParseJson(raw); if (parsedRaw !== null) { return parsedRaw as J; } } - const textContent = this.text?.(); if (typeof textContent === 'string') { const parsedText = tryParseJson(textContent); @@ -244,7 +245,6 @@ export function createCallResult(raw: T): CallResult { return parsedText as J; } } - const markdownContent = this.markdown?.(); if (typeof markdownContent === 'string') { const parsedMarkdown = tryParseJson(markdownContent); @@ -255,17 +255,14 @@ export function createCallResult(raw: T): CallResult { return null; }, images() { - const content = extractContentArray(raw); - if (!content) { - return null; - } - return collectImages(content); + const collected = getCollectedContent(); + return collectImages(collected.images); }, content() { - return extractContentArray(raw); + return getCollectedContent().content; }, structuredContent() { - return extractStructuredContent(raw); + return getCollectedContent().structuredContent; }, }; } diff --git a/tests/call-arguments.test.ts b/tests/call-arguments.test.ts index 8883f2b..03c6ed4 100644 --- a/tests/call-arguments.test.ts +++ b/tests/call-arguments.test.ts @@ -11,6 +11,12 @@ describe('parseCallArguments', () => { expect(parsed.args.format).toBe('json'); }); + it.each(['--server', '--mcp'] as const)('captures %s as server override', (flag) => { + const parsed = parseCallArguments([flag, 'linear', 'list_documents']); + expect(parsed.server).toBe('linear'); + expect(parsed.tool).toBe('list_documents'); + }); + it('consumes function-style call expressions with HTTP selectors', () => { const call = 'https://example.com/mcp.getComponents(limit: 3, projectId: "123")'; const parsed = parseCallArguments([call]); @@ -50,17 +56,14 @@ describe('parseCallArguments', () => { warnSpy.mockRestore(); }); - it('coerces numeric strings to numbers by default', () => { - const parsed = parseCallArguments(['server.tool', 'code=123456']); - expect(parsed.args.code).toBe(123456); - expect(typeof parsed.args.code).toBe('number'); - }); - - it('preserves numeric strings when --raw-strings flag is used', () => { - const parsed = parseCallArguments(['--raw-strings', 'server.tool', 'code=123456']); - expect(parsed.args.code).toBe('123456'); - expect(typeof parsed.args.code).toBe('string'); - expect(parsed.rawStrings).toBe(true); + it.each([ + ['default', [], 123456, 'number'], + ['raw-strings', ['--raw-strings'], '123456', 'string'], + ['no-coerce', ['--no-coerce'], '123456', 'string'], + ] as const)('handles numeric coercion in %s mode', (_mode, flags, expected, expectedType) => { + const parsed = parseCallArguments([...flags, 'server.tool', 'code=123456']); + expect(parsed.args.code).toBe(expected); + expect(typeof parsed.args.code).toBe(expectedType); }); it('preserves leading zeros when --raw-strings flag is used', () => { @@ -99,7 +102,10 @@ describe('parseCallArguments', () => { expect(parsed.saveImagesDir).toBe('./tmp/images'); }); - it('throws when --save-images has no value', () => { - expect(() => parseCallArguments(['--save-images'])).toThrow(/--save-images requires a directory path/); + it.each([ + ['--save-images', /--save-images requires a directory path/], + ['--args', /--args requires a JSON value/], + ] as const)('throws when %s is missing a value', (flag, expectedError) => { + expect(() => parseCallArguments([flag])).toThrow(expectedError); }); }); diff --git a/tests/cli-image-output.test.ts b/tests/cli-image-output.test.ts new file mode 100644 index 0000000..dde2d01 --- /dev/null +++ b/tests/cli-image-output.test.ts @@ -0,0 +1,65 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, expect, it, vi } from 'vitest'; +import { saveCallImagesIfRequested } from '../src/cli/image-output.js'; +import { printCallOutput } from '../src/cli/output-utils.js'; +import { createCallResult } from '../src/result-utils.js'; + +describe('saveCallImagesIfRequested', () => { + it('does nothing when no output directory is provided', () => { + const wrapped = createCallResult({ + content: [{ type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }], + }); + const writeSpy = vi.spyOn(fs, 'writeFileSync'); + try { + saveCallImagesIfRequested(wrapped, undefined); + expect(writeSpy).not.toHaveBeenCalled(); + } finally { + writeSpy.mockRestore(); + } + }); + + it('saves image content blocks to the requested directory', () => { + const wrapped = createCallResult({ + content: [{ type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }], + }); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcporter-images-')); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + saveCallImagesIfRequested(wrapped, tempDir); + const files = fs.readdirSync(tempDir); + expect(files.length).toBe(1); + const first = files[0]; + expect(first?.endsWith('.png')).toBe(true); + const outputPath = path.join(tempDir, first ?? ''); + expect(fs.readFileSync(outputPath, 'utf8')).toBe('hello'); + } finally { + errorSpy.mockRestore(); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it('keeps json output on stdout unchanged when saving images', () => { + const raw = { + content: [ + { type: 'json', json: { id: 1 } }, + { type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }, + ], + }; + const wrapped = createCallResult(raw); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcporter-images-')); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + printCallOutput(wrapped, raw, 'json'); + saveCallImagesIfRequested(wrapped, tempDir); + expect(logSpy).toHaveBeenCalledTimes(1); + expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual({ id: 1 }); + } finally { + logSpy.mockRestore(); + errorSpy.mockRestore(); + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/cli-output-utils.test.ts b/tests/cli-output-utils.test.ts index 8ec543d..4fa1c0f 100644 --- a/tests/cli-output-utils.test.ts +++ b/tests/cli-output-utils.test.ts @@ -1,10 +1,81 @@ -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; import { describe, expect, it, vi } from 'vitest'; -import { printCallOutput, saveCallImagesIfRequested } from '../src/cli/output-utils.js'; +import { printCallOutput } from '../src/cli/output-utils.js'; import { createCallResult } from '../src/result-utils.js'; +describe('printCallOutput format selection', () => { + it.each([ + [ + 'auto prefers json payloads when available', + 'auto', + { + content: [ + { type: 'text', text: 'fallback text' }, + { type: 'json', json: { source: 'json' } }, + { type: 'markdown', text: '# heading' }, + ], + }, + (logged: unknown) => { + expect(JSON.parse(String(logged))).toEqual({ source: 'json' }); + }, + ], + [ + 'text prefers text over markdown/json', + 'text', + { + content: [ + { type: 'text', text: 'plain text wins' }, + { type: 'markdown', text: '# heading' }, + { type: 'json', json: { source: 'json' } }, + ], + }, + (logged: unknown) => { + expect(logged).toBe('plain text wins\n# heading'); + }, + ], + [ + 'markdown prefers markdown content', + 'markdown', + { + content: [ + { type: 'text', text: 'plain text' }, + { type: 'markdown', text: '## markdown wins' }, + ], + }, + (logged: unknown) => { + expect(logged).toBe('## markdown wins'); + }, + ], + [ + 'json falls back to raw output when no JSON candidate exists', + 'json', + 'raw-only-string', + (logged: unknown) => { + expect(logged).toBe('raw-only-string'); + }, + ], + [ + 'raw prints inspect output even when json exists', + 'raw', + { content: [{ type: 'json', json: { id: 1 } }] }, + (logged: unknown) => { + expect(String(logged)).toContain("type: 'json'"); + }, + ], + ] as const)('%s', (_name, format, raw, assertLogged) => { + const wrapped = createCallResult(raw); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + try { + printCallOutput(wrapped, raw, format); + expect(log).toHaveBeenCalledTimes(1); + const logged = log.mock.calls[0]?.[0]; + assertLogged(logged); + } finally { + log.mockRestore(); + } + }); +}); + describe('printCallOutput raw output', () => { it('does not truncate long strings when printing raw output', () => { const longText = 'x'.repeat(15000); @@ -56,61 +127,3 @@ describe('printCallOutput raw output', () => { } }); }); - -describe('saveCallImagesIfRequested', () => { - it('does nothing when no output directory is provided', () => { - const wrapped = createCallResult({ - content: [{ type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }], - }); - const writeSpy = vi.spyOn(fs, 'writeFileSync'); - try { - saveCallImagesIfRequested(wrapped, undefined); - expect(writeSpy).not.toHaveBeenCalled(); - } finally { - writeSpy.mockRestore(); - } - }); - - it('saves image content blocks to the requested directory', () => { - const wrapped = createCallResult({ - content: [{ type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }], - }); - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcporter-images-')); - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - try { - saveCallImagesIfRequested(wrapped, tempDir); - const files = fs.readdirSync(tempDir); - expect(files.length).toBe(1); - const first = files[0]; - expect(first?.endsWith('.png')).toBe(true); - const outputPath = path.join(tempDir, first ?? ''); - expect(fs.readFileSync(outputPath, 'utf8')).toBe('hello'); - } finally { - errorSpy.mockRestore(); - fs.rmSync(tempDir, { recursive: true, force: true }); - } - }); - - it('keeps json output on stdout unchanged when saving images', () => { - const raw = { - content: [ - { type: 'json', json: { id: 1 } }, - { type: 'image', mimeType: 'image/png', data: 'aGVsbG8=' }, - ], - }; - const wrapped = createCallResult(raw); - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mcporter-images-')); - const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - try { - printCallOutput(wrapped, raw, 'json'); - saveCallImagesIfRequested(wrapped, tempDir); - expect(logSpy).toHaveBeenCalledTimes(1); - expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual({ id: 1 }); - } finally { - logSpy.mockRestore(); - errorSpy.mockRestore(); - fs.rmSync(tempDir, { recursive: true, force: true }); - } - }); -});