diff --git a/cli/src/commands/tools.ts b/cli/src/commands/tools.ts index 9141b4f81..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 = @@ -363,8 +364,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, @@ -376,7 +381,6 @@ export function registerToolsCommands(program: Command): void { remediation: inspectorRenderClassification.remediation, issue: inspectorRenderIssue, }); - const requireRender = options.requireRender === true; const renderFailure = inspectorRenderError && (!inspectorRenderSkipped || requireRender); @@ -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; @@ -422,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, @@ -434,21 +452,16 @@ export function registerToolsCommands(program: Command): void { params, }, target: targetSummary, - outcome: - inspectorRenderError && - (!inspectorRenderSkipped || requireRender) - ? { - status: "error", - error: - inspectorRenderSkipped && requireRender - ? inspectorRenderIssue ?? inspectorRenderError - : inspectorRenderError, - result: debugOutputPayload, - } - : { - status: "success", - result: debugOutputPayload, - }, + outcome: debugOutcomeError + ? { + status: "error", + error: debugOutcomeError, + result: debugOutputPayload, + } + : { + status: "success", + result: debugOutputPayload, + }, snapshot: options.debugOut ? { input: { @@ -514,15 +527,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 +687,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 +732,7 @@ function buildInspectorRenderIssue( render: Record, classification: InspectorRenderErrorClassification, ): InspectorRenderIssue | undefined { - if (!error || !classification.skippable) { + if (!error || !classification.code) { return undefined; } @@ -776,9 +787,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/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/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/docs/cli/reference.mdx b/docs/cli/reference.mdx index a95e4dfac..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. 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. `--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/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/App.tsx b/mcpjam-inspector/client/src/App.tsx index dee52c9ac..655adb213 100644 --- a/mcpjam-inspector/client/src/App.tsx +++ b/mcpjam-inspector/client/src/App.tsx @@ -745,6 +745,7 @@ export default function App() { clearLocalFallbackWorkspaceSelection, pendingDashboardOAuth, isCloudSyncActive, + persistRuntimeServerToWorkspaceIfNeeded, } = useAppState({ currentUserId: workOsUser?.id ?? null, hasOrganizations: effectiveOrganizations.length > 0, @@ -761,6 +762,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]; @@ -1318,6 +1324,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..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 @@ -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, @@ -177,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(); @@ -226,6 +320,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 +1249,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 +1352,571 @@ 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(), + }, + }), + ); + + flushSync(() => { + rerender(); + }); + + 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..38f95dc58 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) => { @@ -535,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]); @@ -678,11 +733,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 +776,7 @@ export function useServerState({ clientId: serverEntry.oauthFlowProfile?.clientId, oauthResourceUrl: serverEntry.oauthFlowProfile?.resourceUrl || - storedOAuthConfig.resourceUrl, + storedOAuthConfig?.resourceUrl, } as const; try { @@ -723,7 +789,7 @@ export function useServerState({ } const newId = await convexCreateServer({ - workspaceId: effectiveActiveWorkspaceId, + workspaceId: latestWorkspaceId, ...payload, }); return newId as string | undefined; @@ -733,12 +799,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 +842,6 @@ export function useServerState({ } }, [ - useLocalFallback, - isAuthenticated, - effectiveActiveWorkspaceId, activeWorkspaceServersFlat, convexUpdateServer, convexCreateServer, @@ -784,6 +849,231 @@ 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 frozenWorkspaceId = + initialWorkspaceKey && initialWorkspaceKey !== "none" + ? initialWorkspaceKey + : null; + let dedupeKey = `${frozenWorkspaceId ?? "pending"}:${serverName}`; + + if (persistRuntimeDedupeKeysRef.current.has(dedupeKey)) { + return "pending"; + } + + persistRuntimeDedupeKeysRef.current.add(dedupeKey); + + const clearDedupeKey = () => { + 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(); + 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") { + if ( + frozenWorkspaceId && + latestWorkspaceId !== frozenWorkspaceId + ) { + clearDedupeKey(); + return "noop"; + } + if (!rekeyDedupe(latestWorkspaceId)) { + return "pending"; + } + 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"; + } + + if ( + effectiveActiveWorkspaceIdRef.current && + effectiveActiveWorkspaceIdRef.current !== workspaceId + ) { + clearDedupeKey(); + return "noop"; + } + + const liveRuntime = appStateServersRef.current[serverName]; + if (!liveRuntime || liveRuntime.connectionStatus !== "connected") { + clearDedupeKey(); + return "noop"; + } + runtime = liveRuntime; + + 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 +1499,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 +3237,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..c1e83911a 100644 --- a/mcpjam-inspector/client/src/hooks/use-workspace-state.ts +++ b/mcpjam-inspector/client/src/hooks/use-workspace-state.ts @@ -438,15 +438,64 @@ 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]) => { + if ( + !activeLocalProjection || + activeProjectionRemoteId !== workspaceId + ) { + return [workspaceId, workspace]; + } + + const projectedServers = Object.fromEntries( + 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) { + 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 +505,15 @@ export function useWorkspaceState({ ); return { - ...convexWorkspaces, + ...mergedConvexWorkspaces, ...workspacesWithoutRemoteMatch, }; - }, [convexWorkspaces, scopedLocalWorkspaces]); + }, [ + appState.activeWorkspaceId, + 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/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, diff --git a/mcpjam-inspector/client/src/lib/session-token.ts b/mcpjam-inspector/client/src/lib/session-token.ts index 621e1c5bc..d34be0900 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,41 @@ 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]" + ); +} + +// 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; + } + + 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 +280,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 +295,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 +331,10 @@ export async function authFetch( }); } - const retryInit = buildAuthFetchInit(init, `Bearer ${refreshedGuestToken}`); + const retryInit = buildAuthFetchInit( + input, + init, + `Bearer ${refreshedGuestToken}`, + ); return fetch(input, retryInit); } 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: