From 6925ceec267c089bb5dc782b223d51d6cf04aa11 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 14:05:41 -0700 Subject: [PATCH 01/18] initial commit --- cli/src/commands/tools.ts | 159 +++++++++++++++++++++++---- cli/src/lib/inspector-render.ts | 59 +++++++++- cli/tests/agent-friendly-cli.test.ts | 48 ++++++++ cli/tests/tools-call-ui.test.ts | 123 ++++++++++++++++++--- docs/cli/apps-conformance.mdx | 4 +- docs/cli/overview.mdx | 7 +- docs/cli/reference.mdx | 7 +- docs/cli/tools-resources-prompts.mdx | 13 ++- mcpjam-inspector/dog-params.json | 3 + 9 files changed, 377 insertions(+), 46 deletions(-) create mode 100644 mcpjam-inspector/dog-params.json diff --git a/cli/src/commands/tools.ts b/cli/src/commands/tools.ts index 375b7dfb3..ca7f21822 100644 --- a/cli/src/commands/tools.ts +++ b/cli/src/commands/tools.ts @@ -45,6 +45,7 @@ interface ToolsCallOptions extends SharedServerTargetOptions { toolName?: string; name?: string; toolArgs?: string; + toolArgsStdin?: boolean; params?: string; validateResponse?: boolean; expectSuccess?: boolean; @@ -120,6 +121,7 @@ export function registerToolsCommands(program: Command): void { "Tool parameter object as JSON, @path, or - for stdin", ) .option("--params ", "Alias for --tool-args") + .option("--tool-args-stdin", "Read tool parameter JSON from stdin") .option( "--validate-response", "Validate the MCP tool-call envelope returned by the server", @@ -136,8 +138,14 @@ export function registerToolsCommands(program: Command): void { "--debug-out ", "Write a structured debug artifact to a file", ) - .option("--ui", "Render the tool result in Inspector App Builder") - .option("--open", "Open Inspector in the system browser before rendering") + .option( + "--ui", + "Render the tool result in Inspector App Builder; opens a browser by default", + ) + .option( + "--open", + "Open Inspector in the system browser before rendering (default with --ui)", + ) .option( "--no-open", "Start/use Inspector without opening a system browser", @@ -190,7 +198,7 @@ export function registerToolsCommands(program: Command): void { "Tool name", { required: true }, ) as string; - const paramsInput = resolveAliasedStringOption( + const resolvedParamsInput = resolveAliasedStringOption( options as Record, [ { key: "toolArgs", flag: "--tool-args" }, @@ -198,6 +206,12 @@ export function registerToolsCommands(program: Command): void { ], "Tool parameters", ); + if (options.toolArgsStdin && resolvedParamsInput !== undefined) { + throw usageError( + "--tool-args-stdin cannot be used together with --tool-args or --params.", + ); + } + const paramsInput = options.toolArgsStdin ? "-" : resolvedParamsInput; const params = parseJsonRecord(paramsInput, "Tool parameters") ?? {}; const targetSummary = summarizeServerDoctorTarget(target, config); const shouldValidateResponse = options.validateResponse === true; @@ -293,6 +307,7 @@ export function registerToolsCommands(program: Command): void { let inspectorRenderError: | { code: string; message: string; details?: unknown } | undefined; + let inspectorRenderSkipped = false; if (options.ui) { const serverName = @@ -311,6 +326,9 @@ export function registerToolsCommands(program: Command): void { baseUrl: options.inspectorUrl, config, frontendUrl, + onProgress: createInspectorUiProgressReporter({ + enabled: openBrowser && !globalOptions.quiet, + }), openBrowser, params, renderContext: renderContext!, @@ -333,9 +351,16 @@ export function registerToolsCommands(program: Command): void { }; } - const compactInspectorRender = buildCompactInspectorRender(uiResult); + inspectorRenderSkipped = + isSkippableInspectorRenderError(inspectorRenderError); + const compactInspectorRender = buildCompactInspectorRender(uiResult, { + skipped: inspectorRenderSkipped, + }); const compactOutputPayload = { - success: !inspectorRenderError && !validationFailed && !toolResultError, + success: + (!inspectorRenderError || inspectorRenderSkipped) && + !validationFailed && + !toolResultError, command: "tools call", inspectorUi: true, ...(typeof compactInspectorRender.browserUrl === "string" @@ -349,7 +374,11 @@ export function registerToolsCommands(program: Command): void { parameterKeys: Object.keys(params), result, inspectorRender: compactInspectorRender, - ...(inspectorRenderError ? { error: inspectorRenderError } : {}), + ...(inspectorRenderError + ? inspectorRenderSkipped + ? { warning: inspectorRenderError } + : { error: inspectorRenderError } + : {}), }; outputPayload = compactOutputPayload; debugOutputPayload = { @@ -357,6 +386,13 @@ export function registerToolsCommands(program: Command): void { params, inspectorRender: uiResult, }; + + writeInspectorRenderWarning({ + error: inspectorRenderError, + globalOptions, + render: compactInspectorRender, + skipped: inspectorRenderSkipped, + }); } await writeCommandDebugArtifact({ @@ -369,7 +405,7 @@ export function registerToolsCommands(program: Command): void { params, }, target: targetSummary, - outcome: inspectorRenderError + outcome: inspectorRenderError && !inspectorRenderSkipped ? { status: "error", error: inspectorRenderError, @@ -416,7 +452,7 @@ export function registerToolsCommands(program: Command): void { if (toolResultError) { setProcessExitCode(1); } - if (inspectorRenderError) { + if (inspectorRenderError && !inspectorRenderSkipped) { setProcessExitCode(1); } }); @@ -424,7 +460,7 @@ export function registerToolsCommands(program: Command): void { function resolveInspectorOpenBrowser( options: Pick, - globalOptions: GlobalOptions, + _globalOptions: GlobalOptions, ): boolean { if (options.attachOnly) { return false; @@ -432,7 +468,7 @@ function resolveInspectorOpenBrowser( if (options.open !== undefined) { return options.open; } - return globalOptions.format === "human" && !globalOptions.quiet; + return true; } function parseInspectorFrontendUrl( @@ -459,15 +495,12 @@ export function resolveInspectorSkipDiscovery( if (options.attachOnly) { return true; } - if ( - (globalOptions.format === "json" || globalOptions.quiet) && - options.open !== true - ) { - return true; - } - if (options.open === true) { + if (options.open !== false) { return false; } + if (globalOptions.format === "json" || globalOptions.quiet) { + return true; + } return false; } @@ -493,11 +526,93 @@ function extractInspectorRenderErrorUrls(error: { ...(typeof details.inspectorFrontendUrl === "string" ? { frontendUrl: details.inspectorFrontendUrl } : {}), + ...(typeof details.inspectorStarted === "boolean" + ? { inspectorStarted: details.inspectorStarted } + : {}), + ...(typeof details.hasActiveClient === "boolean" + ? { hasActiveClient: details.hasActiveClient } + : {}), + }; +} + +function createInspectorUiProgressReporter(options: { + enabled: boolean; +}): ((message: string) => void) | undefined { + if (!options.enabled) { + return undefined; + } + + let lastMessage = ""; + return (message: string) => { + if (!message || message === lastMessage) { + return; + } + lastMessage = message; + process.stderr.write(`${message}\n`); }; } +function writeInspectorRenderWarning(options: { + error: { code: string; message: string } | undefined; + globalOptions: Pick; + render: Record; + skipped: boolean; +}): void { + if (!options.skipped || !options.error || options.globalOptions.quiet) { + return; + } + + process.stderr.write( + `Warning: Inspector UI render skipped: ${options.error.message}\n`, + ); + + if (!isNoActiveClientMessage(options.error)) { + return; + } + + const browserUrl = + typeof options.render.browserUrl === "string" + ? options.render.browserUrl + : undefined; + process.stderr.write( + `Tip: open the Inspector App Builder${browserUrl ? ` at ${browserUrl}` : ""}, or rerun without --no-open or with --open to launch a browser automatically.\n`, + ); +} + +function isSkippableInspectorRenderError( + error: { code: string; message: string } | undefined, +): boolean { + if (!error) { + return false; + } + + const code = error.code.toLowerCase(); + if ( + code === "no_active_client" || + code === "timeout" || + code === "disconnected_server" || + code === "unsupported_in_mode" + ) { + return true; + } + + if (code === "operational_error") { + return /no active (browser )?client/i.test(error.message); + } + + return false; +} + +function isNoActiveClientMessage(error: { code: string; message: string }) { + return ( + error.code.toLowerCase() === "no_active_client" || + /no active (browser )?client/i.test(error.message) + ); +} + function buildCompactInspectorRender( uiResult: Record, + options: { skipped?: boolean } = {}, ): Record { const commands: Record = {}; let hasCommandError = false; @@ -515,12 +630,18 @@ function buildCompactInspectorRender( const topLevelError = uiResult.status === "error" && isRecord(uiResult.error) - ? { error: uiResult.error } + ? options.skipped + ? { warning: uiResult.error } + : { error: uiResult.error } : {}; return { status: - hasCommandError || uiResult.status === "error" ? "error" : "rendered", + options.skipped + ? "skipped" + : hasCommandError || uiResult.status === "error" + ? "error" + : "rendered", // Contract metadata: renders target the active client and fresh tabs do not // hydrate this injected state. mode: "active-client", diff --git a/cli/src/lib/inspector-render.ts b/cli/src/lib/inspector-render.ts index 6dd5854ac..f99c717aa 100644 --- a/cli/src/lib/inspector-render.ts +++ b/cli/src/lib/inspector-render.ts @@ -32,10 +32,13 @@ type InspectorUiRenderResult = InspectorAppRenderResult & { inspectorStarted: boolean; }; +type InspectorUiProgressReporter = (message: string) => void; + export async function runUiRender(options: { baseUrl?: string; config: MCPServerConfig; frontendUrl?: string; + onProgress?: InspectorUiProgressReporter; openBrowser?: boolean; params: Record; renderContext: AppRenderContext; @@ -57,6 +60,15 @@ export async function runUiRender(options: { timeoutMs: options.timeoutMs, }); + if (openBrowser) { + options.onProgress?.(`Inspector App Builder URL: ${ensureResult.url}`); + if (!ensureResult.hasActiveClient) { + options.onProgress?.( + "Waiting for Inspector browser client to connect...", + ); + } + } + if (!ensureResult.hasActiveClient && !openBrowser) { const startedNote = ensureResult.started ? " Inspector was just started by the CLI and is still running." @@ -64,6 +76,7 @@ export async function runUiRender(options: { throw operationalError( `Inspector has no active browser client.${startedNote} Open the Inspector App Builder URL in your browser, then rerun \`tools call --ui\`; or pass \`--open\` to let the CLI open a system browser.`, { + hasActiveClient: ensureResult.hasActiveClient, inspectorBrowserUrl: ensureResult.url, inspectorStarted: ensureResult.started, }, @@ -76,6 +89,7 @@ export async function runUiRender(options: { const renderResult = await runInspectorAppRender({ client, + onProgress: options.onProgress, params: options.params, renderContext: options.renderContext, serverName: options.serverName, @@ -99,6 +113,7 @@ export async function runUiRender(options: { async function runInspectorAppRender(options: { client: InspectorApiClient; + onProgress?: InspectorUiProgressReporter; params: Record; renderContext: AppRenderContext; serverName: string; @@ -164,16 +179,39 @@ async function runInspectorAppRender(options: { async function executeInspectorCommandWithClient( options: { client: InspectorApiClient; + onProgress?: InspectorUiProgressReporter; timeoutMs: number; }, request: Parameters[0], ): Promise { const startedAt = Date.now(); const deadline = startedAt + options.timeoutMs; + let lastProgressAt = 0; let lastResponse: InspectorCommandResponse | undefined; + const emitWaitingProgress = (force = false) => { + if (!options.onProgress) { + return; + } + + const now = Date.now(); + if (!force && now - lastProgressAt < 2_000) { + return; + } + + lastProgressAt = now; + const elapsedSeconds = Math.max(0, Math.floor((now - startedAt) / 1_000)); + options.onProgress( + `Waiting for Inspector browser client to handle ${request.type}... (${elapsedSeconds}s)`, + ); + }; + do { - const response = await options.client.executeCommand(request); + const response = await executeCommandWithProgress( + options.client.executeCommand(request), + emitWaitingProgress, + Boolean(options.onProgress), + ); lastResponse = response; const retryable = response.status === "error" && @@ -184,6 +222,7 @@ async function executeInspectorCommandWithClient( return response; } + emitWaitingProgress(true); const remaining = deadline - Date.now(); if (remaining <= 0) { return response; @@ -197,6 +236,24 @@ async function executeInspectorCommandWithClient( return lastResponse; } +async function executeCommandWithProgress( + pending: Promise, + onProgress: () => void, + enabled: boolean, +): Promise { + if (!enabled) { + return await pending; + } + + const interval = setInterval(onProgress, 2_000); + interval.unref?.(); + try { + return await pending; + } finally { + clearInterval(interval); + } +} + export function findInspectorRenderError( renderResult: Record, ): Extract["error"] | undefined { diff --git a/cli/tests/agent-friendly-cli.test.ts b/cli/tests/agent-friendly-cli.test.ts index aa7361ba8..8150aa3b5 100644 --- a/cli/tests/agent-friendly-cli.test.ts +++ b/cli/tests/agent-friendly-cli.test.ts @@ -245,3 +245,51 @@ test("JSON options accept stdin and reject duplicate stdin consumers", async () assert.equal(duplicate.exitCode, 2); assert.match(duplicate.stderr, /stdin was already consumed/); }); + +test("tools call accepts --tool-args-stdin shorthand", async () => { + const server = await startMockStreamableHttpServer(); + + try { + const result = await runCli( + [ + "--format", + "json", + "tools", + "call", + "--url", + server.url, + "--tool-name", + "echo", + "--tool-args-stdin", + ], + '{"message":"from stdin shorthand"}\n', + ); + + assert.equal(result.exitCode, 0, result.stderr); + const payload = JSON.parse(result.stdout) as { + content?: Array<{ type?: string; text?: string }>; + }; + assert.equal(payload.content?.[0]?.text, "Echo: from stdin shorthand"); + } finally { + await server.stop(); + } + + const conflict = await runCli( + [ + "--format", + "json", + "tools", + "call", + "--command", + "node", + "--tool-name", + "echo", + "--tool-args", + "{}", + "--tool-args-stdin", + ], + "{}\n", + ); + assert.equal(conflict.exitCode, 2); + assert.match(conflict.stderr, /--tool-args-stdin cannot be used/); +}); diff --git a/cli/tests/tools-call-ui.test.ts b/cli/tests/tools-call-ui.test.ts index 9437f3bd3..ca65dbdf0 100644 --- a/cli/tests/tools-call-ui.test.ts +++ b/cli/tests/tools-call-ui.test.ts @@ -102,6 +102,9 @@ async function startMockServer(options: { toolResult?: unknown; toolRpcError?: { code: number; message: string }; failRender?: boolean; + commandErrors?: Partial< + Record + >; }) { const requests: Array<{ method?: string; url?: string; body?: unknown }> = []; const toolResult = options.toolResult ?? { @@ -210,6 +213,24 @@ async function startMockServer(options: { if (request.url === "/api/mcp/command") { const type = body.type as string | undefined; + const commandError = type ? options.commandErrors?.[type] : undefined; + if (commandError) { + response.writeHead(commandError.status ?? 500, { + "Content-Type": "application/json", + }); + response.end( + JSON.stringify({ + id: (body.id as string | undefined) ?? "cmd", + status: "error", + error: { + code: commandError.code, + message: commandError.message, + }, + }), + ); + return; + } + const isRender = type === "renderToolResult"; response.writeHead(options.failRender && isRender ? 500 : 200, { "Content-Type": "application/json", @@ -444,7 +465,7 @@ test("tools call --ui --frontend-url uses the explicit browser URL without probi payload.inspectorRender.browserUrl, `http://localhost:${server.port}/inspector/#app-builder`, ); - assert.equal(payload.inspectorRender.browserOpenRequested, false); + assert.equal(payload.inspectorRender.browserOpenRequested, true); assert.equal( server.requests.some((entry) => entry.method === "GET" && entry.url === "/"), false, @@ -483,7 +504,7 @@ test("tools call without --ui preserves raw output and does not contact Inspecto } }); -test("tools call --ui in JSON mode does not open or wait without an active Inspector client", async () => { +test("tools call --ui --no-open in JSON mode skips render without an active Inspector client", async () => { const toolResult = { content: [{ type: "text", text: "view created" }] }; const server = await startMockServer({ hasActiveClient: false, toolResult }); @@ -494,6 +515,7 @@ test("tools call --ui in JSON mode does not open or wait without an active Inspe "tools", "call", "--ui", + "--no-open", "--inspector-url", `http://127.0.0.1:${server.port}`, "--url", @@ -504,19 +526,26 @@ test("tools call --ui in JSON mode does not open or wait without an active Inspe "{}", ]); - assert.equal(result.exitCode, 1, result.stderr); + assert.equal(result.exitCode, 0, result.stderr); + assert.match(result.stderr, /Warning: Inspector UI render skipped/); + assert.match(result.stderr, /Tip: open the Inspector App Builder/); const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< string, any >; - assert.equal(payload.success, false); + assert.equal(payload.success, true); assert.equal( payload.inspectorBrowserUrl, `http://127.0.0.1:${server.port}/#app-builder`, ); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.hasActiveClient, false); + assert.equal(payload.inspectorRender.inspectorStarted, false); assert.equal(payload.inspectorRender.browserOpenRequested, undefined); - assert.equal(payload.error.code, "OPERATIONAL_ERROR"); - assert.match(payload.error.message, /no active browser client/i); + assert.equal(payload.error, undefined); + assert.equal(payload.warning.code, "OPERATIONAL_ERROR"); + assert.match(payload.warning.message, /no active browser client/i); + assert.equal(payload.inspectorRender.warning.code, "OPERATIONAL_ERROR"); assert.equal( server.requests.some((entry) => entry.url === "/api/mcp/command"), false, @@ -526,7 +555,7 @@ test("tools call --ui in JSON mode does not open or wait without an active Inspe } }); -test("tools call --ui --quiet --format json does not scan nearby frontend ports", async () => { +test("tools call --ui --no-open --quiet --format json does not scan nearby frontend ports", async () => { const frontend = await startFrontendServerOnAvailablePort([ 5181, 5182, 5183, 5184, 5185, ]); @@ -548,6 +577,7 @@ test("tools call --ui --quiet --format json does not scan nearby frontend ports" "tools", "call", "--ui", + "--no-open", "--inspector-url", `http://127.0.0.1:${server.port}`, "--url", @@ -558,25 +588,29 @@ test("tools call --ui --quiet --format json does not scan nearby frontend ports" "{}", ]); - assert.equal(result.exitCode, 1, result.stderr); + assert.equal(result.exitCode, 0, result.stderr); assert.equal(frontend.rootRequests, 0); const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< string, any >; - assert.equal(payload.success, false); + assert.equal(payload.success, true); assert.equal( payload.inspectorBrowserUrl, `${staleFrontendUrl}/#app-builder`, ); - assert.match(payload.error.message, /no active browser client/i); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.hasActiveClient, false); + assert.equal(payload.inspectorRender.inspectorStarted, false); + assert.equal(payload.error, undefined); + assert.match(payload.warning.message, /no active browser client/i); } finally { await server.stop(); await frontend.stop(); } }); -test("tools call --ui --open may render while waiting for a browser client", async () => { +test("tools call --ui opens by default and may render while waiting for a browser client", async () => { const toolResult = { content: [{ type: "text", text: "view created" }] }; const server = await startMockServer({ hasActiveClient: false, toolResult }); @@ -587,7 +621,6 @@ test("tools call --ui --open may render while waiting for a browser client", asy "tools", "call", "--ui", - "--open", "--inspector-url", `http://127.0.0.1:${server.port}`, "--url", @@ -599,6 +632,8 @@ test("tools call --ui --open may render while waiting for a browser client", asy ]); assert.equal(result.exitCode, 0, result.stderr); + assert.match(result.stderr, /Inspector App Builder URL:/); + assert.match(result.stderr, /Waiting for Inspector browser client/); const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< string, any @@ -616,6 +651,62 @@ test("tools call --ui --open may render while waiting for a browser client", asy } }); +test("tools call --ui --open treats Inspector command timeouts as render skips", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ + commandErrors: { + openAppBuilder: { + code: "timeout", + message: "Inspector command timed out after 30000ms.", + status: 504, + }, + }, + toolResult, + }); + + try { + const result = await runCli([ + "--format", + "json", + "tools", + "call", + "--ui", + "--open", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 0, result.stderr); + assert.match(result.stderr, /Inspector App Builder URL:/); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, true); + assert.equal(payload.error, undefined); + assert.equal(payload.warning.code, "timeout"); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal( + payload.inspectorRender.commands.openAppBuilder.error.code, + "timeout", + ); + assert.deepEqual( + server.requests + .filter((entry) => entry.url === "/api/mcp/command") + .map((entry) => (entry.body as { type?: string }).type), + ["openAppBuilder"], + ); + } finally { + await server.stop(); + } +}); + test("tools call --ui --open can discover a nearby dev frontend", async () => { const frontend = await startFrontendServerOnAvailablePort([ 5181, 5182, 5183, 5184, 5185, @@ -820,7 +911,9 @@ test("tools call help lists frontend-url", async () => { const result = await runCli(["tools", "call", "--help"]); assert.equal(result.exitCode, 0, result.stderr); + assert.match(result.stdout, /--tool-args-stdin/); assert.match(result.stdout, /--frontend-url /); + assert.match(result.stdout, /default with --ui/); }); test("tools call --ui rejects attach-only with open", async () => { @@ -911,8 +1004,10 @@ test("tools call --ui resolves discovery policy from output mode", () => { assert.equal(resolveInspectorSkipDiscovery({}, humanOptions), false); assert.equal(resolveInspectorSkipDiscovery({ open: true }, jsonOptions), false); - assert.equal(resolveInspectorSkipDiscovery({}, jsonOptions), true); - assert.equal(resolveInspectorSkipDiscovery({}, quietOptions), true); + assert.equal(resolveInspectorSkipDiscovery({}, jsonOptions), false); + assert.equal(resolveInspectorSkipDiscovery({}, quietOptions), false); + assert.equal(resolveInspectorSkipDiscovery({ open: false }, jsonOptions), true); + assert.equal(resolveInspectorSkipDiscovery({ open: false }, quietOptions), true); assert.equal(resolveInspectorSkipDiscovery({ attachOnly: true }, humanOptions), true); assert.equal( resolveInspectorSkipDiscovery( diff --git a/docs/cli/apps-conformance.mdx b/docs/cli/apps-conformance.mdx index 5a833c884..6853ee168 100644 --- a/docs/cli/apps-conformance.mdx +++ b/docs/cli/apps-conformance.mdx @@ -169,7 +169,7 @@ mcpjam apps conformance \ Use `tools list` to find UI-capable tools. A tool has interactive UI when its metadata includes `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata`. -Then use `tools call --ui` when you need to inspect the output of one UI-capable tool call. The CLI starts or attaches to the local Inspector backend, connects the target server, focuses App Builder in the active Inspector browser client, injects the already-completed tool result, and requests a UI snapshot. `--inspector-url` is the Inspector backend/API URL; `--frontend-url` is the browser/client URL and skips frontend discovery. JSON/quiet invocations do not open a system browser or scan local frontend ports by default, but they may start Inspector; pass `--open` for interactive use and local dev frontend discovery, use `--attach-only` to disallow startup, browser opening, and discovery, or open `http://127.0.0.1:6274/#app-builder` with browser automation before running the command. +Then use `tools call --ui` when you need to inspect the output of one UI-capable tool call. The CLI starts or attaches to the local Inspector backend, opens Inspector by default, connects the target server, focuses App Builder in the active Inspector browser client, injects the already-completed tool result, and requests a UI snapshot. `--inspector-url` is the Inspector backend/API URL; `--frontend-url` is the browser/client URL and skips frontend discovery. Use `--no-open` when browser automation already opened `http://127.0.0.1:6274/#app-builder`; use `--attach-only` to disallow startup, browser opening, and discovery. Default `--ui` and explicit `--open` flows print browser-client wait progress on stderr unless `--quiet` is set. ```bash mcpjam tools call \ @@ -190,7 +190,7 @@ mcpjam tools call \ client; opening the URL in a fresh tab later does not hydrate the old render. -If the command returns `success: false` with an `error`, check whether the failure came from `result` or from `inspectorRender`. A render failure can coexist with a successful tool execution. +If `inspectorRender.status` is `skipped`, the tool call succeeded but Inspector had no active browser client, a render precondition was missing, or the render wait timed out; the envelope includes a top-level `warning`, non-quiet runs print a stderr tip, and the command keeps the tool-call exit code. If the command returns `success: false` with an `error`, check whether the failure came from `result` or from an Inspector render command. Raw `ui://` resource HTML can be inspected directly: diff --git a/docs/cli/overview.mdx b/docs/cli/overview.mdx index 86905f38d..f12f2c8b4 100644 --- a/docs/cli/overview.mdx +++ b/docs/cli/overview.mdx @@ -69,7 +69,7 @@ The CLI auto-detects whether stdout is a terminal: **For agents**: raw JSON is the source of truth. Human format is a lossy presentation layer. If you're parsing output programmatically, pass `--quiet --format json`. -JSON-valued flags accept inline JSON, `@path`, or `-` for stdin. Use files or stdin for large payloads to avoid shell escaping issues: +JSON-valued flags accept inline JSON, `@path`, or `-` for stdin. `tools call` also accepts `--tool-args-stdin` as a shorthand for `--tool-args -`. Use files or stdin for large payloads to avoid shell escaping issues: ```bash echo '{"key":"value"}' | mcpjam tools call --url $URL --access-token $TOKEN \ @@ -150,9 +150,8 @@ mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ # 5. If the server exposes MCP Apps, validate the ui:// surface mcpjam apps conformance --url https://your-server.com/mcp --access-token $TOKEN -# 6. Render one UI-capable tool result in Inspector's App Builder -# Open http://127.0.0.1:6274/#app-builder in your browser automation first -# (or pass --open for an interactive run). +# 6. Render one UI-capable tool result in Inspector's App Builder. +# --ui opens Inspector by default; add --no-open if browser automation already opened it. mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ --tool-name my_app_tool --tool-args @params.json --ui --quiet --format json ``` diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index 97cb74819..ee0434b0c 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -101,12 +101,13 @@ Uses shared connection flags. | `--name ` | Legacy alias for `--tool-name` | | `--tool-args ` | Tool arguments as inline JSON, `@path`, or `-` for stdin | | `--params ` | Legacy alias for `--tool-args` | +| `--tool-args-stdin` | Read tool arguments JSON from stdin | | `--validate-response` | Validate the MCP tool-call envelope returned by the server | | `--expect-success` | Fail when the tool result reports `isError` | | `--reporter ` | `json-summary` or `junit-xml` validation report output | | `--debug-out ` | Write debug artifact to file | -| `--ui` | Attach to Inspector and render the completed tool result in App Builder | -| `--open` | Open Inspector in the system browser before rendering | +| `--ui` | Attach to Inspector and render the completed tool result in App Builder; opens a browser by default | +| `--open` | Open Inspector in the system browser before rendering (default with `--ui`) | | `--no-open` | Start/use Inspector without opening a system browser | | `--attach-only` | Require an already-running Inspector browser client; do not start or open Inspector | | `--inspector-url ` | Local Inspector backend/API base URL | @@ -120,7 +121,7 @@ Uses shared connection flags. Plus shared connection flags. -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. JSON/quiet invocations do not open a system browser or scan local frontend ports by default, but they may start Inspector; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Open `inspectorBrowserUrl` first with your browser automation, then rerun the command, or pass `--open` for interactive use. Human TTY and `--open` flows may discover local dev frontends. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. Skipped renders are emitted as top-level `warning` fields and do not change the tool-call exit code; when not quiet, the CLI also prints a short stderr warning and browser tip. Tool failures, validation failures, and render command errors still exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Human and default `--ui` flows may discover local dev frontends and print browser-client wait progress on stderr unless `--quiet` is set. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. --- diff --git a/docs/cli/tools-resources-prompts.mdx b/docs/cli/tools-resources-prompts.mdx index df5a1bf83..60b7d1e2e 100644 --- a/docs/cli/tools-resources-prompts.mdx +++ b/docs/cli/tools-resources-prompts.mdx @@ -24,7 +24,7 @@ mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ --tool-args @params.json --quiet --format json ``` -Tool arguments can be inline JSON, `@path`, or `-` for stdin. The result includes the tool's response content. +Tool arguments can be inline JSON, `@path`, `-` for stdin, or `--tool-args-stdin` as a shorthand for stdin. The result includes the tool's response content. `tools call` supports `--debug-out ` to capture the full request/response @@ -45,9 +45,9 @@ mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ --quiet --format json ``` -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it returns an envelope containing the raw `result`, `inspectorBrowserUrl`, and `inspectorRender` evidence. Browser automation should open `inspectorBrowserUrl`; `--inspector-url` is the local Inspector backend/API base URL. If you already know the Inspector browser/client URL, pass `--frontend-url ` to use it directly and skip health-advertised frontend checks and local dev port discovery. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default and returns an envelope containing the raw `result`, `inspectorBrowserUrl`, and `inspectorRender` evidence. `inspectorRender.status` is the UI signal: `rendered` means Inspector accepted the render, `skipped` means the tool succeeded but the active browser client, a render precondition, or the render wait was missing, and `error` means a non-recoverable Inspector render command failed. Skipped renders appear as top-level `warning` fields, do not change the tool-call exit code, and print a short stderr warning unless `--quiet` is set. Browser automation can open `inspectorBrowserUrl` itself and pass `--no-open`; `--inspector-url` is the local Inspector backend/API base URL. If you already know the Inspector browser/client URL, pass `--frontend-url ` to use it directly and skip health-advertised frontend checks and local dev port discovery. -For agent/browser automation, open `http://127.0.0.1:6274/#app-builder` in the automation browser first, then run `tools call --ui --quiet --format json`. JSON/quiet `--ui` will not open a system browser or scan local frontend ports unless you pass `--open`, but it may start Inspector; add `--attach-only` when startup, browser opening, and discovery should be disallowed. Human TTY and `--open` flows may discover local dev frontends. The normal JSON output is compact; pass `--debug-out ` for the full render envelope. +For agent/browser automation, either let `--ui` open Inspector automatically, or open `http://127.0.0.1:6274/#app-builder` in the automation browser first and run `tools call --ui --no-open --quiet --format json`. Add `--attach-only` when startup, browser opening, and discovery should be disallowed. Default `--ui` and explicit `--open` flows may discover local dev frontends and print browser-client wait progress on stderr unless `--quiet` is set. The normal JSON output is compact; pass `--debug-out ` for the full render envelope. For a local stdio server: @@ -134,6 +134,13 @@ echo '{"query":"setup guide"}' | mcpjam tools call --url $URL --access-token $TO --tool-name search_docs --tool-args - --quiet --format json ``` +`--tool-args-stdin` is equivalent to `--tool-args -`: + +```bash +generate-params | mcpjam tools call --url $URL --access-token $TOKEN \ + --tool-name search_docs --tool-args-stdin --quiet --format json +``` + ### Using a credentials file Instead of passing `--access-token` to every command, save credentials once and reuse the file: diff --git a/mcpjam-inspector/dog-params.json b/mcpjam-inspector/dog-params.json new file mode 100644 index 000000000..0dad093ee --- /dev/null +++ b/mcpjam-inspector/dog-params.json @@ -0,0 +1,3 @@ +{ + "elements": "[{\"type\":\"cameraUpdate\",\"width\":800,\"height\":600,\"x\":0,\"y\":0},{\"type\":\"text\",\"id\":\"title\",\"x\":285,\"y\":25,\"text\":\"A Very Good Boy\",\"fontSize\":26,\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"lear\",\"x\":292,\"y\":152,\"width\":68,\"height\":105,\"backgroundColor\":\"#92400e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#78350f\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"rear\",\"x\":440,\"y\":152,\"width\":68,\"height\":105,\"backgroundColor\":\"#92400e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#78350f\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"head\",\"x\":318,\"y\":153,\"width\":164,\"height\":154,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":3},{\"type\":\"ellipse\",\"id\":\"ilear\",\"x\":308,\"y\":163,\"width\":40,\"height\":68,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":65},{\"type\":\"ellipse\",\"id\":\"irear\",\"x\":452,\"y\":163,\"width\":40,\"height\":68,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":65},{\"type\":\"rectangle\",\"id\":\"body\",\"x\":292,\"y\":283,\"width\":216,\"height\":168,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":3,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"tummy\",\"x\":335,\"y\":303,\"width\":130,\"height\":108,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":70},{\"type\":\"ellipse\",\"id\":\"leyw\",\"x\":340,\"y\":196,\"width\":34,\"height\":34,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"leyp\",\"x\":347,\"y\":202,\"width\":20,\"height\":22,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"leys\",\"x\":352,\"y\":205,\"width\":7,\"height\":7,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"reyw\",\"x\":426,\"y\":196,\"width\":34,\"height\":34,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"reyp\",\"x\":433,\"y\":202,\"width\":20,\"height\":22,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"reys\",\"x\":438,\"y\":205,\"width\":7,\"height\":7,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"muzzle\",\"x\":358,\"y\":248,\"width\":84,\"height\":58,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"nose\",\"x\":374,\"y\":252,\"width\":52,\"height\":32,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"nosesh\",\"x\":381,\"y\":256,\"width\":11,\"height\":9,\"backgroundColor\":\"#9ca3af\",\"fillStyle\":\"solid\",\"strokeColor\":\"#9ca3af\"},{\"type\":\"ellipse\",\"id\":\"tongue\",\"x\":381,\"y\":276,\"width\":38,\"height\":42,\"backgroundColor\":\"#ec4899\",\"fillStyle\":\"solid\",\"strokeColor\":\"#be185d\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"tcrease\",\"x\":400,\"y\":278,\"width\":0,\"height\":32,\"points\":[[0,0],[0,32]],\"strokeColor\":\"#be185d\",\"strokeWidth\":2,\"endArrowhead\":null},{\"type\":\"rectangle\",\"id\":\"lleg\",\"x\":312,\"y\":435,\"width\":58,\"height\":88,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"lpaw\",\"x\":303,\"y\":507,\"width\":70,\"height\":30,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2},{\"type\":\"rectangle\",\"id\":\"rleg\",\"x\":430,\"y\":435,\"width\":58,\"height\":88,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"rpaw\",\"x\":424,\"y\":507,\"width\":70,\"height\":30,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"tail\",\"x\":498,\"y\":345,\"width\":85,\"height\":-90,\"points\":[[0,0],[45,-45],[85,-90]],\"strokeColor\":\"#92400e\",\"strokeWidth\":9,\"endArrowhead\":null,\"roughness\":2},{\"type\":\"ellipse\",\"id\":\"bubble\",\"x\":528,\"y\":182,\"width\":118,\"height\":64,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\",\"strokeWidth\":2},{\"type\":\"text\",\"id\":\"woof\",\"x\":554,\"y\":205,\"text\":\"Woof!\",\"fontSize\":22,\"strokeColor\":\"#4a9eed\"},{\"type\":\"arrow\",\"id\":\"conn\",\"x\":520,\"y\":238,\"width\":-22,\"height\":8,\"points\":[[0,0],[-22,8]],\"strokeColor\":\"#1e1e1e\",\"strokeWidth\":2,\"endArrowhead\":null}]" +} From 14da9bd28d5689740d0cdd420bf026624c957730 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 14:41:49 -0700 Subject: [PATCH 02/18] nits --- cli/src/commands/tools.ts | 284 +++++++++++++++----- cli/src/lib/inspector-render.ts | 8 +- cli/tests/tools-call-ui.test.ts | 371 ++++++++++++++++++++++++++- docs/cli/apps-conformance.mdx | 4 +- docs/cli/overview.mdx | 2 +- docs/cli/reference.mdx | 48 +++- docs/cli/tools-resources-prompts.mdx | 4 +- mcpjam-inspector/cat-params.json | 3 + 8 files changed, 638 insertions(+), 86 deletions(-) create mode 100644 mcpjam-inspector/cat-params.json diff --git a/cli/src/commands/tools.ts b/cli/src/commands/tools.ts index ca7f21822..9141b4f81 100644 --- a/cli/src/commands/tools.ts +++ b/cli/src/commands/tools.ts @@ -52,6 +52,7 @@ interface ToolsCallOptions extends SharedServerTargetOptions { reporter?: string; debugOut?: string; ui?: boolean; + requireRender?: boolean; open?: boolean; attachOnly?: boolean; inspectorUrl?: string; @@ -140,11 +141,15 @@ export function registerToolsCommands(program: Command): void { ) .option( "--ui", - "Render the tool result in Inspector App Builder; opens a browser by default", + "Render the tool result in Inspector App Builder; opens a browser by default in a TTY", + ) + .option( + "--require-render", + "Treat skipped Inspector renders as errors (with --ui)", ) .option( "--open", - "Open Inspector in the system browser before rendering (default with --ui)", + "Open Inspector in the system browser before rendering (default with --ui in a TTY)", ) .option( "--no-open", @@ -220,6 +225,9 @@ export function registerToolsCommands(program: Command): void { if (options.ui && reporter) { throw usageError("--ui cannot be used together with --reporter."); } + if (options.requireRender && !options.ui) { + throw usageError("--require-render requires --ui."); + } if (options.attachOnly && options.open === true) { throw usageError("--attach-only cannot be used together with --open."); } @@ -308,16 +316,18 @@ export function registerToolsCommands(program: Command): void { | { code: string; message: string; details?: unknown } | undefined; let inspectorRenderSkipped = false; + let inspectorRenderIssue: InspectorRenderIssue | undefined; if (options.ui) { const serverName = typeof options.serverName === "string" && options.serverName.trim() ? options.serverName.trim() : buildInspectorServerName(options); - const openBrowser = resolveInspectorOpenBrowser(options, globalOptions); + const openBrowser = resolveInspectorOpenBrowser(options); const skipDiscovery = resolveInspectorSkipDiscovery( options, globalOptions, + { openBrowser }, ); let uiResult: Record; @@ -328,7 +338,9 @@ export function registerToolsCommands(program: Command): void { frontendUrl, onProgress: createInspectorUiProgressReporter({ enabled: openBrowser && !globalOptions.quiet, + stderrIsTTY: resolveInspectorUiStderrIsTTY(), }), + heartbeatEnabled: resolveInspectorUiStderrIsTTY(), openBrowser, params, renderContext: renderContext!, @@ -351,14 +363,26 @@ export function registerToolsCommands(program: Command): void { }; } - inspectorRenderSkipped = - isSkippableInspectorRenderError(inspectorRenderError); + const inspectorRenderClassification = + classifyInspectorRenderError(inspectorRenderError); + inspectorRenderSkipped = inspectorRenderClassification.skippable; + inspectorRenderIssue = buildInspectorRenderIssue( + inspectorRenderError, + uiResult, + inspectorRenderClassification, + ); const compactInspectorRender = buildCompactInspectorRender(uiResult, { skipped: inspectorRenderSkipped, + remediation: inspectorRenderClassification.remediation, + issue: inspectorRenderIssue, }); + const requireRender = options.requireRender === true; + const renderFailure = + inspectorRenderError && + (!inspectorRenderSkipped || requireRender); const compactOutputPayload = { success: - (!inspectorRenderError || inspectorRenderSkipped) && + !renderFailure && !validationFailed && !toolResultError, command: "tools call", @@ -375,9 +399,11 @@ export function registerToolsCommands(program: Command): void { result, inspectorRender: compactInspectorRender, ...(inspectorRenderError - ? inspectorRenderSkipped - ? { warning: inspectorRenderError } - : { error: inspectorRenderError } + ? inspectorRenderSkipped && !requireRender + ? { warning: inspectorRenderIssue ?? inspectorRenderError } + : inspectorRenderSkipped + ? { error: inspectorRenderIssue ?? inspectorRenderError } + : { error: inspectorRenderError } : {}), }; outputPayload = compactOutputPayload; @@ -388,13 +414,16 @@ export function registerToolsCommands(program: Command): void { }; writeInspectorRenderWarning({ - error: inspectorRenderError, + issue: inspectorRenderIssue, globalOptions, render: compactInspectorRender, + required: requireRender, skipped: inspectorRenderSkipped, }); } + const requireRender = options.requireRender === true; + await writeCommandDebugArtifact({ outputPath: options.debugOut, format: globalOptions.format, @@ -405,16 +434,21 @@ export function registerToolsCommands(program: Command): void { params, }, target: targetSummary, - outcome: inspectorRenderError && !inspectorRenderSkipped - ? { - status: "error", - error: inspectorRenderError, - result: debugOutputPayload, - } - : { - status: "success", - result: debugOutputPayload, - }, + outcome: + inspectorRenderError && + (!inspectorRenderSkipped || requireRender) + ? { + status: "error", + error: + inspectorRenderSkipped && requireRender + ? inspectorRenderIssue ?? inspectorRenderError + : inspectorRenderError, + result: debugOutputPayload, + } + : { + status: "success", + result: debugOutputPayload, + }, snapshot: options.debugOut ? { input: { @@ -452,53 +486,109 @@ export function registerToolsCommands(program: Command): void { if (toolResultError) { setProcessExitCode(1); } - if (inspectorRenderError && !inspectorRenderSkipped) { + if (inspectorRenderError && (!inspectorRenderSkipped || requireRender)) { setProcessExitCode(1); } }); } -function resolveInspectorOpenBrowser( +type InspectorRenderRemediation = + | "open_browser" + | "retry" + | "reconnect_server" + | "none"; + +type InspectorRenderSkippableCode = + | "no_active_client" + | "timeout" + | "disconnected_server" + | "unsupported_in_mode"; + +type InspectorRenderIssue = { + code: InspectorRenderSkippableCode; + message: string; + remediation: InspectorRenderRemediation; + browserUrl?: string; + hasActiveClient?: boolean; + inspectorStarted?: boolean; +}; + +type InspectorRenderErrorClassification = + | { + skippable: true; + code: InspectorRenderSkippableCode; + remediation: InspectorRenderRemediation; + } + | { + skippable: false; + remediation: InspectorRenderRemediation; + }; + +function parseInspectorUiTtyOverride(name: string): boolean | undefined { + const value = process.env[name]?.trim().toLowerCase(); + if (!value) { + return undefined; + } + if (value === "1" || value === "true") { + return true; + } + if (value === "0" || value === "false") { + return false; + } + return undefined; +} + +function resolveInspectorUiStdoutIsTTY(): boolean { + return ( + parseInspectorUiTtyOverride("MCPJAM_CLI_TEST_STDOUT_TTY") ?? + Boolean(process.stdout.isTTY) + ); +} + +function resolveInspectorUiStderrIsTTY(): boolean { + return ( + parseInspectorUiTtyOverride("MCPJAM_CLI_TEST_STDERR_TTY") ?? + Boolean(process.stderr.isTTY) + ); +} + +export function resolveInspectorOpenBrowser( options: Pick, - _globalOptions: GlobalOptions, + environment: { stdoutIsTTY?: boolean } = {}, ): boolean { + const stdoutIsTTY = + environment.stdoutIsTTY ?? resolveInspectorUiStdoutIsTTY(); if (options.attachOnly) { return false; } if (options.open !== undefined) { return options.open; } - return true; -} - -function parseInspectorFrontendUrl( - value: string | undefined, -): string | undefined { - if (value === undefined) { - return undefined; - } - - const frontendUrl = normalizeInspectorFrontendUrl(value); - if (!frontendUrl) { - throw usageError(`Invalid --frontend-url "${value}".`); - } - return frontendUrl; + return stdoutIsTTY; } export function resolveInspectorSkipDiscovery( options: Pick, globalOptions: GlobalOptions, + environment: { openBrowser?: boolean; stdoutIsTTY?: boolean } = {}, ): boolean { + const stdoutIsTTY = + environment.stdoutIsTTY ?? resolveInspectorUiStdoutIsTTY(); + const openBrowser = + environment.openBrowser ?? resolveInspectorOpenBrowser(options, { stdoutIsTTY }); if (typeof options.frontendUrl === "string" && options.frontendUrl.trim()) { return true; } if (options.attachOnly) { return true; } - if (options.open !== false) { + if (openBrowser) { return false; } - if (globalOptions.format === "json" || globalOptions.quiet) { + if (options.open === undefined && !stdoutIsTTY) { + return true; + } + if (options.open === false && (globalOptions.format === "json" || globalOptions.quiet)) { return true; } return false; @@ -537,6 +627,7 @@ function extractInspectorRenderErrorUrls(error: { function createInspectorUiProgressReporter(options: { enabled: boolean; + stderrIsTTY: boolean; }): ((message: string) => void) | undefined { if (!options.enabled) { return undefined; @@ -547,26 +638,32 @@ function createInspectorUiProgressReporter(options: { if (!message || message === lastMessage) { return; } + if (!options.stderrIsTTY && /\(\d+s\)$/.test(message)) { + return; + } lastMessage = message; process.stderr.write(`${message}\n`); }; } function writeInspectorRenderWarning(options: { - error: { code: string; message: string } | undefined; + issue: InspectorRenderIssue | undefined; globalOptions: Pick; render: Record; + required: boolean; skipped: boolean; }): void { - if (!options.skipped || !options.error || options.globalOptions.quiet) { + if (!options.skipped || !options.issue || options.globalOptions.quiet) { return; } process.stderr.write( - `Warning: Inspector UI render skipped: ${options.error.message}\n`, + `${options.required ? "Error" : "Warning"}: Inspector UI render ${ + options.required ? "required but " : "" + }skipped: ${options.issue.message}\n`, ); - if (!isNoActiveClientMessage(options.error)) { + if (options.issue.code !== "no_active_client") { return; } @@ -579,28 +676,69 @@ function writeInspectorRenderWarning(options: { ); } -function isSkippableInspectorRenderError( +function classifyInspectorRenderError( error: { code: string; message: string } | undefined, -): boolean { +): InspectorRenderErrorClassification { if (!error) { - return false; + return { skippable: false, remediation: "none" }; } const code = error.code.toLowerCase(); - if ( - code === "no_active_client" || - code === "timeout" || - code === "disconnected_server" || - code === "unsupported_in_mode" - ) { - return true; + if (code === "no_active_client" || isNoActiveClientMessage(error)) { + return { + skippable: true, + code: "no_active_client", + remediation: "open_browser", + }; + } + if (code === "timeout") { + return { + skippable: true, + code: "timeout", + remediation: "retry", + }; + } + if (code === "disconnected_server") { + return { + skippable: true, + code: "disconnected_server", + remediation: "reconnect_server", + }; + } + if (code === "unsupported_in_mode") { + return { + skippable: true, + code: "unsupported_in_mode", + remediation: "none", + }; } - if (code === "operational_error") { - return /no active (browser )?client/i.test(error.message); + return { skippable: false, remediation: "none" }; +} + +function buildInspectorRenderIssue( + error: { code: string; message: string } | undefined, + render: Record, + classification: InspectorRenderErrorClassification, +): InspectorRenderIssue | undefined { + if (!error || !classification.skippable) { + return undefined; } - return false; + return { + code: classification.code, + message: error.message, + remediation: classification.remediation, + ...(typeof render.browserUrl === "string" + ? { browserUrl: render.browserUrl } + : {}), + ...(typeof render.hasActiveClient === "boolean" + ? { hasActiveClient: render.hasActiveClient } + : {}), + ...(typeof render.inspectorStarted === "boolean" + ? { inspectorStarted: render.inspectorStarted } + : {}), + }; } function isNoActiveClientMessage(error: { code: string; message: string }) { @@ -612,7 +750,11 @@ function isNoActiveClientMessage(error: { code: string; message: string }) { function buildCompactInspectorRender( uiResult: Record, - options: { skipped?: boolean } = {}, + options: { + skipped?: boolean; + remediation?: InspectorRenderRemediation; + issue?: InspectorRenderIssue; + } = {}, ): Record { const commands: Record = {}; let hasCommandError = false; @@ -628,11 +770,14 @@ function buildCompactInspectorRender( } } - const topLevelError = - uiResult.status === "error" && isRecord(uiResult.error) - ? options.skipped + const topLevelError = options.skipped + ? options.issue + ? { warning: options.issue } + : uiResult.status === "error" && isRecord(uiResult.error) ? { warning: uiResult.error } - : { error: uiResult.error } + : {} + : uiResult.status === "error" && isRecord(uiResult.error) + ? { error: uiResult.error } : {}; return { @@ -642,6 +787,7 @@ function buildCompactInspectorRender( : hasCommandError || uiResult.status === "error" ? "error" : "rendered", + remediation: options.remediation ?? "none", // Contract metadata: renders target the active client and fresh tabs do not // hydrate this injected state. mode: "active-client", @@ -669,6 +815,20 @@ function buildCompactInspectorRender( }; } +function parseInspectorFrontendUrl( + value: string | undefined, +): string | undefined { + if (value === undefined) { + return undefined; + } + + const frontendUrl = normalizeInspectorFrontendUrl(value); + if (!frontendUrl) { + throw usageError(`Invalid --frontend-url "${value}".`); + } + return frontendUrl; +} + type CompactInspectorCommandStatus = "success" | "error"; function compactInspectorCommandResponse( diff --git a/cli/src/lib/inspector-render.ts b/cli/src/lib/inspector-render.ts index f99c717aa..f914df678 100644 --- a/cli/src/lib/inspector-render.ts +++ b/cli/src/lib/inspector-render.ts @@ -38,6 +38,7 @@ export async function runUiRender(options: { baseUrl?: string; config: MCPServerConfig; frontendUrl?: string; + heartbeatEnabled?: boolean; onProgress?: InspectorUiProgressReporter; openBrowser?: boolean; params: Record; @@ -89,6 +90,7 @@ export async function runUiRender(options: { const renderResult = await runInspectorAppRender({ client, + heartbeatEnabled: options.heartbeatEnabled, onProgress: options.onProgress, params: options.params, renderContext: options.renderContext, @@ -113,6 +115,7 @@ export async function runUiRender(options: { async function runInspectorAppRender(options: { client: InspectorApiClient; + heartbeatEnabled?: boolean; onProgress?: InspectorUiProgressReporter; params: Record; renderContext: AppRenderContext; @@ -179,6 +182,7 @@ async function runInspectorAppRender(options: { async function executeInspectorCommandWithClient( options: { client: InspectorApiClient; + heartbeatEnabled?: boolean; onProgress?: InspectorUiProgressReporter; timeoutMs: number; }, @@ -190,7 +194,7 @@ async function executeInspectorCommandWithClient( let lastResponse: InspectorCommandResponse | undefined; const emitWaitingProgress = (force = false) => { - if (!options.onProgress) { + if (!options.onProgress || !options.heartbeatEnabled) { return; } @@ -210,7 +214,7 @@ async function executeInspectorCommandWithClient( const response = await executeCommandWithProgress( options.client.executeCommand(request), emitWaitingProgress, - Boolean(options.onProgress), + Boolean(options.onProgress && options.heartbeatEnabled), ); lastResponse = response; const retryable = diff --git a/cli/tests/tools-call-ui.test.ts b/cli/tests/tools-call-ui.test.ts index ca65dbdf0..8aa418066 100644 --- a/cli/tests/tools-call-ui.test.ts +++ b/cli/tests/tools-call-ui.test.ts @@ -8,6 +8,7 @@ import path from "node:path"; import test from "node:test"; import type { AddressInfo } from "node:net"; import { + resolveInspectorOpenBrowser, resolveInspectorSkipDiscovery, resolveInspectorStartIfNeeded, } from "../src/commands/tools.js"; @@ -22,7 +23,10 @@ const CLI_ENTRY_PATH = path.join(CLI_DIR, "src", "index.ts"); const INSPECTOR_FRONTEND_HTML = 'MCPJam Inspector
'; -async function runCli(args: string[]): Promise<{ +async function runCli( + args: string[], + options: { env?: NodeJS.ProcessEnv } = {}, +): Promise<{ exitCode: number; stdout: string; stderr: string; @@ -38,6 +42,7 @@ async function runCli(args: string[]): Promise<{ ...process.env, MCPJAM_CLI_DISABLE_BROWSER_OPEN: "1", MCPJAM_TELEMETRY_DISABLED: "1", + ...options.env, }, }, (error, stdout, stderr) => { @@ -102,6 +107,7 @@ async function startMockServer(options: { toolResult?: unknown; toolRpcError?: { code: number; message: string }; failRender?: boolean; + commandDelays?: Partial>; commandErrors?: Partial< Record >; @@ -214,6 +220,10 @@ async function startMockServer(options: { if (request.url === "/api/mcp/command") { const type = body.type as string | undefined; const commandError = type ? options.commandErrors?.[type] : undefined; + const commandDelayMs = type ? options.commandDelays?.[type] : undefined; + if (typeof commandDelayMs === "number" && commandDelayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, commandDelayMs)); + } if (commandError) { response.writeHead(commandError.status ?? 500, { "Content-Type": "application/json", @@ -465,7 +475,7 @@ test("tools call --ui --frontend-url uses the explicit browser URL without probi payload.inspectorRender.browserUrl, `http://localhost:${server.port}/inspector/#app-builder`, ); - assert.equal(payload.inspectorRender.browserOpenRequested, true); + assert.equal(payload.inspectorRender.browserOpenRequested, false); assert.equal( server.requests.some((entry) => entry.method === "GET" && entry.url === "/"), false, @@ -539,13 +549,61 @@ test("tools call --ui --no-open in JSON mode skips render without an active Insp `http://127.0.0.1:${server.port}/#app-builder`, ); assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.remediation, "open_browser"); assert.equal(payload.inspectorRender.hasActiveClient, false); assert.equal(payload.inspectorRender.inspectorStarted, false); assert.equal(payload.inspectorRender.browserOpenRequested, undefined); assert.equal(payload.error, undefined); - assert.equal(payload.warning.code, "OPERATIONAL_ERROR"); + assert.equal(payload.warning.code, "no_active_client"); + assert.equal(payload.warning.remediation, "open_browser"); + assert.equal(payload.warning.browserUrl, payload.inspectorBrowserUrl); + assert.equal(payload.warning.hasActiveClient, false); + assert.equal(payload.warning.inspectorStarted, false); assert.match(payload.warning.message, /no active browser client/i); - assert.equal(payload.inspectorRender.warning.code, "OPERATIONAL_ERROR"); + assert.equal(payload.inspectorRender.warning.code, "no_active_client"); + assert.equal(payload.inspectorRender.warning.remediation, "open_browser"); + assert.equal( + server.requests.some((entry) => entry.url === "/api/mcp/command"), + false, + ); + } finally { + await server.stop(); + } +}); + +test("tools call --ui defaults to no-open in a non-TTY shell", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ hasActiveClient: false, toolResult }); + + try { + const result = await runCli([ + "--format", + "json", + "tools", + "call", + "--ui", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 0, result.stderr); + assert.doesNotMatch(result.stderr, /Inspector App Builder URL:/); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, true); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.browserOpenRequested, undefined); + assert.equal(payload.inspectorRender.remediation, "open_browser"); + assert.equal(payload.warning.code, "no_active_client"); + assert.equal(payload.warning.remediation, "open_browser"); assert.equal( server.requests.some((entry) => entry.url === "/api/mcp/command"), false, @@ -610,7 +668,7 @@ test("tools call --ui --no-open --quiet --format json does not scan nearby front } }); -test("tools call --ui opens by default and may render while waiting for a browser client", async () => { +test("tools call --ui --require-render turns skipped renders into hard errors", async () => { const toolResult = { content: [{ type: "text", text: "view created" }] }; const server = await startMockServer({ hasActiveClient: false, toolResult }); @@ -621,6 +679,7 @@ test("tools call --ui opens by default and may render while waiting for a browse "tools", "call", "--ui", + "--require-render", "--inspector-url", `http://127.0.0.1:${server.port}`, "--url", @@ -631,6 +690,53 @@ test("tools call --ui opens by default and may render while waiting for a browse "{}", ]); + assert.equal(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /Error: Inspector UI render required but skipped/); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, false); + assert.equal(payload.warning, undefined); + assert.equal(payload.error.code, "no_active_client"); + assert.equal(payload.error.remediation, "open_browser"); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.remediation, "open_browser"); + assert.equal(payload.inspectorRender.warning.code, "no_active_client"); + } finally { + await server.stop(); + } +}); + +test("tools call --ui opens by default in a TTY and may render while waiting for a browser client", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ hasActiveClient: false, toolResult }); + + try { + const result = await runCli( + [ + "--format", + "json", + "tools", + "call", + "--ui", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ], + { + env: { + MCPJAM_CLI_TEST_STDOUT_TTY: "1", + MCPJAM_CLI_TEST_STDERR_TTY: "1", + }, + }, + ); + assert.equal(result.exitCode, 0, result.stderr); assert.match(result.stderr, /Inspector App Builder URL:/); assert.match(result.stderr, /Waiting for Inspector browser client/); @@ -651,6 +757,51 @@ test("tools call --ui opens by default and may render while waiting for a browse } }); +test("tools call --ui --open keeps milestone progress but drops the elapsed heartbeat when stderr is not a TTY", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ + commandDelays: { openAppBuilder: 2_100 }, + hasActiveClient: false, + toolResult, + }); + + try { + const result = await runCli( + [ + "--format", + "json", + "tools", + "call", + "--ui", + "--open", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ], + { + env: { + MCPJAM_CLI_TEST_STDERR_TTY: "0", + }, + }, + ); + + assert.equal(result.exitCode, 0, result.stderr); + assert.match(result.stderr, /Inspector App Builder URL:/); + assert.match(result.stderr, /Waiting for Inspector browser client to connect/); + assert.doesNotMatch( + result.stderr, + /Waiting for Inspector browser client to handle .* \(\d+s\)/, + ); + } finally { + await server.stop(); + } +}); + test("tools call --ui --open treats Inspector command timeouts as render skips", async () => { const toolResult = { content: [{ type: "text", text: "view created" }] }; const server = await startMockServer({ @@ -691,7 +842,11 @@ test("tools call --ui --open treats Inspector command timeouts as render skips", assert.equal(payload.success, true); assert.equal(payload.error, undefined); assert.equal(payload.warning.code, "timeout"); + assert.equal(payload.warning.remediation, "retry"); assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.remediation, "retry"); + assert.equal(payload.inspectorRender.warning.code, "timeout"); + assert.equal(payload.inspectorRender.warning.remediation, "retry"); assert.equal( payload.inspectorRender.commands.openAppBuilder.error.code, "timeout", @@ -707,6 +862,104 @@ test("tools call --ui --open treats Inspector command timeouts as render skips", } }); +test("tools call --ui classifies disconnected Inspector clients for agent recovery", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ + commandErrors: { + openAppBuilder: { + code: "disconnected_server", + message: "The Inspector client disconnected before the command completed.", + status: 409, + }, + }, + toolResult, + }); + + try { + const result = await runCli([ + "--timeout", + "250", + "--format", + "json", + "tools", + "call", + "--ui", + "--open", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 0, result.stderr); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, true); + assert.equal(payload.warning.code, "disconnected_server"); + assert.equal(payload.warning.remediation, "reconnect_server"); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.remediation, "reconnect_server"); + assert.equal(payload.inspectorRender.warning.code, "disconnected_server"); + } finally { + await server.stop(); + } +}); + +test("tools call --ui classifies unsupported Inspector modes for agent recovery", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ + commandErrors: { + openAppBuilder: { + code: "unsupported_in_mode", + message: "App Builder is unavailable in the current Inspector mode.", + status: 409, + }, + }, + toolResult, + }); + + try { + const result = await runCli([ + "--timeout", + "250", + "--format", + "json", + "tools", + "call", + "--ui", + "--open", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 0, result.stderr); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, true); + assert.equal(payload.warning.code, "unsupported_in_mode"); + assert.equal(payload.warning.remediation, "none"); + assert.equal(payload.inspectorRender.status, "skipped"); + assert.equal(payload.inspectorRender.remediation, "none"); + assert.equal(payload.inspectorRender.warning.code, "unsupported_in_mode"); + } finally { + await server.stop(); + } +}); + test("tools call --ui --open can discover a nearby dev frontend", async () => { const frontend = await startFrontendServerOnAvailablePort([ 5181, 5182, 5183, 5184, 5185, @@ -907,13 +1160,33 @@ test("tools call --ui rejects reporter output", async () => { assert.match(result.stderr, /--ui cannot be used together with --reporter/); }); +test("tools call --require-render requires --ui", async () => { + const result = await runCli([ + "--format", + "json", + "tools", + "call", + "--require-render", + "--url", + "http://example.test/mcp", + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 2); + assert.match(result.stderr, /--require-render requires --ui/); +}); + test("tools call help lists frontend-url", async () => { const result = await runCli(["tools", "call", "--help"]); assert.equal(result.exitCode, 0, result.stderr); assert.match(result.stdout, /--tool-args-stdin/); assert.match(result.stdout, /--frontend-url /); - assert.match(result.stdout, /default with --ui/); + assert.match(result.stdout, /--require-render/); + assert.match(result.stdout, /default with --ui in a TTY/); }); test("tools call --ui rejects attach-only with open", async () => { @@ -991,7 +1264,24 @@ test("tools call --ui treats no-open as startable but attach-only as strict atta assert.equal(resolveInspectorStartIfNeeded({ attachOnly: true }), false); }); -test("tools call --ui resolves discovery policy from output mode", () => { +test("tools call --ui resolves browser opening from TTY, attach, and explicit flags", () => { + assert.equal(resolveInspectorOpenBrowser({}, { stdoutIsTTY: true }), true); + assert.equal(resolveInspectorOpenBrowser({}, { stdoutIsTTY: false }), false); + assert.equal( + resolveInspectorOpenBrowser({ open: true }, { stdoutIsTTY: false }), + true, + ); + assert.equal( + resolveInspectorOpenBrowser({ open: false }, { stdoutIsTTY: true }), + false, + ); + assert.equal( + resolveInspectorOpenBrowser({ attachOnly: true }, { stdoutIsTTY: true }), + false, + ); +}); + +test("tools call --ui resolves discovery policy from output mode and TTY defaults", () => { const humanOptions = { format: "human" as const, quiet: false, @@ -1002,20 +1292,73 @@ test("tools call --ui resolves discovery policy from output mode", () => { const jsonOptions = { ...humanOptions, format: "json" as const }; const quietOptions = { ...humanOptions, quiet: true }; - assert.equal(resolveInspectorSkipDiscovery({}, humanOptions), false); - assert.equal(resolveInspectorSkipDiscovery({ open: true }, jsonOptions), false); - assert.equal(resolveInspectorSkipDiscovery({}, jsonOptions), false); - assert.equal(resolveInspectorSkipDiscovery({}, quietOptions), false); - assert.equal(resolveInspectorSkipDiscovery({ open: false }, jsonOptions), true); - assert.equal(resolveInspectorSkipDiscovery({ open: false }, quietOptions), true); - assert.equal(resolveInspectorSkipDiscovery({ attachOnly: true }, humanOptions), true); + assert.equal( + resolveInspectorSkipDiscovery({}, humanOptions, { + openBrowser: true, + stdoutIsTTY: true, + }), + false, + ); + assert.equal( + resolveInspectorSkipDiscovery({ open: true }, jsonOptions, { + openBrowser: true, + stdoutIsTTY: false, + }), + false, + ); + assert.equal( + resolveInspectorSkipDiscovery({}, jsonOptions, { + openBrowser: false, + stdoutIsTTY: false, + }), + true, + ); + assert.equal( + resolveInspectorSkipDiscovery({}, quietOptions, { + openBrowser: false, + stdoutIsTTY: false, + }), + true, + ); + assert.equal( + resolveInspectorSkipDiscovery({ open: false }, jsonOptions, { + openBrowser: false, + stdoutIsTTY: true, + }), + true, + ); + assert.equal( + resolveInspectorSkipDiscovery({ open: false }, quietOptions, { + openBrowser: false, + stdoutIsTTY: true, + }), + true, + ); + assert.equal( + resolveInspectorSkipDiscovery({ attachOnly: true }, humanOptions, { + openBrowser: false, + stdoutIsTTY: true, + }), + true, + ); assert.equal( resolveInspectorSkipDiscovery( { frontendUrl: "http://localhost:5173", open: true }, humanOptions, + { + openBrowser: true, + stdoutIsTTY: true, + }, ), true, ); + assert.equal( + resolveInspectorSkipDiscovery({}, humanOptions, { + openBrowser: false, + stdoutIsTTY: false, + }), + true, + ); }); test("tools call --ui validates render flags before executing the tool", async () => { diff --git a/docs/cli/apps-conformance.mdx b/docs/cli/apps-conformance.mdx index 6853ee168..c705e3d79 100644 --- a/docs/cli/apps-conformance.mdx +++ b/docs/cli/apps-conformance.mdx @@ -169,7 +169,7 @@ mcpjam apps conformance \ Use `tools list` to find UI-capable tools. A tool has interactive UI when its metadata includes `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata`. -Then use `tools call --ui` when you need to inspect the output of one UI-capable tool call. The CLI starts or attaches to the local Inspector backend, opens Inspector by default, connects the target server, focuses App Builder in the active Inspector browser client, injects the already-completed tool result, and requests a UI snapshot. `--inspector-url` is the Inspector backend/API URL; `--frontend-url` is the browser/client URL and skips frontend discovery. Use `--no-open` when browser automation already opened `http://127.0.0.1:6274/#app-builder`; use `--attach-only` to disallow startup, browser opening, and discovery. Default `--ui` and explicit `--open` flows print browser-client wait progress on stderr unless `--quiet` is set. +Then use `tools call --ui` when you need to inspect the output of one UI-capable tool call. The CLI starts or attaches to the local Inspector backend, opens Inspector by default in a TTY, connects the target server, focuses App Builder in the active Inspector browser client, injects the already-completed tool result, and requests a UI snapshot. `--inspector-url` is the Inspector backend/API URL; `--frontend-url` is the browser/client URL and skips frontend discovery. Use `--no-open` when browser automation already opened `http://127.0.0.1:6274/#app-builder`; use `--attach-only` to disallow startup, browser opening, and discovery. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. TTY stderr runs print the App Builder URL and initial wait progress unless `--quiet` is set; the elapsed-seconds heartbeat only appears when stderr is a TTY. ```bash mcpjam tools call \ @@ -190,7 +190,7 @@ mcpjam tools call \ client; opening the URL in a fresh tab later does not hydrate the old render. -If `inspectorRender.status` is `skipped`, the tool call succeeded but Inspector had no active browser client, a render precondition was missing, or the render wait timed out; the envelope includes a top-level `warning`, non-quiet runs print a stderr tip, and the command keeps the tool-call exit code. If the command returns `success: false` with an `error`, check whether the failure came from `result` or from an Inspector render command. +If `inspectorRender.status` is `skipped`, the tool call succeeded but Inspector had no active browser client, a render precondition was missing, or the render wait timed out; the envelope includes a stable root `warning` and `inspectorRender.warning` with one of the stable codes `no_active_client`, `timeout`, `disconnected_server`, or `unsupported_in_mode`, plus an `inspectorRender.remediation` hint. Non-quiet runs print a stderr tip, and the command keeps the tool-call exit code unless `--require-render` is set. If the command returns `success: false` with an `error`, check whether the failure came from `result` or from an Inspector render command. Raw `ui://` resource HTML can be inspected directly: diff --git a/docs/cli/overview.mdx b/docs/cli/overview.mdx index f12f2c8b4..b90d5482f 100644 --- a/docs/cli/overview.mdx +++ b/docs/cli/overview.mdx @@ -151,7 +151,7 @@ mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ mcpjam apps conformance --url https://your-server.com/mcp --access-token $TOKEN # 6. Render one UI-capable tool result in Inspector's App Builder. -# --ui opens Inspector by default; add --no-open if browser automation already opened it. +# --ui opens Inspector by default in a TTY; add --no-open if browser automation already opened it. mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ --tool-name my_app_tool --tool-args @params.json --ui --quiet --format json ``` diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index ee0434b0c..a95e4dfac 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -106,8 +106,9 @@ Uses shared connection flags. | `--expect-success` | Fail when the tool result reports `isError` | | `--reporter ` | `json-summary` or `junit-xml` validation report output | | `--debug-out ` | Write debug artifact to file | -| `--ui` | Attach to Inspector and render the completed tool result in App Builder; opens a browser by default | -| `--open` | Open Inspector in the system browser before rendering (default with `--ui`) | +| `--ui` | Attach to Inspector and render the completed tool result in App Builder; opens a browser by default in a TTY | +| `--require-render` | Treat skipped Inspector renders as errors (requires `--ui`) | +| `--open` | Open Inspector in the system browser before rendering (default with `--ui` in a TTY) | | `--no-open` | Start/use Inspector without opening a system browser | | `--attach-only` | Require an already-running Inspector browser client; do not start or open Inspector | | `--inspector-url ` | Local Inspector backend/API base URL | @@ -121,7 +122,48 @@ Uses shared connection flags. Plus shared connection flags. -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. Skipped renders are emitted as top-level `warning` fields and do not change the tool-call exit code; when not quiet, the CLI also prints a short stderr warning and browser tip. Tool failures, validation failures, and render command errors still exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Human and default `--ui` flows may discover local dev frontends and print browser-client wait progress on stderr unless `--quiet` is set. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders are emitted as a stable root `warning` plus `inspectorRender.warning`, both with the shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Skipped renders keep the tool-call exit code unless `--require-render` is set; tool failures, validation failures, non-skippable render command errors, and `--require-render` skipped renders all exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. TTY stderr runs print the App Builder URL and initial browser-client wait progress unless `--quiet` is set; the elapsed-seconds heartbeat only appears when stderr is a TTY. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. + +### Reading `tools call --ui` output as an agent + +Treat the tool result and the Inspector render as separate outcomes. An exit code of `0` means the tool call succeeded and no hard render error occurred; it does not, by itself, prove the UI rendered. Confirm UI delivery with `inspectorRender.status === "rendered"`. If `inspectorRender.status === "skipped"`, branch on `inspectorRender.remediation` or the stable root `warning.code`. If `--require-render` is set, the same skipped-render issue moves from root `warning` to root `error` and the command exits with code `1`. + +```json +{ + "success": true, + "command": "tools call", + "inspectorUi": true, + "inspectorBrowserUrl": "http://127.0.0.1:6274/#app-builder", + "result": { + "content": [{ "type": "text", "text": "view created" }] + }, + "inspectorRender": { + "status": "skipped", + "remediation": "open_browser", + "mode": "active-client", + "urlHydratesRender": false, + "browserUrl": "http://127.0.0.1:6274/#app-builder", + "hasActiveClient": false, + "inspectorStarted": false, + "warning": { + "code": "no_active_client", + "message": "Inspector has no active browser client. Open the Inspector App Builder URL in your browser, then rerun `tools call --ui`; or pass `--open` to let the CLI open a system browser.", + "remediation": "open_browser", + "browserUrl": "http://127.0.0.1:6274/#app-builder", + "hasActiveClient": false, + "inspectorStarted": false + } + }, + "warning": { + "code": "no_active_client", + "message": "Inspector has no active browser client. Open the Inspector App Builder URL in your browser, then rerun `tools call --ui`; or pass `--open` to let the CLI open a system browser.", + "remediation": "open_browser", + "browserUrl": "http://127.0.0.1:6274/#app-builder", + "hasActiveClient": false, + "inspectorStarted": false + } +} +``` --- diff --git a/docs/cli/tools-resources-prompts.mdx b/docs/cli/tools-resources-prompts.mdx index 60b7d1e2e..e5f13f4a7 100644 --- a/docs/cli/tools-resources-prompts.mdx +++ b/docs/cli/tools-resources-prompts.mdx @@ -45,9 +45,9 @@ mcpjam tools call --url https://your-server.com/mcp --access-token $TOKEN \ --quiet --format json ``` -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default and returns an envelope containing the raw `result`, `inspectorBrowserUrl`, and `inspectorRender` evidence. `inspectorRender.status` is the UI signal: `rendered` means Inspector accepted the render, `skipped` means the tool succeeded but the active browser client, a render precondition, or the render wait was missing, and `error` means a non-recoverable Inspector render command failed. Skipped renders appear as top-level `warning` fields, do not change the tool-call exit code, and print a short stderr warning unless `--quiet` is set. Browser automation can open `inspectorBrowserUrl` itself and pass `--no-open`; `--inspector-url` is the local Inspector backend/API base URL. If you already know the Inspector browser/client URL, pass `--frontend-url ` to use it directly and skip health-advertised frontend checks and local dev port discovery. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns an envelope containing the raw `result`, `inspectorBrowserUrl`, and compact `inspectorRender` evidence. `inspectorRender.status` is the UI signal: `rendered` means Inspector accepted the render, `skipped` means the tool succeeded but the active browser client, a render precondition, or the render wait was missing, and `error` means a non-recoverable Inspector render command failed. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders emit a stable root `warning` plus `inspectorRender.warning` with `code`, `message`, `remediation`, and optional `browserUrl`, `hasActiveClient`, and `inspectorStarted` fields. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Pass `--require-render` when a skipped render should become a hard error instead of a warning. Browser automation can open `inspectorBrowserUrl` itself and pass `--no-open`; `--inspector-url` is the local Inspector backend/API base URL. If you already know the Inspector browser/client URL, pass `--frontend-url ` to use it directly and skip health-advertised frontend checks and local dev port discovery. -For agent/browser automation, either let `--ui` open Inspector automatically, or open `http://127.0.0.1:6274/#app-builder` in the automation browser first and run `tools call --ui --no-open --quiet --format json`. Add `--attach-only` when startup, browser opening, and discovery should be disallowed. Default `--ui` and explicit `--open` flows may discover local dev frontends and print browser-client wait progress on stderr unless `--quiet` is set. The normal JSON output is compact; pass `--debug-out ` for the full render envelope. +For agent/browser automation, either let `--ui` open Inspector automatically in a TTY, or open `http://127.0.0.1:6274/#app-builder` in the automation browser first and run `tools call --ui --no-open --quiet --format json`. Add `--attach-only` when startup, browser opening, and discovery should be disallowed. Agents should confirm `inspectorRender.status === "rendered"` before assuming the UI is visible, and use `inspectorRender.remediation` to recover from skipped renders. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. TTY stderr runs print the App Builder URL and initial wait message unless `--quiet` is set; the elapsed-seconds heartbeat only appears when stderr is a TTY. The normal JSON output is compact; pass `--debug-out ` for the full render envelope. For a local stdio server: diff --git a/mcpjam-inspector/cat-params.json b/mcpjam-inspector/cat-params.json new file mode 100644 index 000000000..e82ecd5c8 --- /dev/null +++ b/mcpjam-inspector/cat-params.json @@ -0,0 +1,3 @@ +{ + "elements": "[{\"type\":\"cameraUpdate\",\"width\":400,\"height\":300,\"x\":215,\"y\":40},{\"type\":\"diamond\",\"id\":\"ear_l\",\"x\":290,\"y\":50,\"width\":55,\"height\":85,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"ear_l_in\",\"x\":302,\"y\":65,\"width\":30,\"height\":50,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffc9c9\"},{\"type\":\"diamond\",\"id\":\"ear_r\",\"x\":415,\"y\":50,\"width\":55,\"height\":85,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"ear_r_in\",\"x\":427,\"y\":65,\"width\":30,\"height\":50,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffc9c9\"},{\"type\":\"ellipse\",\"id\":\"head\",\"x\":280,\"y\":118,\"width\":200,\"height\":178,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"snout\",\"x\":348,\"y\":245,\"width\":64,\"height\":44,\"backgroundColor\":\"#f4f4f4\",\"fillStyle\":\"solid\",\"strokeColor\":\"#c0c0c0\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"eye_l\",\"x\":306,\"y\":173,\"width\":46,\"height\":28,\"backgroundColor\":\"#22c55e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#15803d\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"pupil_l\",\"x\":324,\"y\":167,\"width\":10,\"height\":28,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"shine_l\",\"x\":326,\"y\":170,\"width\":5,\"height\":8,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"eye_r\",\"x\":408,\"y\":173,\"width\":46,\"height\":28,\"backgroundColor\":\"#22c55e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#15803d\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"pupil_r\",\"x\":426,\"y\":167,\"width\":10,\"height\":28,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"shine_r\",\"x\":428,\"y\":170,\"width\":5,\"height\":8,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"nose\",\"x\":372,\"y\":255,\"width\":16,\"height\":12,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ec4899\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"mouth_l\",\"x\":372,\"y\":270,\"width\":-16,\"height\":12,\"points\":[[0,0],[-16,12]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"mouth_r\",\"x\":388,\"y\":270,\"width\":16,\"height\":12,\"points\":[[0,0],[16,12]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"wh_l1\",\"x\":348,\"y\":258,\"width\":-108,\"height\":-6,\"points\":[[0,0],[-108,-6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_l2\",\"x\":348,\"y\":267,\"width\":-108,\"height\":0,\"points\":[[0,0],[-108,0]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_l3\",\"x\":348,\"y\":276,\"width\":-108,\"height\":6,\"points\":[[0,0],[-108,6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r1\",\"x\":412,\"y\":258,\"width\":108,\"height\":-6,\"points\":[[0,0],[108,-6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r2\",\"x\":412,\"y\":267,\"width\":108,\"height\":0,\"points\":[[0,0],[108,0]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r3\",\"x\":412,\"y\":276,\"width\":108,\"height\":6,\"points\":[[0,0],[108,6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"cameraUpdate\",\"width\":800,\"height\":600,\"x\":80,\"y\":20},{\"type\":\"ellipse\",\"id\":\"body\",\"x\":230,\"y\":262,\"width\":290,\"height\":200,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"rectangle\",\"id\":\"leg_fl\",\"x\":272,\"y\":415,\"width\":58,\"height\":105,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"rectangle\",\"id\":\"leg_fr\",\"x\":358,\"y\":415,\"width\":58,\"height\":105,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"arrow\",\"id\":\"tail\",\"x\":518,\"y\":325,\"width\":105,\"height\":95,\"points\":[[0,0],[55,45],[95,75],[105,95]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":5},{\"type\":\"text\",\"id\":\"label\",\"x\":350,\"y\":545,\"text\":\"Meow!\",\"fontSize\":24,\"strokeColor\":\"#757575\"}]" +} From ac1fc7f80946d72fbb60671220532fa17f781113 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 15:28:06 -0700 Subject: [PATCH 03/18] skill nits --- skills/mcp-inspector/SKILL.md | 28 +++++++++++++------ .../references/cli-surface-notes.md | 14 +++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/skills/mcp-inspector/SKILL.md b/skills/mcp-inspector/SKILL.md index c38178f96..1d8e27a6d 100644 --- a/skills/mcp-inspector/SKILL.md +++ b/skills/mcp-inspector/SKILL.md @@ -1,7 +1,8 @@ --- -name: mcp-inspector + +## name: mcp-inspector + description: Interpret and use `mcpjam` probe, doctor, OAuth, apps conformance, tools, resources, and prompts output conservatively against MCP 2025-11-25. Use when interacting with MCP servers, executing tools, triaging findings, performing security reviews, deciding whether a CLI finding is real or overstated, or turning inspection output into an engineer-facing report with severity and confidence. ---- # MCPJam CLI Investigation @@ -17,14 +18,19 @@ Use this skill when analyzing MCP server behavior from `mcpjam` or MCP Inspector When the user wants to connect to a server and use it: 1. Probe the server first: `server probe --url --quiet --format json`. - - Use the probe to learn auth posture, resource metadata, authorization-server metadata, and registration strategies before assuming the connected surface is public. + - Use the probe to learn auth posture, resource metadata, authorization-server metadata, and registration strategies before assuming the connected surface is public. 2. If the probe shows `oauth_required`, authenticate with `oauth login --credentials-out ` or run `oauth conformance --credentials-out ` when the task is specifically to test the OAuth flow. 3. Discover tools: `tools list --url --credentials-file --quiet --format json`. - - Tools with `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata` have interactive UI. + - Tools with `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata` have interactive UI. 4. Execute a tool: `tools call --url --tool-name --tool-args --credentials-file `. 5. Execute with UI: `tools call --url --tool-name --tool-args --credentials-file --ui`. - - `--ui` starts Inspector and renders the completed result in App Builder. - - Use `--ui` only when the tool has UI metadata or the user explicitly asks to see UI. + - `--ui` starts or attaches to the local Inspector backend and renders the completed result in App Builder. + - In non-TTY, agent, and CI runs, `--ui` does not open a browser by default. Pass `--open` when the CLI should open App Builder itself. + - Use `--no-open` when browser automation already opened Inspector App Builder. Use `--attach-only` when startup, browser opening, and discovery must all be disallowed. + - `no_active_client` means the Inspector backend may be running but no browser client is attached. If manual recovery is needed, use `mcpjam inspector open`, not `mcpjam inspector start`. + - Treat UI success as `inspectorRender.status === "rendered"`, not exit code `0` alone. If the render is `skipped`, branch on `inspectorRender.remediation` or the stable root `warning.code`. + - Use `--require-render` when the UI render itself is the deliverable and a skipped render should fail the command. + - Use `--ui` only when the tool has UI metadata or the user explicitly asks to see UI. When the user asks to investigate, audit, or triage, use the Investigation workflow below. @@ -46,6 +52,10 @@ When the user asks to investigate, audit, or triage, use the Investigation workf 6. If the output came from `server doctor` or a `--debug-out` artifact, split it into primary command evidence, probe evidence, and connected-sweep evidence. 7. If the claim is specifically about MCP Apps tool metadata or `ui://` resources, start with `apps conformance --quiet --format json` before dropping to `tools list` or `resources read`. 8. If the claim is about a tool result rendering in Inspector, use `tools call --tool-name --tool-args --ui --quiet --format json`. + - In non-TTY runs, add `--open` if no Inspector browser client is already attached. + - If browser automation already opened `http://127.0.0.1:6274/#app-builder`, add `--no-open`. + - Confirm UI delivery with `inspectorRender.status === "rendered"`. Treat `inspectorRender.remediation` and stable skipped-render `warning.code` values as recovery hints, not MCP tool failures. + - Use `--require-render` when a skipped render should become a hard error instead of a warning. 9. If a field may be CLI-added or SDK-normalized, read `references/cli-surface-notes.md` before concluding anything. 10. If the claim depends on MCP semantics, read `references/mcp-2025-11-25-interpretation.md`. 11. If the task involves security review, read `references/security-best-practices.md` for the full checklist and follow the security review workflow below. @@ -181,8 +191,8 @@ For each claimed security-review finding, return: ## Reference map - `references/cli-surface-notes.md` - Use for command-specific caveats, artifact shapes, local enrichments, merged errors, and normalized empty arrays. +Use for command-specific caveats, artifact shapes, local enrichments, merged errors, and normalized empty arrays. - `references/mcp-2025-11-25-interpretation.md` - Use for capability, lifecycle, transport, authorization, tools, resources, and prompts interpretation against the latest MCP spec. +Use for capability, lifecycle, transport, authorization, tools, resources, and prompts interpretation against the latest MCP spec. - `references/security-best-practices.md` - Use for security review checks mapped to CLI commands. Covers SSRF, confused deputy, PKCE, token passthrough, scope minimization, auth-posture checks, and session security. Source: https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices +Use for security review checks mapped to CLI commands. Covers SSRF, confused deputy, PKCE, token passthrough, scope minimization, auth-posture checks, and session security. Source: [https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices) \ No newline at end of file diff --git a/skills/mcp-inspector/references/cli-surface-notes.md b/skills/mcp-inspector/references/cli-surface-notes.md index 6f956c14a..1e9d10b2c 100644 --- a/skills/mcp-inspector/references/cli-surface-notes.md +++ b/skills/mcp-inspector/references/cli-surface-notes.md @@ -113,8 +113,20 @@ If a higher-priority surface contradicts a lower-priority summary, trust the hig - Good for checking argument validation, result shape, and execution failures. - Without `--ui`, the command returns the raw tool result. -- With `--ui`, the command executes the tool once, starts or attaches to the local Inspector, connects the server, opens App Builder, injects the already-completed tool result through `renderToolResult`, and then requests a snapshot. +- With `--ui`, the command executes the tool once, starts or attaches to the local Inspector backend, connects the server, opens App Builder when a browser client is available or `--open` is passed, injects the already-completed tool result through `renderToolResult`, and then requests a snapshot. +- In non-TTY runs, `--ui` does not open a browser by default. Add `--open` when the CLI should open App Builder itself. +- `--open` can start Inspector if needed and then open the browser. The manual recovery command for "bring up Inspector and App Builder" is `mcpjam inspector open`; `mcpjam inspector start` starts only the backend process. +- `no_active_client` means no Inspector browser client is attached. It does not necessarily mean the Inspector backend is down. - `inspectorRender` is UI command-bus evidence, not a second server-side tool call. A render failure can coexist with a successful `result`. +- Treat UI success as `inspectorRender.status === "rendered"`, not exit code `0` alone. +- `inspectorRender.status: "skipped"` means the tool succeeded but the UI render did not complete. The envelope includes a stable root `warning` plus `inspectorRender.warning` with shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. +- Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. +- Stable skipped-render remediations are: + - `open_browser` for `no_active_client` + - `retry` for `timeout` + - `reconnect_server` for `disconnected_server` + - `none` for `unsupported_in_mode` +- Use `--require-render` when the UI render itself is the deliverable and skipped renders should fail the command. - `success: false` with an `error` from `inspectorRender` means the Inspector render path failed. Check the individual `openAppBuilder`, `setAppContext`, `renderToolResult`, and `snapshot` responses before blaming the MCP server. - Large tool results can appear in multiple places, such as `result`, `inspectorRender.renderToolResult.result`, and `inspectorRender.snapshot.result.toolOutput`. Summarize large duplicated payloads. - Distinguish: From d3f3773e0b986d8b99c286bbdf98325e633d9549 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 15:31:49 -0700 Subject: [PATCH 04/18] nit --- .../client/src/components/ui-playground/AppBuilderTab.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/mcpjam-inspector/client/src/components/ui-playground/AppBuilderTab.tsx b/mcpjam-inspector/client/src/components/ui-playground/AppBuilderTab.tsx index d3c7f123a..23dc0c73d 100644 --- a/mcpjam-inspector/client/src/components/ui-playground/AppBuilderTab.tsx +++ b/mcpjam-inspector/client/src/components/ui-playground/AppBuilderTab.tsx @@ -523,6 +523,7 @@ export function AppBuilderTab({ // If a specific tool is selected, detect its protocol if (selectedTool) { const tool = tools[selectedTool]; + if (!tool) return; const uiType = detectUiTypeFromTool(tool); if (uiType === UIType.OPENAI_SDK_AND_MCP_APPS) { // Tool supports both protocols - only set default if no stored preference From df0da27d64290ed3dae9f34c11171317176ab1b6 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 15:48:38 -0700 Subject: [PATCH 05/18] fix race --- .../client/src/hooks/use-chat-session.ts | 65 +++++++++++++++---- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/mcpjam-inspector/client/src/hooks/use-chat-session.ts b/mcpjam-inspector/client/src/hooks/use-chat-session.ts index dd396074a..cec704d04 100644 --- a/mcpjam-inspector/client/src/hooks/use-chat-session.ts +++ b/mcpjam-inspector/client/src/hooks/use-chat-session.ts @@ -880,6 +880,18 @@ function shouldForkChatSession( ); } +function areAuthHeadersEqual( + a: Record | undefined, + b: Record | undefined +): boolean { + if (a === b) return true; + if (!a || !b) return !a && !b; + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + return aKeys.every((key) => a[key] === b[key]); +} + function isAuthDeniedError(error: unknown): boolean { if (!error || typeof error !== "object") return false; const withStatus = error as { status?: unknown; message?: unknown }; @@ -984,6 +996,10 @@ export function useChatSession({ !!(hostedShareToken || hostedChatboxToken); const guestMode = directGuestMode || sharedGuestMode; const skipNextForkDetectionRef = useRef(false); + const hasResolvedAuthHeadersRef = useRef(false); + const lastResolvedAuthHeadersRef = useRef< + Record | undefined + >(undefined); const pendingSessionHydrationRef = useRef( null ); @@ -1707,12 +1723,14 @@ export function useChatSession({ setIsSessionBootstrapComplete(false); (async () => { let resolved = false; + let resolvedAuthHeaders: Record | undefined; try { const token = await getAccessToken?.(); if (!active) return; if (token) { - setAuthHeaders({ Authorization: `Bearer ${token}` }); + resolvedAuthHeaders = { Authorization: `Bearer ${token}` }; + setAuthHeaders(resolvedAuthHeaders); resolved = true; } } catch { @@ -1727,9 +1745,11 @@ export function useChatSession({ const guestToken = await getGuestBearerToken(); if (!active) return; if (guestToken) { - setAuthHeaders({ Authorization: `Bearer ${guestToken}` }); + resolvedAuthHeaders = { Authorization: `Bearer ${guestToken}` }; + setAuthHeaders(resolvedAuthHeaders); resolved = true; } else { + resolvedAuthHeaders = undefined; setAuthHeaders(undefined); } } else if ( @@ -1742,25 +1762,46 @@ export function useChatSession({ const guestToken = await getGuestBearerToken(); if (!active) return; if (guestToken) { - setAuthHeaders({ Authorization: `Bearer ${guestToken}` }); + resolvedAuthHeaders = { Authorization: `Bearer ${guestToken}` }; + setAuthHeaders(resolvedAuthHeaders); resolved = true; } else { + resolvedAuthHeaders = undefined; setAuthHeaders(undefined); } } else if (!resolved && active) { + resolvedAuthHeaders = undefined; setAuthHeaders(undefined); } - // Reset chat to force new session with updated auth headers + // Only reset chat state when the resolved auth headers actually changed. + // The first bootstrap pass always transitions undefined → resolved, but + // there is no prior session to invalidate (chatSessionId is freshly + // generated, messages are empty, no hydration has run). Resetting here + // would race with state injected during the async resolution — for + // example CLI `tools call --ui` commands that arrive while the guest + // bearer fetch is still in flight, whose injected messages would be + // wiped by setMessages([]). if (active) { - skipNextForkDetectionRef.current = true; - clearPendingSessionHydration(); - setChatSessionId(generateId()); - setMessages([]); - setPersistedSnapshotToolCallIds([]); - syncResumedVersion(null); - syncRestoredToolRenderOverrides({}); - onResetRef.current?.("auth-bootstrap"); + const previousAuthHeaders = lastResolvedAuthHeadersRef.current; + const hasResolvedBefore = hasResolvedAuthHeadersRef.current; + const authHeadersChanged = + hasResolvedBefore && + !areAuthHeadersEqual(previousAuthHeaders, resolvedAuthHeaders); + + if (authHeadersChanged) { + skipNextForkDetectionRef.current = true; + clearPendingSessionHydration(); + setChatSessionId(generateId()); + setMessages([]); + setPersistedSnapshotToolCallIds([]); + syncResumedVersion(null); + syncRestoredToolRenderOverrides({}); + onResetRef.current?.("auth-bootstrap"); + } + + hasResolvedAuthHeadersRef.current = true; + lastResolvedAuthHeadersRef.current = resolvedAuthHeaders; setIsSessionBootstrapComplete(true); } })(); From bc88b0ddf5b112843b0a4266d920b18ed718cc7c Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 16:06:55 -0700 Subject: [PATCH 06/18] nit --- docs/inspector/skills.mdx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/inspector/skills.mdx b/docs/inspector/skills.mdx index 9e31121f3..1f55c8ea7 100644 --- a/docs/inspector/skills.mdx +++ b/docs/inspector/skills.mdx @@ -50,6 +50,20 @@ You can upload skill folders directly from the Skills tab. The upload dialog sup Uploading a skill with a name that already exists will be rejected. Delete the existing skill first if you need to replace it. +## Updating Skills + +To pull the latest version of the `mcp-inspector` skill — which contains MCPJam's guidance for interpreting probe, doctor, OAuth, and conformance output — run: + +```bash +npx skills update mcp-inspector +``` + +To update every installed skill at once: + +```bash +npx skills update +``` + ## Using Skills ### Progressive Disclosure From 0209adf0c1e8fd35f135966e5ddd31810b49ba46 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 16:09:55 -0700 Subject: [PATCH 07/18] rm --- mcpjam-inspector/cat-params.json | 3 --- mcpjam-inspector/dog-params.json | 3 --- 2 files changed, 6 deletions(-) delete mode 100644 mcpjam-inspector/cat-params.json delete mode 100644 mcpjam-inspector/dog-params.json diff --git a/mcpjam-inspector/cat-params.json b/mcpjam-inspector/cat-params.json deleted file mode 100644 index e82ecd5c8..000000000 --- a/mcpjam-inspector/cat-params.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "elements": "[{\"type\":\"cameraUpdate\",\"width\":400,\"height\":300,\"x\":215,\"y\":40},{\"type\":\"diamond\",\"id\":\"ear_l\",\"x\":290,\"y\":50,\"width\":55,\"height\":85,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"ear_l_in\",\"x\":302,\"y\":65,\"width\":30,\"height\":50,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffc9c9\"},{\"type\":\"diamond\",\"id\":\"ear_r\",\"x\":415,\"y\":50,\"width\":55,\"height\":85,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"ear_r_in\",\"x\":427,\"y\":65,\"width\":30,\"height\":50,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffc9c9\"},{\"type\":\"ellipse\",\"id\":\"head\",\"x\":280,\"y\":118,\"width\":200,\"height\":178,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"snout\",\"x\":348,\"y\":245,\"width\":64,\"height\":44,\"backgroundColor\":\"#f4f4f4\",\"fillStyle\":\"solid\",\"strokeColor\":\"#c0c0c0\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"eye_l\",\"x\":306,\"y\":173,\"width\":46,\"height\":28,\"backgroundColor\":\"#22c55e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#15803d\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"pupil_l\",\"x\":324,\"y\":167,\"width\":10,\"height\":28,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"shine_l\",\"x\":326,\"y\":170,\"width\":5,\"height\":8,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"eye_r\",\"x\":408,\"y\":173,\"width\":46,\"height\":28,\"backgroundColor\":\"#22c55e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#15803d\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"pupil_r\",\"x\":426,\"y\":167,\"width\":10,\"height\":28,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"shine_r\",\"x\":428,\"y\":170,\"width\":5,\"height\":8,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"nose\",\"x\":372,\"y\":255,\"width\":16,\"height\":12,\"backgroundColor\":\"#ffc9c9\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ec4899\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"mouth_l\",\"x\":372,\"y\":270,\"width\":-16,\"height\":12,\"points\":[[0,0],[-16,12]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"mouth_r\",\"x\":388,\"y\":270,\"width\":16,\"height\":12,\"points\":[[0,0],[16,12]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"wh_l1\",\"x\":348,\"y\":258,\"width\":-108,\"height\":-6,\"points\":[[0,0],[-108,-6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_l2\",\"x\":348,\"y\":267,\"width\":-108,\"height\":0,\"points\":[[0,0],[-108,0]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_l3\",\"x\":348,\"y\":276,\"width\":-108,\"height\":6,\"points\":[[0,0],[-108,6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r1\",\"x\":412,\"y\":258,\"width\":108,\"height\":-6,\"points\":[[0,0],[108,-6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r2\",\"x\":412,\"y\":267,\"width\":108,\"height\":0,\"points\":[[0,0],[108,0]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"arrow\",\"id\":\"wh_r3\",\"x\":412,\"y\":276,\"width\":108,\"height\":6,\"points\":[[0,0],[108,6]],\"endArrowhead\":null,\"strokeColor\":\"#a0a0a0\",\"strokeWidth\":1},{\"type\":\"cameraUpdate\",\"width\":800,\"height\":600,\"x\":80,\"y\":20},{\"type\":\"ellipse\",\"id\":\"body\",\"x\":230,\"y\":262,\"width\":290,\"height\":200,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2},{\"type\":\"rectangle\",\"id\":\"leg_fl\",\"x\":272,\"y\":415,\"width\":58,\"height\":105,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"rectangle\",\"id\":\"leg_fr\",\"x\":358,\"y\":415,\"width\":58,\"height\":105,\"backgroundColor\":\"#e8e8e8\",\"fillStyle\":\"solid\",\"strokeColor\":\"#757575\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"arrow\",\"id\":\"tail\",\"x\":518,\"y\":325,\"width\":105,\"height\":95,\"points\":[[0,0],[55,45],[95,75],[105,95]],\"endArrowhead\":null,\"strokeColor\":\"#757575\",\"strokeWidth\":5},{\"type\":\"text\",\"id\":\"label\",\"x\":350,\"y\":545,\"text\":\"Meow!\",\"fontSize\":24,\"strokeColor\":\"#757575\"}]" -} diff --git a/mcpjam-inspector/dog-params.json b/mcpjam-inspector/dog-params.json deleted file mode 100644 index 0dad093ee..000000000 --- a/mcpjam-inspector/dog-params.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "elements": "[{\"type\":\"cameraUpdate\",\"width\":800,\"height\":600,\"x\":0,\"y\":0},{\"type\":\"text\",\"id\":\"title\",\"x\":285,\"y\":25,\"text\":\"A Very Good Boy\",\"fontSize\":26,\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"lear\",\"x\":292,\"y\":152,\"width\":68,\"height\":105,\"backgroundColor\":\"#92400e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#78350f\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"rear\",\"x\":440,\"y\":152,\"width\":68,\"height\":105,\"backgroundColor\":\"#92400e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#78350f\",\"strokeWidth\":2},{\"type\":\"ellipse\",\"id\":\"head\",\"x\":318,\"y\":153,\"width\":164,\"height\":154,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":3},{\"type\":\"ellipse\",\"id\":\"ilear\",\"x\":308,\"y\":163,\"width\":40,\"height\":68,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":65},{\"type\":\"ellipse\",\"id\":\"irear\",\"x\":452,\"y\":163,\"width\":40,\"height\":68,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":65},{\"type\":\"rectangle\",\"id\":\"body\",\"x\":292,\"y\":283,\"width\":216,\"height\":168,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":3,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"tummy\",\"x\":335,\"y\":303,\"width\":130,\"height\":108,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"transparent\",\"opacity\":70},{\"type\":\"ellipse\",\"id\":\"leyw\",\"x\":340,\"y\":196,\"width\":34,\"height\":34,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"leyp\",\"x\":347,\"y\":202,\"width\":20,\"height\":22,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"leys\",\"x\":352,\"y\":205,\"width\":7,\"height\":7,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"reyw\",\"x\":426,\"y\":196,\"width\":34,\"height\":34,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"reyp\",\"x\":433,\"y\":202,\"width\":20,\"height\":22,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"reys\",\"x\":438,\"y\":205,\"width\":7,\"height\":7,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#ffffff\"},{\"type\":\"ellipse\",\"id\":\"muzzle\",\"x\":358,\"y\":248,\"width\":84,\"height\":58,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":1},{\"type\":\"ellipse\",\"id\":\"nose\",\"x\":374,\"y\":252,\"width\":52,\"height\":32,\"backgroundColor\":\"#1e1e1e\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\"},{\"type\":\"ellipse\",\"id\":\"nosesh\",\"x\":381,\"y\":256,\"width\":11,\"height\":9,\"backgroundColor\":\"#9ca3af\",\"fillStyle\":\"solid\",\"strokeColor\":\"#9ca3af\"},{\"type\":\"ellipse\",\"id\":\"tongue\",\"x\":381,\"y\":276,\"width\":38,\"height\":42,\"backgroundColor\":\"#ec4899\",\"fillStyle\":\"solid\",\"strokeColor\":\"#be185d\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"tcrease\",\"x\":400,\"y\":278,\"width\":0,\"height\":32,\"points\":[[0,0],[0,32]],\"strokeColor\":\"#be185d\",\"strokeWidth\":2,\"endArrowhead\":null},{\"type\":\"rectangle\",\"id\":\"lleg\",\"x\":312,\"y\":435,\"width\":58,\"height\":88,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"lpaw\",\"x\":303,\"y\":507,\"width\":70,\"height\":30,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2},{\"type\":\"rectangle\",\"id\":\"rleg\",\"x\":430,\"y\":435,\"width\":58,\"height\":88,\"backgroundColor\":\"#f59e0b\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2,\"roundness\":{\"type\":3}},{\"type\":\"ellipse\",\"id\":\"rpaw\",\"x\":424,\"y\":507,\"width\":70,\"height\":30,\"backgroundColor\":\"#fcd34d\",\"fillStyle\":\"solid\",\"strokeColor\":\"#92400e\",\"strokeWidth\":2},{\"type\":\"arrow\",\"id\":\"tail\",\"x\":498,\"y\":345,\"width\":85,\"height\":-90,\"points\":[[0,0],[45,-45],[85,-90]],\"strokeColor\":\"#92400e\",\"strokeWidth\":9,\"endArrowhead\":null,\"roughness\":2},{\"type\":\"ellipse\",\"id\":\"bubble\",\"x\":528,\"y\":182,\"width\":118,\"height\":64,\"backgroundColor\":\"#ffffff\",\"fillStyle\":\"solid\",\"strokeColor\":\"#1e1e1e\",\"strokeWidth\":2},{\"type\":\"text\",\"id\":\"woof\",\"x\":554,\"y\":205,\"text\":\"Woof!\",\"fontSize\":22,\"strokeColor\":\"#4a9eed\"},{\"type\":\"arrow\",\"id\":\"conn\",\"x\":520,\"y\":238,\"width\":-22,\"height\":8,\"points\":[[0,0],[-22,8]],\"strokeColor\":\"#1e1e1e\",\"strokeWidth\":2,\"endArrowhead\":null}]" -} From 2300c8caca5f07d266ee6539ea01b65b51e22aea Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 16:22:49 -0700 Subject: [PATCH 08/18] fix skill.md --- skills/mcp-inspector/SKILL.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/skills/mcp-inspector/SKILL.md b/skills/mcp-inspector/SKILL.md index 1d8e27a6d..b165fff4a 100644 --- a/skills/mcp-inspector/SKILL.md +++ b/skills/mcp-inspector/SKILL.md @@ -1,8 +1,7 @@ --- - -## name: mcp-inspector - +name: mcp-inspector description: Interpret and use `mcpjam` probe, doctor, OAuth, apps conformance, tools, resources, and prompts output conservatively against MCP 2025-11-25. Use when interacting with MCP servers, executing tools, triaging findings, performing security reviews, deciding whether a CLI finding is real or overstated, or turning inspection output into an engineer-facing report with severity and confidence. +--- # MCPJam CLI Investigation @@ -191,8 +190,8 @@ For each claimed security-review finding, return: ## Reference map - `references/cli-surface-notes.md` -Use for command-specific caveats, artifact shapes, local enrichments, merged errors, and normalized empty arrays. + Use for command-specific caveats, artifact shapes, local enrichments, merged errors, and normalized empty arrays. - `references/mcp-2025-11-25-interpretation.md` -Use for capability, lifecycle, transport, authorization, tools, resources, and prompts interpretation against the latest MCP spec. + Use for capability, lifecycle, transport, authorization, tools, resources, and prompts interpretation against the latest MCP spec. - `references/security-best-practices.md` -Use for security review checks mapped to CLI commands. Covers SSRF, confused deputy, PKCE, token passthrough, scope minimization, auth-posture checks, and session security. Source: [https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices](https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices) \ No newline at end of file + Use for security review checks mapped to CLI commands. Covers SSRF, confused deputy, PKCE, token passthrough, scope minimization, auth-posture checks, and session security. Source: https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices From afbc9104faa9f266a3ce9bdfbfe70a2fc52653d3 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 21:12:53 -0700 Subject: [PATCH 09/18] initial commit --- cli/src/commands/tools.ts | 45 +- cli/tests/tools-call-ui.test.ts | 42 ++ mcpjam-inspector/client/src/App.tsx | 13 + .../use-chat-session.hosted.test.tsx | 61 ++ .../use-server-state.hosted-oauth.test.tsx | 4 + .../hooks/__tests__/use-server-state.test.tsx | 626 +++++++++++++++++- .../__tests__/use-workspace-state.test.tsx | 67 +- .../client/src/hooks/use-app-state.ts | 4 + .../client/src/hooks/use-chat-session.ts | 34 +- .../client/src/hooks/use-server-state.ts | 258 +++++++- .../client/src/hooks/use-workspace-state.ts | 49 +- .../src/lib/__tests__/session-token.test.ts | 35 + .../client/src/lib/session-token.ts | 47 +- 13 files changed, 1238 insertions(+), 47 deletions(-) diff --git a/cli/src/commands/tools.ts b/cli/src/commands/tools.ts index 9141b4f81..d544fd4b5 100644 --- a/cli/src/commands/tools.ts +++ b/cli/src/commands/tools.ts @@ -363,8 +363,12 @@ export function registerToolsCommands(program: Command): void { }; } - const inspectorRenderClassification = - classifyInspectorRenderError(inspectorRenderError); + const inspectorRenderClassification = classifyInspectorRenderError( + inspectorRenderError, + { + noActiveClientIsSkippable: options.attachOnly !== true, + }, + ); inspectorRenderSkipped = inspectorRenderClassification.skippable; inspectorRenderIssue = buildInspectorRenderIssue( inspectorRenderError, @@ -401,9 +405,7 @@ export function registerToolsCommands(program: Command): void { ...(inspectorRenderError ? inspectorRenderSkipped && !requireRender ? { warning: inspectorRenderIssue ?? inspectorRenderError } - : inspectorRenderSkipped - ? { error: inspectorRenderIssue ?? inspectorRenderError } - : { error: inspectorRenderError } + : { error: inspectorRenderIssue ?? inspectorRenderError } : {}), }; outputPayload = compactOutputPayload; @@ -439,10 +441,7 @@ export function registerToolsCommands(program: Command): void { (!inspectorRenderSkipped || requireRender) ? { status: "error", - error: - inspectorRenderSkipped && requireRender - ? inspectorRenderIssue ?? inspectorRenderError - : inspectorRenderError, + error: inspectorRenderIssue ?? inspectorRenderError, result: debugOutputPayload, } : { @@ -514,15 +513,11 @@ type InspectorRenderIssue = { }; type InspectorRenderErrorClassification = - | { - skippable: true; - code: InspectorRenderSkippableCode; - remediation: InspectorRenderRemediation; - } - | { - skippable: false; - remediation: InspectorRenderRemediation; - }; + { + skippable: boolean; + remediation: InspectorRenderRemediation; + code?: InspectorRenderSkippableCode; + }; function parseInspectorUiTtyOverride(name: string): boolean | undefined { const value = process.env[name]?.trim().toLowerCase(); @@ -678,15 +673,17 @@ function writeInspectorRenderWarning(options: { function classifyInspectorRenderError( error: { code: string; message: string } | undefined, + options: { noActiveClientIsSkippable?: boolean } = {}, ): InspectorRenderErrorClassification { if (!error) { return { skippable: false, remediation: "none" }; } + const noActiveClientIsSkippable = options.noActiveClientIsSkippable ?? true; const code = error.code.toLowerCase(); if (code === "no_active_client" || isNoActiveClientMessage(error)) { return { - skippable: true, + skippable: noActiveClientIsSkippable, code: "no_active_client", remediation: "open_browser", }; @@ -721,7 +718,7 @@ function buildInspectorRenderIssue( render: Record, classification: InspectorRenderErrorClassification, ): InspectorRenderIssue | undefined { - if (!error || !classification.skippable) { + if (!error || !classification.code) { return undefined; } @@ -776,9 +773,11 @@ function buildCompactInspectorRender( : uiResult.status === "error" && isRecord(uiResult.error) ? { warning: uiResult.error } : {} - : uiResult.status === "error" && isRecord(uiResult.error) - ? { error: uiResult.error } - : {}; + : options.issue + ? { error: options.issue } + : uiResult.status === "error" && isRecord(uiResult.error) + ? { error: uiResult.error } + : {}; return { status: diff --git a/cli/tests/tools-call-ui.test.ts b/cli/tests/tools-call-ui.test.ts index 8aa418066..b410d7a16 100644 --- a/cli/tests/tools-call-ui.test.ts +++ b/cli/tests/tools-call-ui.test.ts @@ -1213,6 +1213,48 @@ test("tools call --ui rejects attach-only with open", async () => { ); }); +test("tools call --ui --attach-only keeps missing browser clients as hard errors", async () => { + const toolResult = { content: [{ type: "text", text: "view created" }] }; + const server = await startMockServer({ hasActiveClient: false, toolResult }); + + try { + const result = await runCli([ + "--format", + "json", + "tools", + "call", + "--ui", + "--attach-only", + "--inspector-url", + `http://127.0.0.1:${server.port}`, + "--url", + `http://127.0.0.1:${server.port}/mcp`, + "--tool-name", + "create_view", + "--tool-args", + "{}", + ]); + + assert.equal(result.exitCode, 1, result.stderr); + const payload = JSON.parse(lastJsonLine(result.stdout)) as Record< + string, + any + >; + assert.equal(payload.success, false); + assert.equal(payload.warning, undefined); + assert.equal(payload.error.code, "no_active_client"); + assert.equal(payload.error.remediation, "open_browser"); + assert.equal(payload.inspectorRender.status, "error"); + assert.equal(payload.inspectorRender.error.code, "no_active_client"); + assert.equal( + server.requests.some((entry) => entry.url === "/api/mcp/command"), + false, + ); + } finally { + await server.stop(); + } +}); + test("tools call --ui accepts frontend-url with open", async () => { const toolResult = { content: [{ type: "text", text: "view created" }] }; const server = await startMockServer({ diff --git a/mcpjam-inspector/client/src/App.tsx b/mcpjam-inspector/client/src/App.tsx index 8738dbfba..12fdf30f5 100644 --- a/mcpjam-inspector/client/src/App.tsx +++ b/mcpjam-inspector/client/src/App.tsx @@ -715,6 +715,7 @@ export default function App() { clearLocalFallbackWorkspaceSelection, pendingDashboardOAuth, isCloudSyncActive, + persistRuntimeServerToWorkspaceIfNeeded, } = useAppState({ currentUserId: workOsUser?.id ?? null, hasOrganizations: effectiveOrganizations.length > 0, @@ -731,6 +732,11 @@ export default function App() { workspaceServersRef.current = workspaceServers; const selectedServerRef = useRef(appState.selectedServer); selectedServerRef.current = appState.selectedServer; + const persistRuntimeServerToWorkspaceRef = useRef( + persistRuntimeServerToWorkspaceIfNeeded, + ); + persistRuntimeServerToWorkspaceRef.current = + persistRuntimeServerToWorkspaceIfNeeded; const getInspectorServerState = useCallback((serverName: string) => { const runtimeServer = oauthDebuggerServersRef.current[serverName]; const workspaceServer = workspaceServersRef.current[serverName]; @@ -1288,6 +1294,13 @@ export default function App() { } setSelectedServer(command.payload.serverName); + const runtimeForPersist = serverState.runtimeServer; + if (runtimeForPersist?.connectionStatus === "connected") { + void persistRuntimeServerToWorkspaceRef.current( + command.payload.serverName, + runtimeForPersist, + ); + } } applyNavigation("app-builder", { updateHash: true }); diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsx index 92a186d8e..c0f6c5576 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsx @@ -1,4 +1,5 @@ import { act, renderHook, waitFor } from "@testing-library/react"; +import { generateId } from "ai"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { useChatSession } from "../use-chat-session"; import { useTrafficLogStore } from "@/stores/traffic-log-store"; @@ -258,6 +259,7 @@ describe("useChatSession hosted mode", () => { mockState.convexAuth.isLoading = false; mockState.authFetch.mockReset(); mockState.authFetch.mockResolvedValue(new Response(null, { status: 200 })); + mockState.setMessages.mockReset(); mockState.buildHostedServerRequest.mockReset(); mockState.getAccessToken.mockReset(); mockState.getAccessToken.mockResolvedValue("access-token"); @@ -265,6 +267,8 @@ describe("useChatSession hosted mode", () => { mockState.getGuestBearerToken.mockResolvedValue("guest-token"); mockState.selectedModelId = "anthropic/claude-haiku-4.5"; mockState.latestOnData = undefined; + vi.mocked(generateId).mockReset(); + vi.mocked(generateId).mockReturnValue("chat-session-id"); useTrafficLogStore.getState().clear(); }); @@ -343,6 +347,63 @@ describe("useChatSession hosted mode", () => { unmount(); }); + it("resets the thread when the hosted scope changes under the same auth header", async () => { + vi.mocked(generateId) + .mockImplementationOnce(() => "chat-session-id") + .mockImplementation(() => "chat-session-id-2"); + const onReset = vi.fn(); + + const { result, rerender } = renderHook( + ({ + hostedContext, + }: { + hostedContext: { + workspaceId: string; + selectedServerIds: string[]; + shareToken: string; + }; + }) => + useChatSession({ + selectedServers: ["server-1"], + hostedContext, + onReset, + }), + { + initialProps: { + hostedContext: { + workspaceId: "workspace-1", + selectedServerIds: ["server-id-1"], + shareToken: "share-token-1", + }, + }, + } + ); + + await waitFor(() => { + expect(result.current.isSessionBootstrapComplete).toBe(true); + }); + + expect(result.current.chatSessionId).toBe("chat-session-id"); + + mockState.setMessages.mockClear(); + onReset.mockClear(); + + rerender({ + hostedContext: { + workspaceId: "workspace-2", + selectedServerIds: ["server-id-2"], + shareToken: "share-token-2", + }, + }); + + await waitFor(() => { + expect(result.current.chatSessionId).toBe("chat-session-id-2"); + }); + + expect(mockState.setMessages).toHaveBeenCalledTimes(1); + expect(onReset).toHaveBeenCalledWith("auth-bootstrap"); + }); + it("marks session bootstrap complete only after auth setup finishes", async () => { let resolveAccessToken: (value: string) => void = () => {}; mockState.getAccessToken.mockImplementation( diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.hosted-oauth.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.hosted-oauth.test.tsx index e461d1cae..7e3e635bc 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.hosted-oauth.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.hosted-oauth.test.tsx @@ -147,6 +147,7 @@ function renderHostedServerState( dispatch, isLoading: false, isAuthenticated: true, + hasSignedInUser: true, isAuthLoading: false, isLoadingWorkspaces: false, useLocalFallback: false, @@ -229,6 +230,7 @@ describe("useServerState hosted OAuth callback guards", () => { dispatch: vi.fn(), isLoading: false, isAuthenticated: true, + hasSignedInUser: true, isAuthLoading: false, isLoadingWorkspaces: false, useLocalFallback: false, @@ -282,6 +284,7 @@ describe("useServerState hosted OAuth callback guards", () => { dispatch, isLoading: false, isAuthenticated: true, + hasSignedInUser: true, isAuthLoading: false, isLoadingWorkspaces: false, useLocalFallback: false, @@ -359,6 +362,7 @@ describe("useServerState hosted OAuth callback guards", () => { dispatch: vi.fn(), isLoading: false, isAuthenticated: true, + hasSignedInUser: true, isAuthLoading: false, isLoadingWorkspaces: false, useLocalFallback: false, diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx index 6f40f9a76..a46c9f057 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx @@ -1,6 +1,7 @@ import { act, renderHook, waitFor } from "@testing-library/react"; +import { flushSync } from "react-dom"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { AppState, AppAction } from "@/state/app-types"; +import type { AppState, AppAction, ServerWithName } from "@/state/app-types"; import { CLIENT_CONFIG_SYNC_PENDING_ERROR_MESSAGE } from "@/lib/client-config"; import type { WorkspaceClientConfig } from "@/lib/client-config"; import { useClientConfigStore } from "@/stores/client-config-store"; @@ -18,6 +19,8 @@ const { readStoredOAuthConfigMock, testConnectionMock, mockConvexQuery, + mockCreateServer, + mockUpdateServer, } = vi.hoisted(() => ({ toastError: vi.fn(), toastSuccess: vi.fn(), @@ -29,6 +32,8 @@ const { readStoredOAuthConfigMock: vi.fn(), testConnectionMock: vi.fn(), mockConvexQuery: vi.fn(), + mockCreateServer: vi.fn(), + mockUpdateServer: vi.fn(), })); vi.mock("sonner", () => ({ @@ -85,8 +90,8 @@ vi.mock("@/stores/ui-playground-store", () => ({ vi.mock("../useWorkspaces", () => ({ useServerMutations: () => ({ - createServer: vi.fn(), - updateServer: vi.fn(), + createServer: mockCreateServer, + updateServer: mockUpdateServer, deleteServer: vi.fn(), }), })); @@ -147,6 +152,7 @@ function renderUseServerState( dispatch: (action: AppAction) => void, appState = createAppState(), options?: { + hasSignedInUser?: boolean; isAuthenticated?: boolean; useLocalFallback?: boolean; effectiveWorkspaces?: AppState["workspaces"]; @@ -160,6 +166,7 @@ function renderUseServerState( dispatch, isLoading: false, isAuthenticated: options?.isAuthenticated ?? false, + hasSignedInUser: options?.hasSignedInUser ?? false, isAuthLoading: false, isLoadingWorkspaces: false, useLocalFallback: options?.useLocalFallback ?? true, @@ -226,6 +233,8 @@ describe("useServerState OAuth callback failures", () => { useRegistryOAuthProxy: false, }); mockConvexQuery.mockResolvedValue(null); + mockCreateServer.mockReset(); + mockUpdateServer.mockReset(); }); it("marks the pending server as failed when authorization is denied", async () => { @@ -1153,6 +1162,53 @@ describe("useServerState OAuth callback in-flight dispatch", () => { vi.clearAllMocks(); localStorage.clear(); window.history.replaceState({}, "", "/"); + useClientConfigStore.setState({ + activeWorkspaceId: null, + defaultConfig: null, + savedConfig: undefined, + draftConfig: null, + connectionDefaultsText: "{}", + clientCapabilitiesText: "{}", + clientCapabilitiesError: null, + connectionDefaultsError: null, + isSaving: false, + isDirty: false, + pendingWorkspaceId: null, + pendingSavedConfig: undefined, + isAwaitingRemoteEcho: false, + }); + useHostContextStore.setState({ + activeWorkspaceId: null, + defaultHostContext: {}, + savedHostContext: undefined, + draftHostContext: {}, + hostContextText: "{}", + hostContextError: null, + isSaving: false, + isDirty: false, + pendingWorkspaceId: null, + pendingSavedHostContext: undefined, + isAwaitingRemoteEcho: false, + }); + // This block is not nested under "OAuth callback failures"; restore defaults + // so readStoredOAuthConfig is not a bare vi.fn() returning undefined. + getStoredTokensMock.mockReturnValue(undefined); + testConnectionMock.mockResolvedValue({ + success: true, + initInfo: null, + }); + readStoredOAuthConfigMock.mockReturnValue({ + registryServerId: undefined, + useRegistryOAuthProxy: false, + }); + completeHostedOAuthCallbackMock.mockReset(); + completeHostedOAuthCallbackMock.mockResolvedValue({ + success: false, + error: "Hosted OAuth callback should be mocked per test", + }); + mockConvexQuery.mockResolvedValue(null); + mockCreateServer.mockReset(); + mockUpdateServer.mockReset(); }); it("dispatches CONNECT_REQUEST for the pending server before token exchange completes", async () => { @@ -1209,3 +1265,567 @@ describe("useServerState OAuth callback in-flight dispatch", () => { }); }); }); + +describe("persistRuntimeServerToWorkspaceIfNeeded", () => { + function buildCloudPersistState( + connectionStatus: ServerWithName["connectionStatus"] = "connected", + ): AppState { + const workspaces: AppState["workspaces"] = { + ws_cloud: { + id: "ws_cloud", + name: "Cloud", + servers: {}, + createdAt: new Date(), + updatedAt: new Date(), + isDefault: true, + }, + }; + return { + workspaces, + activeWorkspaceId: "ws_cloud", + servers: { + "rt-server": { + name: "rt-server", + config: { url: "https://runtime.example/mcp" } as any, + lastConnectionTime: new Date(), + connectionStatus, + retryCount: 0, + enabled: true, + useOAuth: false, + }, + }, + selectedServer: "rt-server", + selectedMultipleServers: [], + isMultiSelectMode: false, + }; + } + + beforeEach(() => { + vi.clearAllMocks(); + mockCreateServer.mockReset(); + mockUpdateServer.mockReset(); + useClientConfigStore.setState({ + activeWorkspaceId: null, + defaultConfig: null, + savedConfig: undefined, + draftConfig: null, + connectionDefaultsText: "{}", + clientCapabilitiesText: "{}", + clientCapabilitiesError: null, + connectionDefaultsError: null, + isSaving: false, + isDirty: false, + pendingWorkspaceId: null, + pendingSavedConfig: undefined, + isAwaitingRemoteEcho: false, + }); + useHostContextStore.setState({ + activeWorkspaceId: null, + defaultHostContext: {}, + savedHostContext: undefined, + draftHostContext: {}, + hostContextText: "{}", + hostContextError: null, + isSaving: false, + isDirty: false, + pendingWorkspaceId: null, + pendingSavedHostContext: undefined, + isAwaitingRemoteEcho: false, + }); + mockConvexQuery.mockResolvedValue(null); + }); + + it("persists selected runtime-only connected server", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState("connected"); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: [], + }; + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + mockCreateServer.mockImplementation(async () => { + flatRef.current = [{ _id: "new_srv_id", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + return "new_srv_id"; + }); + + await act(async () => { + const out = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(out).toBe("persisted"); + }); + + expect(mockCreateServer).toHaveBeenCalledTimes(1); + expect(mockUpdateServer).not.toHaveBeenCalled(); + }); + + it("does nothing for guest-like or unsigned state", async () => { + mockCreateServer.mockResolvedValue("id"); + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const { result } = renderUseServerState(dispatch, appState, { + hasSignedInUser: false, + isAuthenticated: true, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + + await act(async () => { + expect( + await result.current.persistRuntimeServerToWorkspaceIfNeeded( + "rt-server", + ), + ).toBe("noop"); + }); + + const { result: r2 } = renderUseServerState(dispatch, appState, { + hasSignedInUser: true, + isAuthenticated: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + await act(async () => { + expect( + await r2.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("noop"); + }); + + const { result: r3 } = renderUseServerState(dispatch, appState, { + hasSignedInUser: true, + isAuthenticated: true, + useLocalFallback: true, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + await act(async () => { + expect( + await r3.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("noop"); + }); + + expect(mockCreateServer).not.toHaveBeenCalled(); + }); + + it("does nothing for missing or non-connected runtime server", async () => { + mockCreateServer.mockResolvedValue("id"); + const dispatch = vi.fn(); + const appState = buildCloudPersistState("connecting"); + const { result } = renderUseServerState(dispatch, appState, { + hasSignedInUser: true, + isAuthenticated: true, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + + await act(async () => { + expect( + await result.current.persistRuntimeServerToWorkspaceIfNeeded( + "rt-server", + ), + ).toBe("noop"); + }); + + for (const status of ["failed", "disconnected", "oauth-flow"] as const) { + const st = buildCloudPersistState(status); + const { result: r } = renderUseServerState(dispatch, st, { + hasSignedInUser: true, + isAuthenticated: true, + useLocalFallback: false, + effectiveWorkspaces: st.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + await act(async () => { + expect( + await r.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("noop"); + }); + } + + const missing = buildCloudPersistState(); + const { result: rm } = renderUseServerState(dispatch, missing, { + hasSignedInUser: true, + isAuthenticated: true, + useLocalFallback: false, + effectiveWorkspaces: missing.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: [], + }); + await act(async () => { + expect( + await rm.current.persistRuntimeServerToWorkspaceIfNeeded("nope"), + ).toBe("noop"); + }); + + expect(mockCreateServer).not.toHaveBeenCalled(); + }); + + it("waits for workspace server snapshot before deciding collision", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: undefined, + }; + + mockCreateServer.mockImplementation(async () => { + flatRef.current = [{ _id: "new", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + return "new"; + }); + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + const done = act(async () => { + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + }); + + expect(mockCreateServer).not.toHaveBeenCalled(); + + flatRef.current = []; + flushSync(() => { + rerender(); + }); + + await done; + + expect(mockCreateServer).toHaveBeenCalledTimes(1); + }); + + it("skips write when same-name saved server appears after waiting", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: undefined, + }; + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + const done = act(async () => { + const out = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(out).toBe("skipped_existing_name"); + }); + + flatRef.current = [{ _id: "existing", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + + await done; + expect(mockCreateServer).not.toHaveBeenCalled(); + }); + + it("clears pending key on failed mutation", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: [], + }; + + mockCreateServer.mockResolvedValueOnce(undefined); + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + await act(async () => { + expect( + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("failed"); + }); + + mockCreateServer.mockReset(); + mockCreateServer.mockImplementation(async () => { + flatRef.current = [{ _id: "n2", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + return "n2"; + }); + + await act(async () => { + expect( + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("persisted"); + }); + + expect(mockCreateServer).toHaveBeenCalledTimes(1); + }); + + it("clears pending key when Convex echo lands", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: [], + }; + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + mockCreateServer.mockImplementation(async () => { + flatRef.current = [{ _id: "echo", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + return "echo"; + }); + + await act(async () => { + expect( + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ).toBe("persisted"); + }); + + await act(async () => { + const followUp = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(followUp).toBe("skipped_existing_name"); + }); + }); + + it("dedupes repeated calls while first persist is in flight", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: [], + }; + let resolveCreate!: (v: string) => void; + mockCreateServer.mockImplementation( + () => + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: true, + hasSignedInUser: true, + isAuthLoading: false, + isLoadingWorkspaces: false, + useLocalFallback: false, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: "ws_cloud", + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + const p1 = act(async () => + result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"), + ); + + await act(async () => { + await Promise.resolve(); + }); + + await act(async () => { + const second = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(second).toBe("pending"); + }); + + expect(mockCreateServer).toHaveBeenCalledTimes(1); + + resolveCreate!("srv1"); + flatRef.current = [{ _id: "srv1", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + + await act(async () => { + await p1; + }); + await act(async () => { + const again = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(again).toBe("skipped_existing_name"); + }); + expect(mockCreateServer).toHaveBeenCalledTimes(1); + }); + + it("waits for auth and workspace readiness before persisting", async () => { + const dispatch = vi.fn(); + const appState = buildCloudPersistState(); + const flatRef: { current: { _id: string; name: string }[] | undefined } = { + current: undefined, + }; + + const readiness = { + isAuthenticated: false, + hasSignedInUser: true, + isAuthLoading: true, + isLoadingWorkspaces: true, + useLocalFallback: false, + effectiveActiveWorkspaceId: "none", + }; + + const { result, rerender } = renderHook(() => + useServerState({ + appState, + dispatch, + isLoading: false, + isAuthenticated: readiness.isAuthenticated, + hasSignedInUser: readiness.hasSignedInUser, + isAuthLoading: readiness.isAuthLoading, + isLoadingWorkspaces: readiness.isLoadingWorkspaces, + useLocalFallback: readiness.useLocalFallback, + effectiveWorkspaces: appState.workspaces, + effectiveActiveWorkspaceId: readiness.effectiveActiveWorkspaceId, + activeWorkspaceServersFlat: flatRef.current, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + }), + ); + + mockCreateServer.mockImplementation(async () => { + flatRef.current = [{ _id: "late_srv", name: "rt-server" }]; + flushSync(() => { + rerender(); + }); + return "late_srv"; + }); + + const done = act(async () => { + const out = + await result.current.persistRuntimeServerToWorkspaceIfNeeded("rt-server"); + expect(out).toBe("persisted"); + }); + + expect(mockCreateServer).not.toHaveBeenCalled(); + + readiness.isAuthenticated = true; + readiness.isAuthLoading = false; + readiness.isLoadingWorkspaces = false; + readiness.effectiveActiveWorkspaceId = "ws_cloud"; + flatRef.current = []; + flushSync(() => { + rerender(); + }); + + await done; + + expect(mockCreateServer).toHaveBeenCalledTimes(1); + }); +}); diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx index b86a82a9a..3655f951b 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-workspace-state.test.tsx @@ -18,6 +18,7 @@ const { updateWorkspaceMock, deleteWorkspaceMock, workspaceQueryState, + workspaceServersQueryState, organizationBillingStatusState, useOrganizationBillingStatusMock, serializeServersForSharingMock, @@ -32,6 +33,10 @@ const { workspaces: undefined as any, isLoading: false, }, + workspaceServersQueryState: { + servers: undefined as any, + isLoading: false, + }, organizationBillingStatusState: { value: undefined as | { @@ -61,8 +66,8 @@ vi.mock("../useWorkspaces", () => ({ deleteWorkspace: deleteWorkspaceMock, }), useWorkspaceServers: () => ({ - servers: undefined, - isLoading: false, + servers: workspaceServersQueryState.servers, + isLoading: workspaceServersQueryState.isLoading, }), })); @@ -208,6 +213,8 @@ describe("useWorkspaceState automatic workspace creation", () => { workspaceQueryState.allWorkspaces = []; workspaceQueryState.workspaces = []; workspaceQueryState.isLoading = false; + workspaceServersQueryState.servers = undefined; + workspaceServersQueryState.isLoading = false; organizationBillingStatusState.value = undefined; useOrganizationBillingStatusMock.mockImplementation( () => organizationBillingStatusState.value, @@ -1754,6 +1761,62 @@ describe("useWorkspaceState automatic workspace creation", () => { expect(result.current.effectiveWorkspaces["convex-workspace-a"]).toBeDefined(); }); + it("merges non-terminal runtime-only local server projections into the matched remote workspace", () => { + const runtimeOnlyServer = { + name: "excalidraw", + config: { url: "https://mcp.excalidraw.com/" } as any, + lastConnectionTime: new Date("2026-01-01T00:00:00.000Z"), + connectionStatus: "connected" as const, + retryCount: 0, + enabled: true, + }; + + workspaceQueryState.allWorkspaces = [ + { + _id: "convex-workspace-a", + name: "Workspace A", + servers: {}, + ownerId: "user-1", + organizationId: "org-owner", + createdAt: 1, + updatedAt: 1, + }, + ]; + workspaceQueryState.workspaces = [...workspaceQueryState.allWorkspaces]; + workspaceServersQueryState.servers = []; + + const appState = { + ...createAppState({ + "workspace-a": createLocalWorkspace("workspace-a", { + name: "Workspace A", + organizationId: "org-owner", + sharedWorkspaceId: "convex-workspace-a", + servers: { + excalidraw: runtimeOnlyServer, + }, + }), + }), + activeWorkspaceId: "workspace-a", + servers: { + excalidraw: runtimeOnlyServer, + }, + selectedServer: "excalidraw", + }; + + const { result } = renderUseWorkspaceState({ + appState, + activeOrganizationId: "org-owner", + }); + + expect(result.current.useLocalFallback).toBe(false); + expect(result.current.effectiveActiveWorkspaceId).toBe("convex-workspace-a"); + expect( + result.current.effectiveWorkspaces["convex-workspace-a"]?.servers[ + "excalidraw" + ], + ).toEqual(runtimeOnlyServer); + }); + it("rejects authenticated client-config saves when the hook unmounts mid-sync", async () => { const savedConfig: WorkspaceConnectionConfigDraft = { version: 1, diff --git a/mcpjam-inspector/client/src/hooks/use-app-state.ts b/mcpjam-inspector/client/src/hooks/use-app-state.ts index db6582bba..23ff3d585 100644 --- a/mcpjam-inspector/client/src/hooks/use-app-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-app-state.ts @@ -27,6 +27,7 @@ export type { ServerWithName } from "@/state/app-types"; export type { EnsureServersReadyResult, ServerUpdateResult, + PersistRuntimeServerResult, } from "./use-server-state"; export interface PendingDashboardOAuthState { @@ -441,6 +442,7 @@ export function useAppState({ dispatch, isLoading, isAuthenticated, + hasSignedInUser: currentUserId != null, isAuthLoading, isLoadingWorkspaces: workspaceState.isLoadingWorkspaces, useLocalFallback: workspaceState.useLocalFallback, @@ -652,6 +654,8 @@ export function useAppState({ serverState.handleConnectWithTokensFromOAuthFlow, handleRefreshTokensFromOAuthFlow: serverState.handleRefreshTokensFromOAuthFlow, + persistRuntimeServerToWorkspaceIfNeeded: + serverState.persistRuntimeServerToWorkspaceIfNeeded, handleSwitchWorkspace, handleCreateWorkspace: workspaceState.handleCreateWorkspace, diff --git a/mcpjam-inspector/client/src/hooks/use-chat-session.ts b/mcpjam-inspector/client/src/hooks/use-chat-session.ts index cec704d04..dcb6c90f6 100644 --- a/mcpjam-inspector/client/src/hooks/use-chat-session.ts +++ b/mcpjam-inspector/client/src/hooks/use-chat-session.ts @@ -892,6 +892,23 @@ function areAuthHeadersEqual( return aKeys.every((key) => a[key] === b[key]); } +type HostedSessionScope = { + workspaceId?: string; + shareToken?: string; + chatboxToken?: string; +}; + +function areHostedSessionScopesEqual( + a: HostedSessionScope, + b: HostedSessionScope +): boolean { + return ( + a.workspaceId === b.workspaceId && + a.shareToken === b.shareToken && + a.chatboxToken === b.chatboxToken + ); +} + function isAuthDeniedError(error: unknown): boolean { if (!error || typeof error !== "object") return false; const withStatus = error as { status?: unknown; message?: unknown }; @@ -1000,6 +1017,11 @@ export function useChatSession({ const lastResolvedAuthHeadersRef = useRef< Record | undefined >(undefined); + const lastResolvedHostedScopeRef = useRef({ + workspaceId: undefined, + shareToken: undefined, + chatboxToken: undefined, + }); const pendingSessionHydrationRef = useRef( null ); @@ -1784,12 +1806,21 @@ export function useChatSession({ // wiped by setMessages([]). if (active) { const previousAuthHeaders = lastResolvedAuthHeadersRef.current; + const previousHostedScope = lastResolvedHostedScopeRef.current; + const currentHostedScope = { + workspaceId: hostedWorkspaceId, + shareToken: hostedShareToken, + chatboxToken: hostedChatboxToken, + }; const hasResolvedBefore = hasResolvedAuthHeadersRef.current; const authHeadersChanged = hasResolvedBefore && !areAuthHeadersEqual(previousAuthHeaders, resolvedAuthHeaders); + const hostedScopeChanged = + hasResolvedBefore && + !areHostedSessionScopesEqual(previousHostedScope, currentHostedScope); - if (authHeadersChanged) { + if (authHeadersChanged || hostedScopeChanged) { skipNextForkDetectionRef.current = true; clearPendingSessionHydration(); setChatSessionId(generateId()); @@ -1802,6 +1833,7 @@ export function useChatSession({ hasResolvedAuthHeadersRef.current = true; lastResolvedAuthHeadersRef.current = resolvedAuthHeaders; + lastResolvedHostedScopeRef.current = currentHostedScope; setIsSessionBootstrapComplete(true); } })(); diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index f76d8a216..9a83abf28 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -362,6 +362,8 @@ interface UseServerStateParams { dispatch: Dispatch; isLoading: boolean; isAuthenticated: boolean; + /** True when a signed-in WorkOS user is present (not guest Convex-only auth). */ + hasSignedInUser: boolean; isAuthLoading: boolean; isLoadingWorkspaces: boolean; useLocalFallback: boolean; @@ -371,6 +373,19 @@ interface UseServerStateParams { logger: LoggerLike; } +export type PersistRuntimeServerResult = + | "noop" + | "pending" + | "persisted" + | "skipped_existing_name" + | "skipped_workspace_servers_unresolved" + | "failed"; + +const WORKSPACE_SERVERS_SNAPSHOT_WAIT_MS = 10_000; +/** Must stay below Vitest's default 30s test timeout so callers can finish after a full wait + margin. */ +const WORKSPACE_SERVER_ECHO_WAIT_MS = 25_000; +const WORKSPACE_SERVERS_POLL_MS = 100; + export interface ServerUpdateResult { ok: boolean; serverName: string; @@ -406,6 +421,7 @@ export function useServerState({ dispatch, isLoading, isAuthenticated, + hasSignedInUser, isAuthLoading, isLoadingWorkspaces, useLocalFallback, @@ -421,6 +437,28 @@ export function useServerState({ deleteServer: convexDeleteServer, } = useServerMutations(); + const hasSignedInUserRef = useRef(hasSignedInUser); + hasSignedInUserRef.current = hasSignedInUser; + const isAuthenticatedRef = useRef(isAuthenticated); + isAuthenticatedRef.current = isAuthenticated; + const isAuthLoadingRef = useRef(isAuthLoading); + isAuthLoadingRef.current = isAuthLoading; + const isLoadingWorkspacesRef = useRef(isLoadingWorkspaces); + isLoadingWorkspacesRef.current = isLoadingWorkspaces; + const useLocalFallbackRef = useRef(useLocalFallback); + useLocalFallbackRef.current = useLocalFallback; + const effectiveActiveWorkspaceIdRef = useRef(effectiveActiveWorkspaceId); + effectiveActiveWorkspaceIdRef.current = effectiveActiveWorkspaceId; + const appStateServersRef = useRef(appState.servers); + appStateServersRef.current = appState.servers; + const activeWorkspaceServersFlatRef = useRef(activeWorkspaceServersFlat); + activeWorkspaceServersFlatRef.current = activeWorkspaceServersFlat; + const persistRuntimeDedupeKeysRef = useRef>(new Set()); + + async function sleep(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); + } + const oauthCallbackHandledRef = useRef(false); const opTokenRef = useRef>(new Map()); const nextOpToken = (name: string) => { @@ -678,11 +716,22 @@ export function useServerState({ serverName: string, serverEntry: ServerWithName, ): Promise => { - if (useLocalFallback || !isAuthenticated || !effectiveActiveWorkspaceId) { + const latestUseLocalFallback = useLocalFallbackRef.current; + const latestIsAuthenticated = isAuthenticatedRef.current; + const latestWorkspaceId = effectiveActiveWorkspaceIdRef.current; + if ( + latestUseLocalFallback || + !latestIsAuthenticated || + !latestWorkspaceId || + latestWorkspaceId === "none" + ) { return undefined; } - const existingServer = activeWorkspaceServersFlat?.find( + const flatSnapshot = + activeWorkspaceServersFlatRef.current ?? activeWorkspaceServersFlat; + + const existingServer = flatSnapshot?.find( (s) => s.name === serverName, ); @@ -710,7 +759,7 @@ export function useServerState({ clientId: serverEntry.oauthFlowProfile?.clientId, oauthResourceUrl: serverEntry.oauthFlowProfile?.resourceUrl || - storedOAuthConfig.resourceUrl, + storedOAuthConfig?.resourceUrl, } as const; try { @@ -723,7 +772,7 @@ export function useServerState({ } const newId = await convexCreateServer({ - workspaceId: effectiveActiveWorkspaceId, + workspaceId: latestWorkspaceId, ...payload, }); return newId as string | undefined; @@ -733,12 +782,14 @@ export function useServerState({ try { if (existingServer) { const newId = await convexCreateServer({ - workspaceId: effectiveActiveWorkspaceId, + workspaceId: latestWorkspaceId, ...payload, }); return newId as string | undefined; } - const retryExisting = activeWorkspaceServersFlat?.find( + const flatRetry = + activeWorkspaceServersFlatRef.current ?? activeWorkspaceServersFlat; + const retryExisting = flatRetry?.find( (s) => s.name === serverName, ); if (retryExisting) { @@ -774,9 +825,6 @@ export function useServerState({ } }, [ - useLocalFallback, - isAuthenticated, - effectiveActiveWorkspaceId, activeWorkspaceServersFlat, convexUpdateServer, convexCreateServer, @@ -784,6 +832,193 @@ export function useServerState({ ], ); + const persistRuntimeServerToWorkspaceIfNeeded = useCallback( + async ( + serverName: string, + runtimeServerOverride?: ServerWithName, + ): Promise => { + if (HOSTED_MODE) { + return "noop"; + } + const resolveRuntime = (): ServerWithName | undefined => + runtimeServerOverride ?? appStateServersRef.current[serverName]; + + let runtime = resolveRuntime(); + if (!runtime) { + return "noop"; + } + if (runtime.connectionStatus !== "connected") { + return "noop"; + } + + const initialWorkspaceKey = + effectiveActiveWorkspaceIdRef.current ?? effectiveActiveWorkspaceId; + const dedupeKey = `${initialWorkspaceKey ?? "pending"}:${serverName}`; + + if (persistRuntimeDedupeKeysRef.current.has(dedupeKey)) { + return "pending"; + } + + persistRuntimeDedupeKeysRef.current.add(dedupeKey); + + const clearDedupeKey = () => { + persistRuntimeDedupeKeysRef.current.delete(dedupeKey); + }; + + try { + let workspaceId: string | null = null; + const readyStarted = Date.now(); + while (Date.now() - readyStarted < WORKSPACE_SERVERS_SNAPSHOT_WAIT_MS) { + if (isAuthLoadingRef.current || isLoadingWorkspacesRef.current) { + await sleep(WORKSPACE_SERVERS_POLL_MS); + continue; + } + + if ( + !hasSignedInUserRef.current || + !isAuthenticatedRef.current || + useLocalFallbackRef.current + ) { + clearDedupeKey(); + return "noop"; + } + + const latestWorkspaceId = effectiveActiveWorkspaceIdRef.current; + if (latestWorkspaceId && latestWorkspaceId !== "none") { + workspaceId = latestWorkspaceId; + break; + } + + await sleep(WORKSPACE_SERVERS_POLL_MS); + } + + if (!workspaceId) { + if ( + !hasSignedInUserRef.current || + !isAuthenticatedRef.current || + useLocalFallbackRef.current + ) { + clearDedupeKey(); + return "noop"; + } + logger.warn( + "persistRuntimeServerToWorkspaceIfNeeded: auth/workspace state did not become ready in time; skipping Convex write", + { serverName }, + ); + clearDedupeKey(); + return "skipped_workspace_servers_unresolved"; + } + + const startedWait = Date.now(); + while (Date.now() - startedWait < WORKSPACE_SERVERS_SNAPSHOT_WAIT_MS) { + const flat = activeWorkspaceServersFlatRef.current; + if (flat !== undefined) { + break; + } + await sleep(WORKSPACE_SERVERS_POLL_MS); + } + + const flatAfterWait = activeWorkspaceServersFlatRef.current; + if (flatAfterWait === undefined) { + logger.warn( + "persistRuntimeServerToWorkspaceIfNeeded: workspace server snapshot still unresolved; skipping Convex write", + { serverName, workspaceId }, + ); + clearDedupeKey(); + return "skipped_workspace_servers_unresolved"; + } + + runtime = resolveRuntime(); + if (!runtime || runtime.connectionStatus !== "connected") { + clearDedupeKey(); + return "noop"; + } + + if (flatAfterWait.some((s) => s.name === serverName)) { + logger.warn( + "persistRuntimeServerToWorkspaceIfNeeded: runtime server not persisted because a saved server with the same name already exists", + { serverName, workspaceId }, + ); + clearDedupeKey(); + return "skipped_existing_name"; + } + + let convexResult: string | undefined; + try { + convexResult = await syncServerToConvex(serverName, runtime); + } catch (syncError) { + logger.error( + "persistRuntimeServerToWorkspaceIfNeeded: syncServerToConvex threw", + { + serverName, + workspaceId, + phase: "syncServerToConvex", + error: + syncError instanceof Error + ? syncError.message + : "Unknown error", + }, + ); + clearDedupeKey(); + return "failed"; + } + + if (convexResult === undefined) { + logger.error( + "persistRuntimeServerToWorkspaceIfNeeded: syncServerToConvex returned no server id", + { + serverName, + workspaceId, + phase: "after_syncServerToConvex", + }, + ); + clearDedupeKey(); + return "failed"; + } + + const echoStarted = Date.now(); + let echoed = false; + while (Date.now() - echoStarted < WORKSPACE_SERVER_ECHO_WAIT_MS) { + const flatEcho = activeWorkspaceServersFlatRef.current; + if (flatEcho?.some((s) => s.name === serverName)) { + echoed = true; + break; + } + await sleep(WORKSPACE_SERVERS_POLL_MS); + } + + if (!echoed) { + logger.warn( + "persistRuntimeServerToWorkspaceIfNeeded: timed out waiting for workspace server echo after persist", + { serverName, workspaceId }, + ); + } + + clearDedupeKey(); + return "persisted"; + } catch (unexpected) { + logger.error( + "persistRuntimeServerToWorkspaceIfNeeded: unexpected error", + { + serverName, + workspaceId, + error: + unexpected instanceof Error + ? unexpected.message + : "Unknown error", + }, + ); + clearDedupeKey(); + return "failed"; + } + }, + [ + effectiveActiveWorkspaceId, + syncServerToConvex, + logger, + ], + ); + const removeServerFromConvex = useCallback( async (serverName: string) => { if (useLocalFallback || !isAuthenticated || !effectiveActiveWorkspaceId) { @@ -1209,7 +1444,9 @@ export function useServerState({ hostedOAuthCallbackContext?.serverName ?? localStorage.getItem("mcp-oauth-pending"); if (earlyPendingName) { - const earlyServer = effectiveServers[earlyPendingName]; + const earlyServer = + latestEffectiveServersRef.current[earlyPendingName] ?? + effectiveServers[earlyPendingName]; if (earlyServer) { dispatch({ type: "CONNECT_REQUEST", @@ -2945,5 +3182,6 @@ export function useServerState({ saveServerConfigWithoutConnecting, handleConnectWithTokensFromOAuthFlow, handleRefreshTokensFromOAuthFlow, + persistRuntimeServerToWorkspaceIfNeeded, }; } diff --git a/mcpjam-inspector/client/src/hooks/use-workspace-state.ts b/mcpjam-inspector/client/src/hooks/use-workspace-state.ts index bd18b0db1..cdb595685 100644 --- a/mcpjam-inspector/client/src/hooks/use-workspace-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-workspace-state.ts @@ -438,15 +438,56 @@ export function useWorkspaceState({ }, [useLocalFallback, appState.workspaces, scopedLocalWorkspaces]); const authenticatedMergedWorkspaces = useMemo((): Record => { + const mergedConvexWorkspaces = Object.fromEntries( + Object.entries(convexWorkspaces).map(([workspaceId, workspace]) => { + const localProjection = Object.values(scopedLocalWorkspaces).find( + (candidate) => candidate.sharedWorkspaceId === workspaceId, + ); + if (!localProjection) { + return [workspaceId, workspace]; + } + + const projectedServers = Object.fromEntries( + Object.entries(localProjection.servers).filter(([serverName]) => { + if (workspace.servers[serverName]) { + return false; + } + + const runtimeStatus = appState.servers[serverName]?.connectionStatus; + return ( + runtimeStatus === "connected" || + runtimeStatus === "connecting" || + runtimeStatus === "oauth-flow" + ); + }), + ); + + if (Object.keys(projectedServers).length === 0) { + return [workspaceId, workspace]; + } + + return [ + workspaceId, + { + ...workspace, + servers: { + ...workspace.servers, + ...projectedServers, + }, + }, + ]; + }), + ); + const workspacesWithoutRemoteMatch = Object.fromEntries( Object.entries(scopedLocalWorkspaces).filter(([localWorkspaceId, workspace]) => { - if (convexWorkspaces[localWorkspaceId]) { + if (mergedConvexWorkspaces[localWorkspaceId]) { return false; } if ( workspace.sharedWorkspaceId && - convexWorkspaces[workspace.sharedWorkspaceId] + mergedConvexWorkspaces[workspace.sharedWorkspaceId] ) { return false; } @@ -456,10 +497,10 @@ export function useWorkspaceState({ ); return { - ...convexWorkspaces, + ...mergedConvexWorkspaces, ...workspacesWithoutRemoteMatch, }; - }, [convexWorkspaces, scopedLocalWorkspaces]); + }, [appState.servers, convexWorkspaces, scopedLocalWorkspaces]); const activeScopedLocalWorkspace = useMemo( () => scopedLocalWorkspaces[appState.activeWorkspaceId], diff --git a/mcpjam-inspector/client/src/lib/__tests__/session-token.test.ts b/mcpjam-inspector/client/src/lib/__tests__/session-token.test.ts index 052395cbe..6213d1ec9 100644 --- a/mcpjam-inspector/client/src/lib/__tests__/session-token.test.ts +++ b/mcpjam-inspector/client/src/lib/__tests__/session-token.test.ts @@ -299,6 +299,41 @@ describe("session-token module", () => { }); }); + it("keeps session auth on absolute loopback API URLs", async () => { + await sessionToken.authFetch("http://127.0.0.1:6274/api/test"); + + expect(global.fetch).toHaveBeenCalledWith( + "http://127.0.0.1:6274/api/test", + { + headers: { + "X-MCP-Session-Auth": "Bearer fetch-token", + }, + }, + ); + }); + + it("does not add session auth to cross-origin Convex HTTP requests", async () => { + await sessionToken.authFetch( + "https://outstanding-fennec-304.convex.site/web/registry/catalog", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + }, + ); + + expect(global.fetch).toHaveBeenCalledWith( + "https://outstanding-fennec-304.convex.site/web/registry/catalog", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + }, + ); + }); + it("user-provided headers override auth headers", async () => { await sessionToken.authFetch("/api/test", { headers: { diff --git a/mcpjam-inspector/client/src/lib/session-token.ts b/mcpjam-inspector/client/src/lib/session-token.ts index 621e1c5bc..7a220f5ae 100644 --- a/mcpjam-inspector/client/src/lib/session-token.ts +++ b/mcpjam-inspector/client/src/lib/session-token.ts @@ -102,10 +102,13 @@ function hasAuthorizationHeader(headers?: HeadersInit): boolean { } function buildAuthFetchInit( + input: RequestInfo | URL, init: RequestInit | undefined, hostedAuthorizationHeader: string | null ): RequestInit { - const sessionHeaders = getAuthHeaders(); + const sessionHeaders = shouldAttachSessionHeaders(input) + ? getAuthHeaders() + : undefined; const hostedHeaders = hostedAuthorizationHeader ? ({ Authorization: hostedAuthorizationHeader } as HeadersInit) : undefined; @@ -202,6 +205,37 @@ export function getAuthHeaders(): HeadersInit { return { "X-MCP-Session-Auth": `Bearer ${token}` }; } +function isLoopbackHostname(hostname: string): boolean { + return ( + hostname === "localhost" || + hostname === "127.0.0.1" || + hostname === "::1" || + hostname === "[::1]" + ); +} + +function shouldAttachSessionHeaders(input: RequestInfo | URL): boolean { + if (HOSTED_MODE) { + return false; + } + + const baseOrigin = + typeof window !== "undefined" ? window.location.origin : "http://localhost"; + + try { + const parsed = + input instanceof URL + ? input + : typeof Request !== "undefined" && input instanceof Request + ? new URL(input.url, baseOrigin) + : new URL(String(input), baseOrigin); + + return isLoopbackHostname(parsed.hostname) && parsed.pathname.startsWith("/api/"); + } catch { + return typeof input === "string" && input.startsWith("/api/"); + } +} + /** * Add token to URL as query parameter. * Required for SSE/EventSource which doesn't support custom headers. @@ -242,7 +276,8 @@ export function addTokenToUrl(url: string): string { /** * Authenticated fetch wrapper. - * Automatically adds session auth headers to all requests. + * Adds local session auth only for loopback `/api/*` requests and hosted auth + * where applicable. * Use this instead of native fetch for API calls. * * @param input - URL or Request object @@ -256,7 +291,7 @@ export async function authFetch( const surface = resolveAuthFetchSurface(input); const hostedAuthHeader = await getHostedAuthorizationHeader(); const callerProvidedAuthorization = hasAuthorizationHeader(init?.headers); - const mergedInit = buildAuthFetchInit(init, hostedAuthHeader); + const mergedInit = buildAuthFetchInit(input, init, hostedAuthHeader); const response = await fetch(input, mergedInit); if ( @@ -292,6 +327,10 @@ export async function authFetch( }); } - const retryInit = buildAuthFetchInit(init, `Bearer ${refreshedGuestToken}`); + const retryInit = buildAuthFetchInit( + input, + init, + `Bearer ${refreshedGuestToken}`, + ); return fetch(input, retryInit); } From 55ef90d478edce08688704b686e599c4d744af21 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 22:01:02 -0700 Subject: [PATCH 10/18] fix bugs --- cli/src/commands/tools.ts | 42 +++++++++++------ cli/src/lib/inspector-render.ts | 1 - docs/cli/reference.mdx | 2 +- docs/cli/tools-resources-prompts.mdx | 2 +- .../client/src/hooks/use-server-state.ts | 21 ++++++++- .../client/src/hooks/use-workspace-state.ts | 47 ++++++++++++------- 6 files changed, 80 insertions(+), 35 deletions(-) diff --git a/cli/src/commands/tools.ts b/cli/src/commands/tools.ts index d544fd4b5..90951aecf 100644 --- a/cli/src/commands/tools.ts +++ b/cli/src/commands/tools.ts @@ -317,6 +317,7 @@ export function registerToolsCommands(program: Command): void { | undefined; let inspectorRenderSkipped = false; let inspectorRenderIssue: InspectorRenderIssue | undefined; + const requireRender = options.requireRender === true; if (options.ui) { const serverName = @@ -380,7 +381,6 @@ export function registerToolsCommands(program: Command): void { remediation: inspectorRenderClassification.remediation, issue: inspectorRenderIssue, }); - const requireRender = options.requireRender === true; const renderFailure = inspectorRenderError && (!inspectorRenderSkipped || requireRender); @@ -424,7 +424,23 @@ export function registerToolsCommands(program: Command): void { }); } - const requireRender = options.requireRender === true; + const renderIsFailure = Boolean( + inspectorRenderError && (!inspectorRenderSkipped || requireRender), + ); + const debugOutcomeError = renderIsFailure + ? (inspectorRenderIssue ?? inspectorRenderError) + : validationFailed + ? { + code: "validation_failed", + message: "Tool call validation failed.", + details: validationResult, + } + : toolResultError + ? { + code: "tool_result_error", + message: "Tool returned an error result.", + } + : undefined; await writeCommandDebugArtifact({ outputPath: options.debugOut, @@ -436,18 +452,16 @@ export function registerToolsCommands(program: Command): void { params, }, target: targetSummary, - outcome: - inspectorRenderError && - (!inspectorRenderSkipped || requireRender) - ? { - status: "error", - error: inspectorRenderIssue ?? inspectorRenderError, - result: debugOutputPayload, - } - : { - status: "success", - result: debugOutputPayload, - }, + outcome: debugOutcomeError + ? { + status: "error", + error: debugOutcomeError, + result: debugOutputPayload, + } + : { + status: "success", + result: debugOutputPayload, + }, snapshot: options.debugOut ? { input: { diff --git a/cli/src/lib/inspector-render.ts b/cli/src/lib/inspector-render.ts index f914df678..8a4addb51 100644 --- a/cli/src/lib/inspector-render.ts +++ b/cli/src/lib/inspector-render.ts @@ -220,7 +220,6 @@ async function executeInspectorCommandWithClient( const retryable = response.status === "error" && (response.error.code === "no_active_client" || - response.error.code === "unsupported_in_mode" || response.error.code === "disconnected_server"); if (!retryable) { return response; diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index a95e4dfac..19c45be27 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -122,7 +122,7 @@ Uses shared connection flags. Plus shared connection flags. -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders are emitted as a stable root `warning` plus `inspectorRender.warning`, both with the shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Skipped renders keep the tool-call exit code unless `--require-render` is set; tool failures, validation failures, non-skippable render command errors, and `--require-render` skipped renders all exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. TTY stderr runs print the App Builder URL and initial browser-client wait progress unless `--quiet` is set; the elapsed-seconds heartbeat only appears when stderr is a TTY. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders are emitted as a stable root `warning` plus `inspectorRender.warning`, both with the shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Skipped renders keep the tool-call exit code unless `--require-render` is set; tool failures, validation failures, non-skippable render command errors, and `--require-render` skipped renders all exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. When `--open` is in effect (default in TTYs, opt-in elsewhere), the App Builder URL and the initial browser-client wait progress are emitted to stderr unless `--quiet` is set, regardless of whether stderr is a TTY; only the elapsed-seconds heartbeat is gated on stderr being a TTY. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. ### Reading `tools call --ui` output as an agent diff --git a/docs/cli/tools-resources-prompts.mdx b/docs/cli/tools-resources-prompts.mdx index e5f13f4a7..eee758653 100644 --- a/docs/cli/tools-resources-prompts.mdx +++ b/docs/cli/tools-resources-prompts.mdx @@ -134,7 +134,7 @@ echo '{"query":"setup guide"}' | mcpjam tools call --url $URL --access-token $TO --tool-name search_docs --tool-args - --quiet --format json ``` -`--tool-args-stdin` is equivalent to `--tool-args -`: +`--tool-args-stdin` is equivalent to `--tool-args -` and cannot be combined with `--tool-args` or `--params` in the same command: ```bash generate-params | mcpjam tools call --url $URL --access-token $TOKEN \ diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index 9a83abf28..43adb61ce 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -853,7 +853,11 @@ export function useServerState({ const initialWorkspaceKey = effectiveActiveWorkspaceIdRef.current ?? effectiveActiveWorkspaceId; - const dedupeKey = `${initialWorkspaceKey ?? "pending"}:${serverName}`; + const frozenWorkspaceId = + initialWorkspaceKey && initialWorkspaceKey !== "none" + ? initialWorkspaceKey + : null; + const dedupeKey = `${frozenWorkspaceId ?? "pending"}:${serverName}`; if (persistRuntimeDedupeKeysRef.current.has(dedupeKey)) { return "pending"; @@ -885,6 +889,13 @@ export function useServerState({ const latestWorkspaceId = effectiveActiveWorkspaceIdRef.current; if (latestWorkspaceId && latestWorkspaceId !== "none") { + if ( + frozenWorkspaceId && + latestWorkspaceId !== frozenWorkspaceId + ) { + clearDedupeKey(); + return "noop"; + } workspaceId = latestWorkspaceId; break; } @@ -928,6 +939,14 @@ export function useServerState({ return "skipped_workspace_servers_unresolved"; } + if ( + effectiveActiveWorkspaceIdRef.current && + effectiveActiveWorkspaceIdRef.current !== workspaceId + ) { + clearDedupeKey(); + return "noop"; + } + runtime = resolveRuntime(); if (!runtime || runtime.connectionStatus !== "connected") { clearDedupeKey(); diff --git a/mcpjam-inspector/client/src/hooks/use-workspace-state.ts b/mcpjam-inspector/client/src/hooks/use-workspace-state.ts index cdb595685..c1e83911a 100644 --- a/mcpjam-inspector/client/src/hooks/use-workspace-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-workspace-state.ts @@ -438,28 +438,36 @@ export function useWorkspaceState({ }, [useLocalFallback, appState.workspaces, scopedLocalWorkspaces]); const authenticatedMergedWorkspaces = useMemo((): Record => { + const activeLocalProjection = + scopedLocalWorkspaces[appState.activeWorkspaceId]; + const activeProjectionRemoteId = + activeLocalProjection?.sharedWorkspaceId ?? null; + const mergedConvexWorkspaces = Object.fromEntries( Object.entries(convexWorkspaces).map(([workspaceId, workspace]) => { - const localProjection = Object.values(scopedLocalWorkspaces).find( - (candidate) => candidate.sharedWorkspaceId === workspaceId, - ); - if (!localProjection) { + if ( + !activeLocalProjection || + activeProjectionRemoteId !== workspaceId + ) { return [workspaceId, workspace]; } const projectedServers = Object.fromEntries( - Object.entries(localProjection.servers).filter(([serverName]) => { - if (workspace.servers[serverName]) { - return false; - } - - const runtimeStatus = appState.servers[serverName]?.connectionStatus; - return ( - runtimeStatus === "connected" || - runtimeStatus === "connecting" || - runtimeStatus === "oauth-flow" - ); - }), + Object.entries(activeLocalProjection.servers).filter( + ([serverName]) => { + if (workspace.servers[serverName]) { + return false; + } + + const runtimeStatus = + appState.servers[serverName]?.connectionStatus; + return ( + runtimeStatus === "connected" || + runtimeStatus === "connecting" || + runtimeStatus === "oauth-flow" + ); + }, + ), ); if (Object.keys(projectedServers).length === 0) { @@ -500,7 +508,12 @@ export function useWorkspaceState({ ...mergedConvexWorkspaces, ...workspacesWithoutRemoteMatch, }; - }, [appState.servers, convexWorkspaces, scopedLocalWorkspaces]); + }, [ + appState.activeWorkspaceId, + appState.servers, + convexWorkspaces, + scopedLocalWorkspaces, + ]); const activeScopedLocalWorkspace = useMemo( () => scopedLocalWorkspaces[appState.activeWorkspaceId], From f714c86ffb7a120328b05963055ee80ca74264e8 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 22:13:35 -0700 Subject: [PATCH 11/18] nits --- mcpjam-inspector/client/src/hooks/use-server-state.ts | 3 ++- mcpjam-inspector/client/src/lib/session-token.ts | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index 43adb61ce..e5363584e 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -947,7 +947,8 @@ export function useServerState({ return "noop"; } - runtime = resolveRuntime(); + runtime = + appStateServersRef.current[serverName] ?? runtimeServerOverride; if (!runtime || runtime.connectionStatus !== "connected") { clearDedupeKey(); return "noop"; diff --git a/mcpjam-inspector/client/src/lib/session-token.ts b/mcpjam-inspector/client/src/lib/session-token.ts index 7a220f5ae..d34be0900 100644 --- a/mcpjam-inspector/client/src/lib/session-token.ts +++ b/mcpjam-inspector/client/src/lib/session-token.ts @@ -214,6 +214,10 @@ function isLoopbackHostname(hostname: string): boolean { ); } +// The session token is a single-process secret for the local CLI/Inspector +// build; only attach it to loopback `/api/*` calls. Non-hosted Inspector is +// not supported behind a public origin — relaxing this would expose the token +// to any reachable client. function shouldAttachSessionHeaders(input: RequestInfo | URL): boolean { if (HOSTED_MODE) { return false; From 9d6e45c123c489e3842239b096e7d3d211c28a22 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 22:30:06 -0700 Subject: [PATCH 12/18] nits --- docs/cli/reference.mdx | 2 +- mcpjam-inspector/client/src/hooks/use-server-state.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index 19c45be27..aa4a0d5f0 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -122,7 +122,7 @@ Uses shared connection flags. Plus shared connection flags. -Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders are emitted as a stable root `warning` plus `inspectorRender.warning`, both with the shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Skipped renders keep the tool-call exit code unless `--require-render` is set; tool failures, validation failures, non-skippable render command errors, and `--require-render` skipped renders all exit nonzero. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. When `--open` is in effect (default in TTYs, opt-in elsewhere), the App Builder URL and the initial browser-client wait progress are emitted to stderr unless `--quiet` is set, regardless of whether stderr is a TTY; only the elapsed-seconds heartbeat is gated on stderr being a TTY. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. +Without `--ui`, `tools call` returns the raw tool result. With `--ui`, it opens Inspector by default in a TTY and returns a compact envelope with `result`, `inspectorBrowserUrl`, and `inspectorRender` status. `inspectorRender.status` is `rendered` when Inspector accepted the render, `skipped` when the tool succeeded but Inspector had no active browser client, an unsatisfied render precondition, or a render timeout, and `error` for non-recoverable render command failures. `inspectorRender.remediation` is always present and is one of `open_browser`, `retry`, `reconnect_server`, or `none`. Skipped renders are emitted as a stable root `warning` plus `inspectorRender.warning`, both with the shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. Skipped renders keep the tool-call exit code unless `--require-render` is set; tool failures, validation failures, non-skippable render command errors, and `--require-render` skipped renders all exit nonzero. `--attach-only` is an exception to the skipped-render rule for `no_active_client`: by default a missing browser client yields `inspectorRender.status = "skipped"` with `inspectorRender.remediation = "open_browser"`, but when `--attach-only` is set, `no_active_client` is treated as non-skippable, surfaces as a root `error` (not a downgraded `warning`), and exits nonzero like other non-skippable render failures. `--inspector-url` points to the Inspector backend/API; pass `--frontend-url` when you already know the browser/client URL and want to skip health-advertised frontend checks and local dev port discovery. Use `--no-open` when browser automation already opened `inspectorBrowserUrl`; use `--attach-only` when startup, browser opening, and discovery should all be disallowed. Default non-TTY `--ui` runs do not open a browser unless `--open` is passed. When `--open` is in effect (default in TTYs, opt-in elsewhere), the App Builder URL and the initial browser-client wait progress are emitted to stderr unless `--quiet` is set, regardless of whether stderr is a TTY; only the elapsed-seconds heartbeat is gated on stderr being a TTY. The Inspector path injects the completed tool result through `renderToolResult`; it does not call the tool a second time. Fresh tabs do not hydrate the injected render state; use the active Inspector client that received the render. Use `--debug-out` for the full render envelope including params and command responses. `--ui` cannot be combined with `--reporter`. ### Reading `tools call --ui` output as an agent diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index e5363584e..43adb61ce 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -947,8 +947,7 @@ export function useServerState({ return "noop"; } - runtime = - appStateServersRef.current[serverName] ?? runtimeServerOverride; + runtime = resolveRuntime(); if (!runtime || runtime.connectionStatus !== "connected") { clearDedupeKey(); return "noop"; From f7d8c4d9b50f0b54f3f5a33edbafa057c1f9b9d4 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 22:46:11 -0700 Subject: [PATCH 13/18] nit --- mcpjam-inspector/client/src/hooks/use-server-state.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index 43adb61ce..54a44c8f6 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -869,8 +869,8 @@ export function useServerState({ persistRuntimeDedupeKeysRef.current.delete(dedupeKey); }; + let workspaceId: string | null = null; try { - let workspaceId: string | null = null; const readyStarted = Date.now(); while (Date.now() - readyStarted < WORKSPACE_SERVERS_SNAPSHOT_WAIT_MS) { if (isAuthLoadingRef.current || isLoadingWorkspacesRef.current) { From 83b0fcf23ea3943b5309d5f286ffd1eff45a3043 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 23:19:14 -0700 Subject: [PATCH 14/18] nit --- .../client/src/hooks/use-server-state.ts | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index 54a44c8f6..de71bfd1b 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -857,7 +857,7 @@ export function useServerState({ initialWorkspaceKey && initialWorkspaceKey !== "none" ? initialWorkspaceKey : null; - const dedupeKey = `${frozenWorkspaceId ?? "pending"}:${serverName}`; + let dedupeKey = `${frozenWorkspaceId ?? "pending"}:${serverName}`; if (persistRuntimeDedupeKeysRef.current.has(dedupeKey)) { return "pending"; @@ -869,6 +869,21 @@ export function useServerState({ persistRuntimeDedupeKeysRef.current.delete(dedupeKey); }; + const rekeyDedupe = (resolvedWorkspaceId: string): boolean => { + const nextKey = `${resolvedWorkspaceId}:${serverName}`; + if (nextKey === dedupeKey) { + return true; + } + if (persistRuntimeDedupeKeysRef.current.has(nextKey)) { + persistRuntimeDedupeKeysRef.current.delete(dedupeKey); + return false; + } + persistRuntimeDedupeKeysRef.current.delete(dedupeKey); + persistRuntimeDedupeKeysRef.current.add(nextKey); + dedupeKey = nextKey; + return true; + }; + let workspaceId: string | null = null; try { const readyStarted = Date.now(); @@ -896,6 +911,9 @@ export function useServerState({ clearDedupeKey(); return "noop"; } + if (!rekeyDedupe(latestWorkspaceId)) { + return "pending"; + } workspaceId = latestWorkspaceId; break; } @@ -947,11 +965,12 @@ export function useServerState({ return "noop"; } - runtime = resolveRuntime(); - if (!runtime || runtime.connectionStatus !== "connected") { + const liveRuntime = appStateServersRef.current[serverName]; + if (!liveRuntime || liveRuntime.connectionStatus !== "connected") { clearDedupeKey(); return "noop"; } + runtime = liveRuntime; if (flatAfterWait.some((s) => s.name === serverName)) { logger.warn( From 911affc03e1f416fbc259b8269bb2d4923d457ea Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 23:42:04 -0700 Subject: [PATCH 15/18] fix test --- .../client/src/hooks/__tests__/use-server-state.test.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx index a46c9f057..ad6038bb0 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx @@ -1799,6 +1799,10 @@ describe("persistRuntimeServerToWorkspaceIfNeeded", () => { }), ); + flushSync(() => { + rerender(); + }); + mockCreateServer.mockImplementation(async () => { flatRef.current = [{ _id: "late_srv", name: "rt-server" }]; flushSync(() => { From 9519d06d3234f3f1e995446bf65c9164699f5759 Mon Sep 17 00:00:00 2001 From: marcelo Date: Wed, 29 Apr 2026 23:53:54 -0700 Subject: [PATCH 16/18] nit --- .../src/lib/inspector-command-handlers.ts | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/mcpjam-inspector/client/src/lib/inspector-command-handlers.ts b/mcpjam-inspector/client/src/lib/inspector-command-handlers.ts index 1aaff32cf..128894bec 100644 --- a/mcpjam-inspector/client/src/lib/inspector-command-handlers.ts +++ b/mcpjam-inspector/client/src/lib/inspector-command-handlers.ts @@ -35,6 +35,41 @@ type InspectorCommandHandler = ( ) => Promise | unknown; const handlers = new Map(); +const handlerWaiters = new Map void>>(); + +const HANDLER_REGISTRATION_WAIT_MS = 2_000; + +function notifyHandlerWaiters(type: InspectorCommandType): void { + const waiters = handlerWaiters.get(type); + if (!waiters || waiters.size === 0) return; + handlerWaiters.delete(type); + for (const notify of waiters) { + notify(); + } +} + +function waitForHandlerRegistration( + type: InspectorCommandType, + timeoutMs: number, +): Promise { + return new Promise((resolve) => { + const waiters = handlerWaiters.get(type) ?? new Set<() => void>(); + handlerWaiters.set(type, waiters); + + const timeout = setTimeout(() => { + waiters.delete(notify); + if (waiters.size === 0) handlerWaiters.delete(type); + resolve(false); + }, timeoutMs); + + const notify = () => { + clearTimeout(timeout); + resolve(true); + }; + + waiters.add(notify); + }); +} export function registerInspectorCommandHandler( type: InspectorCommandType, @@ -42,6 +77,7 @@ export function registerInspectorCommandHandler( ): () => void { const existingHandlers = handlers.get(type) ?? []; handlers.set(type, [...existingHandlers, handler]); + notifyHandlerWaiters(type); return () => { const nextHandlers = @@ -59,7 +95,16 @@ export function registerInspectorCommandHandler( export async function executeInspectorCommand( command: InspectorCommand, ): Promise { - const typeHandlers = handlers.get(command.type) ?? []; + let typeHandlers = handlers.get(command.type) ?? []; + if (typeHandlers.length === 0) { + const registered = await waitForHandlerRegistration( + command.type, + HANDLER_REGISTRATION_WAIT_MS, + ); + if (registered) { + typeHandlers = handlers.get(command.type) ?? []; + } + } if (typeHandlers.length === 0) { return { id: command.id, From e2ab1e87517657141c4fd4de10b3f5cd75ea6446 Mon Sep 17 00:00:00 2001 From: marcelo Date: Thu, 30 Apr 2026 12:05:22 -0700 Subject: [PATCH 17/18] selectedMCPConfig --- .../hooks/__tests__/use-server-state.test.tsx | 87 +++++++++++++++++++ .../client/src/hooks/use-server-state.ts | 17 ++++ 2 files changed, 104 insertions(+) diff --git a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx index ad6038bb0..606fd1a71 100644 --- a/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx +++ b/mcpjam-inspector/client/src/hooks/__tests__/use-server-state.test.tsx @@ -184,6 +184,93 @@ function renderUseServerState( ); } +describe("useServerState effective server projection", () => { + beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + window.history.replaceState({}, "", "/"); + }); + + it("surfaces connected or connecting runtime-only servers", () => { + const appState = createAppState(); + const persistedServer: ServerWithName = { + name: "persisted-server", + config: { + type: "http", + url: "https://persisted.example.com/mcp", + } as any, + lastConnectionTime: new Date(), + connectionStatus: "disconnected", + retryCount: 0, + enabled: true, + }; + const runtimeConnected: ServerWithName = { + name: "runtime-connected", + config: { + type: "http", + url: "https://runtime-connected.example.com/mcp", + } as any, + lastConnectionTime: new Date(), + connectionStatus: "connected", + retryCount: 0, + enabled: true, + }; + const runtimeConnecting: ServerWithName = { + name: "runtime-connecting", + config: { + type: "http", + url: "https://runtime-connecting.example.com/mcp", + } as any, + lastConnectionTime: new Date(), + connectionStatus: "connecting", + retryCount: 0, + enabled: true, + }; + const runtimeFailed: ServerWithName = { + name: "runtime-failed", + config: { + type: "http", + url: "https://runtime-failed.example.com/mcp", + } as any, + lastConnectionTime: new Date(), + connectionStatus: "failed", + retryCount: 0, + enabled: true, + }; + + appState.workspaces.default.servers = { + "persisted-server": persistedServer, + }; + appState.servers = { + "runtime-connected": runtimeConnected, + "runtime-connecting": runtimeConnecting, + "runtime-failed": runtimeFailed, + }; + appState.selectedServer = "runtime-connected"; + + const dispatch = vi.fn(); + const { result } = renderUseServerState(dispatch, appState); + + expect(result.current.workspaceServers).toEqual( + expect.objectContaining({ + "persisted-server": expect.any(Object), + "runtime-connected": runtimeConnected, + "runtime-connecting": runtimeConnecting, + }), + ); + expect(result.current.workspaceServers).not.toHaveProperty( + "runtime-failed", + ); + expect(result.current.selectedMCPConfig).toBe(runtimeConnected.config); + expect(result.current.connectedOrConnectingServerConfigs).toEqual( + expect.objectContaining({ + "runtime-connected": runtimeConnected, + "runtime-connecting": runtimeConnecting, + }), + ); + }); +}); + describe("useServerState OAuth callback failures", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/mcpjam-inspector/client/src/hooks/use-server-state.ts b/mcpjam-inspector/client/src/hooks/use-server-state.ts index de71bfd1b..38f95dc58 100644 --- a/mcpjam-inspector/client/src/hooks/use-server-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-server-state.ts @@ -573,6 +573,23 @@ export function useServerState({ }; } + // Surface runtime-only servers (e.g. registered via the CLI's + // /api/mcp/connect before they have been persisted to a workspace) so they + // participate in selection, status display, and command routing. Without + // this, the App.tsx auto-select effect would override an explicit + // setSelectedServer because effectiveServers[]?.config is + // undefined. + for (const [name, runtime] of Object.entries(appState.servers)) { + if (serversWithRuntime[name]) continue; + if ( + runtime.connectionStatus !== "connected" && + runtime.connectionStatus !== "connecting" + ) { + continue; + } + serversWithRuntime[name] = runtime; + } + return { ...workspace, servers: serversWithRuntime }; }, [effectiveWorkspaces, effectiveActiveWorkspaceId, appState.servers]); From d4f5d8023996cec0a70a8ca3e87b291beabfb055 Mon Sep 17 00:00:00 2001 From: marcelo Date: Thu, 30 Apr 2026 12:32:01 -0700 Subject: [PATCH 18/18] skill nits --- skills/mcp-inspector/SKILL.md | 8 ++++++-- skills/mcp-inspector/references/cli-surface-notes.md | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/skills/mcp-inspector/SKILL.md b/skills/mcp-inspector/SKILL.md index b165fff4a..3b2b5ce24 100644 --- a/skills/mcp-inspector/SKILL.md +++ b/skills/mcp-inspector/SKILL.md @@ -21,14 +21,17 @@ When the user wants to connect to a server and use it: 2. If the probe shows `oauth_required`, authenticate with `oauth login --credentials-out ` or run `oauth conformance --credentials-out ` when the task is specifically to test the OAuth flow. 3. Discover tools: `tools list --url --credentials-file --quiet --format json`. - Tools with `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata` have interactive UI. + - For a specific tool, check `toolsMetadata.._meta.ui.resourceUri`, `toolsMetadata.._meta["ui/resourceUri"]`, or `toolsMetadata.["openai/outputTemplate"]`. 4. Execute a tool: `tools call --url --tool-name --tool-args --credentials-file `. 5. Execute with UI: `tools call --url --tool-name --tool-args --credentials-file --ui`. - `--ui` starts or attaches to the local Inspector backend and renders the completed result in App Builder. - In non-TTY, agent, and CI runs, `--ui` does not open a browser by default. Pass `--open` when the CLI should open App Builder itself. - - Use `--no-open` when browser automation already opened Inspector App Builder. Use `--attach-only` when startup, browser opening, and discovery must all be disallowed. + - `--open` opens a system browser URL; it does not attach an already-controlled automation browser or make fresh tabs hydrate an injected render. Use `--no-open` when browser automation already opened Inspector App Builder. Use `--attach-only` when startup, browser opening, and discovery must all be disallowed. - `no_active_client` means the Inspector backend may be running but no browser client is attached. If manual recovery is needed, use `mcpjam inspector open`, not `mcpjam inspector start`. + - `unknown_server` in the root `error.code` or an `inspectorRender.commands.*.error.code` means Inspector could not match the requested server. If the message says App Builder is focused on another server, retry with `--server-name `. - Treat UI success as `inspectorRender.status === "rendered"`, not exit code `0` alone. If the render is `skipped`, branch on `inspectorRender.remediation` or the stable root `warning.code`. - Use `--require-render` when the UI render itself is the deliverable and a skipped render should fail the command. + - Do not require external screenshots as proof of render success; iframe/canvas content can defeat browser snapshot tools. Prefer `inspectorRender.status`, command responses, and snapshot evidence. - Use `--ui` only when the tool has UI metadata or the user explicitly asks to see UI. When the user asks to investigate, audit, or triage, use the Investigation workflow below. @@ -52,8 +55,9 @@ When the user asks to investigate, audit, or triage, use the Investigation workf 7. If the claim is specifically about MCP Apps tool metadata or `ui://` resources, start with `apps conformance --quiet --format json` before dropping to `tools list` or `resources read`. 8. If the claim is about a tool result rendering in Inspector, use `tools call --tool-name --tool-args --ui --quiet --format json`. - In non-TTY runs, add `--open` if no Inspector browser client is already attached. - - If browser automation already opened `http://127.0.0.1:6274/#app-builder`, add `--no-open`. + - If browser automation already opened `http://127.0.0.1:6274/#app-builder`, add `--no-open`; `--open` launches a system browser and may not target the automation-controlled client. - Confirm UI delivery with `inspectorRender.status === "rendered"`. Treat `inspectorRender.remediation` and stable skipped-render `warning.code` values as recovery hints, not MCP tool failures. + - If `unknown_server` appears in the root error or command errors and the message names the focused server, retry with `--server-name `. - Use `--require-render` when a skipped render should become a hard error instead of a warning. 9. If a field may be CLI-added or SDK-normalized, read `references/cli-surface-notes.md` before concluding anything. 10. If the claim depends on MCP semantics, read `references/mcp-2025-11-25-interpretation.md`. diff --git a/skills/mcp-inspector/references/cli-surface-notes.md b/skills/mcp-inspector/references/cli-surface-notes.md index 1e9d10b2c..d0d059080 100644 --- a/skills/mcp-inspector/references/cli-surface-notes.md +++ b/skills/mcp-inspector/references/cli-surface-notes.md @@ -108,6 +108,7 @@ If a higher-priority surface contradicts a lower-priority summary, trust the hig - Only `tools` should be treated as server output by default. - `toolsMetadata: {}` means the local cache is empty. It does not mean the server violated MCP. - Tools with `_meta.ui.resourceUri`, deprecated `_meta["ui/resourceUri"]`, or `openai/outputTemplate` in `toolsMetadata` have interactive UI. Use `tools call --ui` to render those tool results in Inspector. +- For a specific tool, inspect metadata at `toolsMetadata.`. UI-capable metadata can appear at `toolsMetadata.._meta.ui.resourceUri`, deprecated `toolsMetadata.._meta["ui/resourceUri"]`, or `toolsMetadata.["openai/outputTemplate"]`. ### `tools call` @@ -115,10 +116,12 @@ If a higher-priority surface contradicts a lower-priority summary, trust the hig - Without `--ui`, the command returns the raw tool result. - With `--ui`, the command executes the tool once, starts or attaches to the local Inspector backend, connects the server, opens App Builder when a browser client is available or `--open` is passed, injects the already-completed tool result through `renderToolResult`, and then requests a snapshot. - In non-TTY runs, `--ui` does not open a browser by default. Add `--open` when the CLI should open App Builder itself. -- `--open` can start Inspector if needed and then open the browser. The manual recovery command for "bring up Inspector and App Builder" is `mcpjam inspector open`; `mcpjam inspector start` starts only the backend process. +- `--open` can start Inspector if needed and then open a system browser. It does not attach an external automation browser or hydrate fresh tabs with an already injected render. If browser automation owns the client, open App Builder there first and pass `--no-open`. The manual recovery command for "bring up Inspector and App Builder" is `mcpjam inspector open`; `mcpjam inspector start` starts only the backend process. - `no_active_client` means no Inspector browser client is attached. It does not necessarily mean the Inspector backend is down. +- `unknown_server` in the root error or an `inspectorRender.commands.*.error.code` is a hard render failure. If the message says App Builder is focused on another server, retry with `--server-name ` so the CLI request matches the active Inspector server name. - `inspectorRender` is UI command-bus evidence, not a second server-side tool call. A render failure can coexist with a successful `result`. - Treat UI success as `inspectorRender.status === "rendered"`, not exit code `0` alone. +- External browser screenshot tools are optional verification only. Inspector UI content may be inside iframe/canvas surfaces that generic snapshots cannot capture; prefer `inspectorRender.status`, command responses, and the returned `snapshot` evidence. - `inspectorRender.status: "skipped"` means the tool succeeded but the UI render did not complete. The envelope includes a stable root `warning` plus `inspectorRender.warning` with shape `{ code, message, remediation, browserUrl?, hasActiveClient?, inspectorStarted? }`. - Stable skipped-render codes are `no_active_client`, `timeout`, `disconnected_server`, and `unsupported_in_mode`. - Stable skipped-render remediations are: