diff --git a/README.md b/README.md index 1cc32ed6e..f9bf664de 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,8 @@ inspector/ Each client has its own README with client-specific detail: [web](./clients/web/README.md) · [cli](./clients/cli/README.md) · [tui](./clients/tui/README.md) · [launcher](./clients/launcher/README.md). +Task-oriented guides live under [`docs/`](./docs) — see [Reviewing an MCP App](./docs/mcp-app-review.md), the CLI-first → one-shot-web recipe for automated App-tool review: `--app-info` probe → deep-link navigate → rendered widget, plus OAuth handoff and proxy support. + ## Setup Requires Node `>=22.19.0`. @@ -120,13 +122,13 @@ Individual clients: `build:web`, `build:cli`, `build:tui`, `build:launcher`. The Each client self-validates from its own folder; the root scripts chain them. There is **no** aggregate root `test` script — use `validate` (fast) or `coverage` (the gate). -| Script | What it does | -| --- | --- | -| `npm run validate` | `format:check` + `lint` + `build` + fast unit tests, per client. The quick inner-loop check. | -| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project. | -| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web). | -| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `smoke` → Storybook. A true superset of GitHub CI. | -| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). | +| Script | What it does | +| --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `npm run validate` | `format:check` + `lint` + `build` + fast unit tests, per client. The quick inner-loop check. | +| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project. | +| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web). | +| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `smoke` → Storybook. A true superset of GitHub CI. | +| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). | Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …). Run `npm run format` (per client) before committing — `validate` runs the non-fixing `format:check` and fails CI on any unformatted file. diff --git a/clients/cli/README.md b/clients/cli/README.md index 3563e870e..7c49d3a35 100644 --- a/clients/cli/README.md +++ b/clients/cli/README.md @@ -193,18 +193,20 @@ For the common case where OAuth was already completed in the **web inspector on | Option | Description | | ----------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `--use-stored-auth` | Read the stored access token for `--server-url` and inject it as `Authorization: Bearer`. Exits `3` (`no_stored_token`) — listing the stored keys — when no token matches. Requires `--server-url`. | -| `--wait-for-auth ` | Poll the OAuth state file (500 ms interval) until a token for `--server-url` appears, then proceed as if `--use-stored-auth` were set. Times out at `` with exit `3` (`auth_wait_timeout`). Use after handing off to a human to complete OAuth in a browser. | +| `--use-stored-auth` | Read the stored auth for `--server-url` and inject `Authorization: Bearer`. When a `refresh_token` is stored, the CLI runs the OAuth refresh grant first and injects the **fresh** access token (persisting the rotation); otherwise it injects the stored access token. Exits `3` (`no_stored_token`) — listing the stored keys — when nothing matches. Requires `--server-url`. | +| `--wait-for-auth ` | Poll the OAuth state file (500 ms interval) until an access token for `--server-url` appears, then inject it. Times out at `` with exit `3` (`auth_wait_timeout`). Use after handing off to a human to complete OAuth in a browser. Unlike `--use-stored-auth`, this injects the freshly-landed access token directly (a token that just completed the browser flow is not expired), so it does **not** run the refresh grant. | | `--list-stored-auth` | Print `{ oauthStatePath, storedServerUrls }` (the server keys that currently have a token) and exit. No server connection is made. | | `--print-handoff` | Print a JSON handoff block (`deepLink`, `portForwardCmd`, `oauthStatePath`, `apiToken`) for `--server-url` and exit — everything a script/remote VM needs to drive the browser-side OAuth dance. Requires `--server-url`. | **State-file resolution** follows `MCP_INSPECTOR_OAUTH_STATE_PATH` → `/oauth.json` → `~/.mcp-inspector/storage/oauth.json` — the same precedence the rest of the Inspector uses, so the CLI and web backend agree on the file. Server keys are canonicalised with `new URL().href` (the scheme the web store writes), so a trailing-slash or case mismatch between the URL a human opened and the one the agent passed still resolves. -**Blind injection.** The token is injected as-is; the CLI does not validate or refresh it (a stored `refresh_token` is not yet used — a natural follow-up). A stale/expired access token surfaces as an HTTP `401` on the first request → exit `3` (`auth_required`). Re-complete the flow in the web inspector (or use `--wait-for-auth`) and retry. +**Token refresh (#1665).** When the stored server state carries a `refresh_token` (plus the `clientInformation` the web inspector persists after a completed flow), `--use-stored-auth` runs the SDK's OAuth `refresh_token` grant to mint a fresh access token before connecting, then writes the rotated tokens back to the state file (owner-only `0o600`, via the shared store writer) so web and CLI stay consistent. The auth-server metadata is reused from the stored state, or discovered from `--server-url` when absent. This covers both an absent and an expired stored access token — the persisted blob carries no expiry, so the refresh token is treated as the durable credential. If the refresh fails (revoked token, transient auth-server error) **and** a stored access token is also present, the CLI falls back to injecting that token rather than hard-failing; only when there is nothing to fall back on does it exit `3` (`auth_required`, envelope `refresh_failed`). Without a stored `refresh_token` the access token is injected as-is, and a stale one surfaces as an HTTP `401` → exit `3`. + +Because there is no stored expiry, a `refresh_token` is refreshed on every `--use-stored-auth` run. Two consequences with rotating refresh tokens: two concurrent invocations against the same state file race the single-use token (one wins), and a crash between a successful grant and the write-back leaves the rotated token unsaved. Both are narrow; re-authorize in the web inspector to recover. **Short-circuit modes.** `--list-stored-auth` and `--print-handoff` each print their output and exit without connecting to a server; they ignore the method/target flags. They are mutually exclusive — if both are passed, `--list-stored-auth` takes precedence. -> The `deepLink` shape emitted by `--print-handoff` is interim, pending the web deep-link auto-connect work ([#1576](https://github.com/modelcontextprotocol/inspector/issues/1576)); it will be reconciled with that issue's canonical handoff format once it lands. +The `deepLink` is the canonical web deep-link ([#1576](https://github.com/modelcontextprotocol/inspector/issues/1576)) — `http://:/?serverUrl=&transport=&autoConnect=` — so navigating it in a browser reaches a connected inspector in one shot. `transport` is derived from the resolved server (`--transport`, else auto-detected from the URL path: `/sse` → `sse`, else `http`), not hardcoded. `autoConnect` is set to `MCP_INSPECTOR_API_TOKEN`; when that env var is unset the link is still emitted but a `note` field flags that the web app's `autoConnect` gate will reject it until the inspector is launched with a known token. ```bash # On a remote VM: print what a human needs to complete OAuth in their browser. diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts index 69be24f59..24c87c49e 100644 --- a/clients/cli/__tests__/stored-auth.test.ts +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -1,10 +1,15 @@ -import { describe, it, expect, beforeAll, afterAll } from "vitest"; -import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { describe, it, expect, beforeAll, afterAll, vi } from "vitest"; +import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { createServer, type Server } from "node:http"; import { join, dirname } from "node:path"; import { tmpdir } from "node:os"; import { runCli } from "./helpers/cli-runner.js"; import { expectCliFailure, expectCliSuccess } from "./helpers/assertions.js"; -import { normalizeServerUrl } from "../src/cli.js"; +import { + normalizeServerUrl, + deepLinkTransport, + refreshStoredAuthToken, +} from "../src/cli.js"; import { createTestServerHttp, createEchoTool, @@ -36,6 +41,198 @@ describe("normalizeServerUrl", () => { }); }); +describe("deepLinkTransport", () => { + it("honors an explicit sse/http transport over the URL path", () => { + expect(deepLinkTransport("https://x.example/mcp", "sse")).toBe("sse"); + expect(deepLinkTransport("https://x.example/sse", "http")).toBe("http"); + }); + + it("auto-detects sse from a /sse path when no transport is given", () => { + expect(deepLinkTransport("https://x.example/sse", undefined)).toBe("sse"); + }); + + it("defaults to http for a /mcp path, an ambiguous path, or stdio", () => { + expect(deepLinkTransport("https://x.example/mcp", undefined)).toBe("http"); + expect(deepLinkTransport("https://x.example/", undefined)).toBe("http"); + expect(deepLinkTransport("https://x.example/mcp", "stdio")).toBe("http"); + }); + + it("defaults to http for an unparseable URL without throwing", () => { + expect(deepLinkTransport("not a url", undefined)).toBe("http"); + }); +}); + +describe("refreshStoredAuthToken", () => { + const SERVER = "https://api.example/mcp"; + const freshTokens = { + access_token: "refreshed-access-token", + token_type: "Bearer", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + }; + + it("runs the refresh grant, injects the fresh token, and persists the rotation", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid", client_secret: "sec" }, + serverMetadata: { + issuer: "https://auth.example", + token_endpoint: "https://auth.example/token", + }, + }, + }); + try { + const refresh = vi.fn().mockResolvedValue(freshTokens); + const discover = vi.fn(); + const token = await refreshStoredAuthToken(SERVER, path, { + refresh, + discover, + }); + expect(token).toBe("refreshed-access-token"); + // Stored metadata present → no discovery needed. + expect(discover).not.toHaveBeenCalled(); + // Refresh called with the stored refresh token + client information. + expect(refresh).toHaveBeenCalledTimes(1); + const [, opts] = refresh.mock.calls[0]!; + expect(opts.refreshToken).toBe("old-refresh"); + expect(opts.clientInformation).toEqual({ + client_id: "cid", + client_secret: "sec", + }); + // Rotation persisted back under the same key. + const persisted = JSON.parse(readFileSync(path, "utf8")) as { + servers: Record; + }; + expect(persisted.servers[SERVER]?.tokens?.refresh_token).toBe( + "rotated-refresh-token", + ); + } finally { + rmSync(path, { force: true }); + } + }); + + it("discovers the auth-server metadata when it was not persisted", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid" }, + }, + }); + try { + const refresh = vi.fn().mockResolvedValue(freshTokens); + const discover = vi.fn().mockResolvedValue({ + issuer: "https://api.example", + token_endpoint: "https://api.example/token", + }); + const token = await refreshStoredAuthToken(SERVER, path, { + refresh, + discover, + }); + expect(token).toBe("refreshed-access-token"); + expect(discover).toHaveBeenCalledTimes(1); + } finally { + rmSync(path, { force: true }); + } + }); + + it("passes metadata=undefined to the refresh grant when discovery returns nothing", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid" }, + }, + }); + try { + const refresh = vi.fn().mockResolvedValue(freshTokens); + const discover = vi.fn().mockResolvedValue(undefined); + const token = await refreshStoredAuthToken(SERVER, path, { + refresh, + discover, + }); + expect(token).toBe("refreshed-access-token"); + const [, opts] = refresh.mock.calls[0]!; + expect(opts.metadata).toBeUndefined(); + } finally { + rmSync(path, { force: true }); + } + }); + + it("uses the distinct no_client_information code when client info is missing", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + }, + }); + try { + await expect( + refreshStoredAuthToken(SERVER, path, { refresh: vi.fn() }), + ).rejects.toMatchObject({ + exitCode: 3, + envelope: { code: "no_client_information" }, + }); + } finally { + rmSync(path, { force: true }); + } + }); + + it("throws AUTH_REQUIRED when no refresh token is stored", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { access_token: "only-access", token_type: "Bearer" }, + }, + }); + try { + await expect( + refreshStoredAuthToken(SERVER, path, { refresh: vi.fn() }), + ).rejects.toMatchObject({ exitCode: 3 }); + } finally { + rmSync(path, { force: true }); + } + }); + + it("throws AUTH_REQUIRED when a refresh token is present but client information is missing", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + }, + }); + try { + await expect( + refreshStoredAuthToken(SERVER, path, { refresh: vi.fn() }), + ).rejects.toMatchObject({ exitCode: 3 }); + } finally { + rmSync(path, { force: true }); + } + }); + + it("surfaces a refresh-grant failure as AUTH_REQUIRED (refresh_failed)", async () => { + const path = writeOAuthFixture({ + [SERVER]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid" }, + serverMetadata: { + issuer: "https://auth.example", + token_endpoint: "https://auth.example/token", + }, + }, + }); + try { + const refresh = vi + .fn() + .mockRejectedValue(new Error("invalid_grant: token revoked")); + await expect( + refreshStoredAuthToken(SERVER, path, { refresh }), + ).rejects.toMatchObject({ + exitCode: 3, + envelope: { code: "refresh_failed" }, + }); + } finally { + rmSync(path, { force: true }); + } + }); +}); + describe("--use-stored-auth", () => { let server: ReturnType; let serverUrl: string; @@ -197,6 +394,121 @@ describe("--use-stored-auth", () => { const last = server.getRecordedRequests().at(-1)!; expect(last.headers?.authorization).toBe(`Bearer ${TOKEN}`); }); + + it("refreshes a stored refresh_token end-to-end and injects the fresh access token (#1665)", async () => { + // Minimal OAuth token endpoint: honors the refresh_token grant with a + // rotated token pair, so the CLI's real SDK refresh path has something to + // talk to (the MCP test server accepts any bearer, so a successful connect + // proves the refreshed token was injected). + let tokenRequests = 0; + const tokenServer: Server = createServer((req, res) => { + tokenRequests += 1; + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + access_token: "refreshed-access-token", + token_type: "Bearer", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + }), + ); + }); + await new Promise((resolve) => tokenServer.listen(0, resolve)); + const addr = tokenServer.address(); + const tokenBase = + typeof addr === "object" && addr ? `http://127.0.0.1:${addr.port}` : ""; + const fixture = writeOAuthFixture({ + [serverUrl]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid", client_secret: "sec" }, + serverMetadata: { + issuer: tokenBase, + token_endpoint: `${tokenBase}/token`, + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_post"], + }, + }, + }); + try { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture } }, + ); + expectCliSuccess(result); + expect(tokenRequests).toBeGreaterThan(0); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe("Bearer refreshed-access-token"); + // Rotation persisted so a subsequent run reuses the new refresh token. + const persisted = JSON.parse(readFileSync(fixture, "utf8")) as { + servers: Record; + }; + expect(persisted.servers[serverUrl]?.tokens?.refresh_token).toBe( + "rotated-refresh-token", + ); + } finally { + rmSync(fixture, { force: true }); + await new Promise((resolve) => tokenServer.close(() => resolve())); + } + }); + + it("falls back to the stored access token when the refresh grant fails (#1665 review #1)", async () => { + // Token endpoint that rejects the refresh grant, so the CLI must fall back + // to the still-present stored access token instead of hard-failing. + const tokenServer: Server = createServer((_req, res) => { + res.writeHead(400, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: "invalid_grant" })); + }); + await new Promise((resolve) => tokenServer.listen(0, resolve)); + const addr = tokenServer.address(); + const tokenBase = + typeof addr === "object" && addr ? `http://127.0.0.1:${addr.port}` : ""; + const fixture = writeOAuthFixture({ + [serverUrl]: { + tokens: { + access_token: "still-usable-access", + refresh_token: "old-refresh", + token_type: "Bearer", + }, + clientInformation: { client_id: "cid", client_secret: "sec" }, + serverMetadata: { + issuer: tokenBase, + token_endpoint: `${tokenBase}/token`, + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_post"], + }, + }, + }); + try { + const result = await runCli( + [ + "--transport", + "http", + "--server-url", + serverUrl, + "--use-stored-auth", + "--method", + "tools/list", + ], + { env: { MCP_INSPECTOR_OAUTH_STATE_PATH: fixture } }, + ); + expectCliSuccess(result); + const last = server.getRecordedRequests().at(-1)!; + expect(last.headers?.authorization).toBe("Bearer still-usable-access"); + } finally { + rmSync(fixture, { force: true }); + await new Promise((resolve) => tokenServer.close(() => resolve())); + } + }); }); describe("--list-stored-auth", () => { @@ -258,6 +570,8 @@ describe("--print-handoff", () => { expect(out.serverUrl).toBe("https://x.example/mcp"); expect(out.deepLink).toContain("autoConnect=tok123"); expect(out.deepLink).toContain("serverUrl=https%3A%2F%2Fx.example%2Fmcp"); + // Canonical #1576 deep-link: a `/mcp` server hands off `transport=http`. + expect(out.deepLink).toContain("transport=http"); expect(out.portForwardCmd).toContain("--tcp 16274:16274"); expect(out.portForwardCmd).toContain("--tcp 16275:16275"); expect(out.oauthStatePath).toBe( @@ -266,6 +580,33 @@ describe("--print-handoff", () => { expect(out.apiToken).toBe("tok123"); }); + it("derives transport=sse for an SSE server (auto-detected from the /sse path)", async () => { + const result = await runCli( + ["--print-handoff", "--server-url", "https://x.example/sse"], + { env: { MCP_INSPECTOR_API_TOKEN: "tok123" } }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { deepLink: string }; + expect(out.deepLink).toContain("transport=sse"); + expect(out.deepLink).not.toContain("transport=http"); + }); + + it("honors an explicit --transport sse over the URL path", async () => { + const result = await runCli( + [ + "--print-handoff", + "--server-url", + "https://x.example/mcp", + "--transport", + "sse", + ], + { env: { MCP_INSPECTOR_API_TOKEN: "tok123" } }, + ); + expectCliSuccess(result); + const out = JSON.parse(result.stdout) as { deepLink: string }; + expect(out.deepLink).toContain("transport=sse"); + }); + it("includes a note when MCP_INSPECTOR_API_TOKEN is unset", async () => { const result = await runCli( ["--print-handoff", "--server-url", "https://x.example/mcp"], diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index 059338db5..2025947ea 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -28,7 +28,22 @@ import type { JsonValue } from "@inspector/core/mcp/index.js"; import { extractAppInfo } from "@inspector/core/mcp/apps.js"; import type { AppInfo } from "@inspector/core/mcp/apps.js"; import { getStateFilePath } from "@inspector/core/auth/node/storage-node.js"; -import { parseOAuthPersistBlob } from "@inspector/core/auth/oauth-persist.js"; +import { + parseOAuthPersistBlob, + serializeOAuthPersistBlob, + type OAuthPersistSnapshot, +} from "@inspector/core/auth/oauth-persist.js"; +import { getAuthorizationServerUrl } from "@inspector/core/auth/discovery.js"; +import { writeStoreFile } from "@inspector/core/storage/store-io.js"; +import { + refreshAuthorization, + discoverAuthorizationServerMetadata, +} from "@modelcontextprotocol/sdk/client/auth.js"; +import type { + OAuthClientInformation, + OAuthMetadata, + OAuthTokens, +} from "@modelcontextprotocol/sdk/shared/auth.js"; import { CliExitCodeError, EXIT_CODES } from "./error-handler.js"; import { ConsoleNavigation, @@ -469,26 +484,59 @@ export function normalizeServerUrl(serverUrl: string): string { } } +/** The subset of a stored server's OAuth state the CLI reads/refreshes. */ +type StoredServerState = { + tokens?: OAuthTokens; + clientInformation?: OAuthClientInformation; + serverMetadata?: OAuthMetadata; +}; /** The stored-server map shape the CLI reads out of the OAuth state file. */ -type StoredServers = Record; +type StoredServers = Record; /** * Read the OAuth state file directly (bypassing the Zustand store cache) so * each call sees the current on-disk state — required for `--wait-for-auth` - * polling. Returns the `servers` map, or an empty object when the file is - * absent or unreadable. Uses the shared {@link parseOAuthPersistBlob} so both - * the plain `{servers,idpSessions}` and legacy `{state,version}` layouts are + * polling. Returns the full snapshot, or an empty one when the file is absent + * or unreadable. Uses the shared {@link parseOAuthPersistBlob} so both the + * plain `{servers,idpSessions}` and legacy `{state,version}` layouts are * accepted, matching whatever the web backend wrote. */ -async function readOAuthServers(statePath: string): Promise { +async function readOAuthSnapshot( + statePath: string, +): Promise { const { readFile } = await import("node:fs/promises"); try { const text = await readFile(statePath, "utf8"); const snapshot = parseOAuthPersistBlob(text); - return (snapshot?.servers as StoredServers | undefined) ?? {}; + if (snapshot) return snapshot; } catch { - return {}; + // Absent/unreadable/malformed → fall through to the empty snapshot below. } + return { servers: {}, idpSessions: {} }; +} + +/** + * Read just the `servers` map. Thin wrapper over {@link readOAuthSnapshot} for + * the read-only lookups (`findStoredToken`, `--wait-for-auth`, key listing). + */ +async function readOAuthServers(statePath: string): Promise { + return (await readOAuthSnapshot(statePath)).servers as StoredServers; +} + +/** + * Look up a stored server's OAuth state, trying the URL-normalised key first + * (how the web store writes it) and the raw string second. Returns the matched + * key so a write-back updates the same entry. + */ +function findStoredServerState( + servers: StoredServers, + serverUrl: string, +): { key: string; state: StoredServerState } | undefined { + const normalized = normalizeServerUrl(serverUrl); + if (servers[normalized]) + return { key: normalized, state: servers[normalized] }; + if (servers[serverUrl]) return { key: serverUrl, state: servers[serverUrl] }; + return undefined; } /** @@ -499,11 +547,91 @@ function findStoredToken( servers: StoredServers, serverUrl: string, ): string | undefined { - const key = normalizeServerUrl(serverUrl); - return ( - servers[key]?.tokens?.access_token ?? - servers[serverUrl]?.tokens?.access_token - ); + return findStoredServerState(servers, serverUrl)?.state.tokens?.access_token; +} + +/** + * Injectable dependencies for {@link refreshStoredAuthToken}, so the refresh + * grant + auth-server discovery can be faked in unit tests without standing up + * a real OAuth token endpoint. Both default to the SDK implementations. + */ +export interface RefreshStoredAuthDeps { + refresh?: typeof refreshAuthorization; + discover?: typeof discoverAuthorizationServerMetadata; +} + +/** + * Run the OAuth `refresh_token` grant for a stored server and persist the + * rotated tokens back to `statePath` (same `{servers,idpSessions}` shape the + * web backend writes), returning the fresh access token. + * + * Reuses the SDK's {@link refreshAuthorization} (not a hand-rolled token + * request) with the stored `clientInformation` + `serverMetadata`; when the + * metadata wasn't persisted it is discovered from the resolved authorization + * server. A missing refresh token or client information, or a failed grant, + * throws {@link CliExitCodeError} with {@link EXIT_CODES.AUTH_REQUIRED} so the + * caller exits with the documented code and a clear message. + */ +export async function refreshStoredAuthToken( + serverUrl: string, + statePath: string, + deps: RefreshStoredAuthDeps = {}, +): Promise { + const refresh = deps.refresh ?? refreshAuthorization; + const discover = deps.discover ?? discoverAuthorizationServerMetadata; + + const snapshot = await readOAuthSnapshot(statePath); + const servers = snapshot.servers as StoredServers; + const found = findStoredServerState(servers, serverUrl); + const refreshToken = found?.state.tokens?.refresh_token; + const clientInformation = found?.state.clientInformation; + if (!found || !refreshToken) { + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `No stored refresh token for ${normalizeServerUrl(serverUrl)} in ${statePath}. Complete the OAuth flow in the web inspector first.`, + { code: "no_stored_token", url: serverUrl }, + ); + } + if (!clientInformation) { + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `Stored auth for ${normalizeServerUrl(serverUrl)} has a refresh token but no client information; cannot refresh. Re-authorize in the web inspector.`, + { code: "no_client_information", url: serverUrl }, + ); + } + + const authServerUrl = found.state.serverMetadata?.issuer + ? new URL(found.state.serverMetadata.issuer) + : getAuthorizationServerUrl(serverUrl); + const metadata = + found.state.serverMetadata ?? (await discover(authServerUrl)) ?? undefined; + + let tokens: OAuthTokens; + try { + tokens = await refresh(authServerUrl, { + metadata, + clientInformation, + refreshToken, + resource: new URL(serverUrl), + }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `Failed to refresh the stored OAuth token for ${normalizeServerUrl(serverUrl)}: ${message}. Re-authorize in the web inspector.`, + { code: "refresh_failed", url: serverUrl }, + ); + } + + // Persist the rotated tokens back under the same key, preserving every other + // server entry and the idpSessions block, so web and CLI stay consistent. + // Route through the shared `writeStoreFile` (not a raw `writeFile`) so the + // secrets file keeps its owner-only `0o600` mode + `mkdir -p`, identical to + // how the web backend's OAuth persist backend writes it. + servers[found.key] = { ...found.state, tokens }; + await writeStoreFile(statePath, serializeOAuthPersistBlob(snapshot)); + + return tokens.access_token; } /** @@ -539,31 +667,64 @@ async function waitForStoredToken( } } +/** + * Derive the web deep-link `transport` value (`http` | `sse`) for a handoff. + * Mirrors `resolveServerConfigs` (core/mcp/node/config.ts) URL-path + * auto-detection (`/sse` → sse, + * everything else → http) but, unlike that resolver, defaults to `http` instead + * of throwing on an ambiguous path — the handoff is best-effort, and the web + * {@link parseDeepLink} likewise defaults an unknown/missing transport to http. + */ +export function deepLinkTransport( + serverUrl: string, + transport: "sse" | "http" | "stdio" | undefined, +): "http" | "sse" { + if (transport === "sse") return "sse"; + if (transport === "http") return "http"; + try { + if (new URL(serverUrl).pathname.endsWith("/sse")) return "sse"; + } catch { + // Unparseable URL: fall through to the http default. The web parser rejects + // a non-http(s) serverUrl anyway, so guessing a transport for it is moot. + } + return "http"; +} + /** * Build the JSON `--print-handoff` emits: everything an automated caller needs * to relay to a human so they can complete OAuth in a browser and have the * token land where the CLI will find it. * - * NOTE: the `deepLink` query shape (serverUrl/transport/autoConnect) is the - * interim format pending the web deep-link auto-connect work (#1576); reconcile - * with that issue's canonical handoff format once it lands. + * The `deepLink` is the canonical web format owned by + * `clients/web/src/utils/deepLink.ts` (#1576): `?serverUrl&transport&autoConnect`, + * where `autoConnect` is the CSRF gate (must equal `MCP_INSPECTOR_API_TOKEN`). + * `transport` is derived from the resolved server via {@link deepLinkTransport} + * rather than hardcoded, so an SSE server hands off a `transport=sse` link. */ -function buildHandoff(serverUrl: string, statePath: string): McpResponse { +function buildHandoff( + serverUrl: string, + statePath: string, + transport: "sse" | "http" | "stdio" | undefined, +): McpResponse { const host = process.env.HOST || "127.0.0.1"; const clientPort = process.env.CLIENT_PORT || "6274"; const sandboxPort = process.env.MCP_SANDBOX_PORT || "6275"; // Treat an empty MCP_INSPECTOR_API_TOKEN the same as unset — an empty token // can't satisfy the deep-link autoConnect gate. const apiToken = process.env.MCP_INSPECTOR_API_TOKEN || undefined; - // TODO(#1576): interim deep-link shape. `transport: "http"` is hardcoded even - // for SSE servers, and `autoConnect=` is NOT one of the three token - // sources the web app reads (window.__INSPECTOR_API_TOKEN__ / - // ?MCP_INSPECTOR_API_TOKEN / sessionStorage — see CLAUDE.md). Reconcile both - // with #1576's canonical handoff/deep-link format once it lands. - const params = new URLSearchParams({ serverUrl, transport: "http" }); + const normalizedUrl = normalizeServerUrl(serverUrl); + // Canonical #1576 deep-link shape: the normalized serverUrl (matching the + // OAuth-store key form the web app reuses) plus the resolved transport, gated + // by `autoConnect=` — the same per-launch token the web parser + // requires. Omitted when no token is set; the `note` below flags that the + // link will be rejected until the web inspector is launched with a token. + const params = new URLSearchParams({ + serverUrl: normalizedUrl, + transport: deepLinkTransport(serverUrl, transport), + }); if (apiToken) params.set("autoConnect", apiToken); return { - serverUrl: normalizeServerUrl(serverUrl), + serverUrl: normalizedUrl, deepLink: `http://${host}:${clientPort}/?${params.toString()}`, portForwardCmd: `coder port-forward --tcp ${clientPort}:${clientPort} --tcp ${sandboxPort}:${sandboxPort}`, oauthStatePath: statePath, @@ -897,7 +1058,9 @@ async function parseArgs(argv?: string[]): Promise { throw new Error("--print-handoff requires --server-url"); } await awaitableLog( - JSON.stringify(buildHandoff(options.serverUrl, oauthStatePath)) + "\n", + JSON.stringify( + buildHandoff(options.serverUrl, oauthStatePath, options.transport), + ) + "\n", ); return { shortCircuit: true }; } @@ -934,11 +1097,13 @@ async function parseArgs(argv?: string[]): Promise { } // Read the OAuth state file directly so the lookup is normalised the same // way the web inspector wrote it (`new URL().href`), and so `--wait-for- - // auth` sees fresh on-disk state on each poll. Header injection is the - // prototype path — only the access token is injected, blindly; a stale one - // surfaces as HTTP 401 → exit 3 (auth_required). Follow-up: when the stored - // `tokens.refresh_token` is present and the access token is expired, mint a - // fresh one instead of injecting the stale token. + // auth` sees fresh on-disk state on each poll. When a `refresh_token` is + // stored, the CLI runs the SDK refresh grant and injects the fresh access + // token (persisting the rotation) rather than blindly injecting a possibly- + // stale stored access token (#1665) — the stored blob carries no expiry, so + // the refresh token is the durable credential. Without a refresh token it + // falls back to injecting the stored access token; a stale one surfaces as + // HTTP 401 → exit 3 (auth_required). let token: string; if (options.waitForAuth !== undefined) { token = await waitForStoredToken( @@ -948,18 +1113,40 @@ async function parseArgs(argv?: string[]): Promise { ); } else { const servers = await readOAuthServers(oauthStatePath); - const found = findStoredToken(servers, options.serverUrl); - if (!found) { - const key = normalizeServerUrl(options.serverUrl); - const stored = Object.keys(servers); - throw new CliExitCodeError( - EXIT_CODES.AUTH_REQUIRED, - `No stored OAuth token for ${key} in ${oauthStatePath}. Complete the OAuth flow in the web inspector first.` + - (stored.length > 0 ? ` Stored keys: ${stored.join(", ")}.` : ""), - { code: "no_stored_token", url: options.serverUrl }, - ); + const stored = findStoredServerState(servers, options.serverUrl); + if (stored?.state.tokens?.refresh_token) { + const storedAccess = stored.state.tokens.access_token; + try { + token = await refreshStoredAuthToken( + options.serverUrl, + oauthStatePath, + ); + } catch (err) { + // A failed refresh (transient auth-server hiccup, missing client + // info) shouldn't turn a previously-working invocation into a hard + // failure when a still-usable access token is also on disk — fall + // back to injecting it (a genuinely stale one surfaces as HTTP 401 → + // exit 3, the same as without a refresh token). With no stored access + // token to fall back on, the refresh error stands. + if (!storedAccess) throw err; + token = storedAccess; + } + } else { + const found = findStoredToken(servers, options.serverUrl); + if (!found) { + const key = normalizeServerUrl(options.serverUrl); + const storedKeys = Object.keys(servers); + throw new CliExitCodeError( + EXIT_CODES.AUTH_REQUIRED, + `No stored OAuth token for ${key} in ${oauthStatePath}. Complete the OAuth flow in the web inspector first.` + + (storedKeys.length > 0 + ? ` Stored keys: ${storedKeys.join(", ")}.` + : ""), + { code: "no_stored_token", url: options.serverUrl }, + ); + } + token = found; } - token = found; } serverOptions.headers = { ...(serverOptions.headers ?? {}), diff --git a/clients/web/README.md b/clients/web/README.md index 329317e4c..a836f4329 100644 --- a/clients/web/README.md +++ b/clients/web/README.md @@ -71,6 +71,45 @@ The Apps screen exposes a small, stable set of `data-testid` / `data-*` attribut The renderer lifecycle itself is `AppRendererStatus` (`loading` | `ready` | `error`) reported via `AppRenderer`'s `onAppStatusChange`; the screen maps it to `data-app-status`. Resource-read failures (malformed/404 UI resource) are surfaced as a toast via the bridge factory's `onResourceError`; because the app never reaches `ready` in that case, a driver times out on `data-app-status` and reads the toast. +## Deep-link auto-connect + +A driver (launcher, CLI `--print-handoff`, CI review harness) can reach a **connected** inspector with a single navigate by encoding the target in the URL query string. Parsing + security gating live in `src/utils/deepLink.ts` (`parseDeepLink`), and a returned `DeepLink` is proof the link passed validation. + +``` +http://127.0.0.1:6274/?serverUrl=&transport=http|sse&autoConnect= +``` + +| Param | Meaning | +| --- | --- | +| `serverUrl` | The MCP server URL. Restricted to `http:` / `https:` (a crafted `javascript:` / `data:` / `file:` value is rejected). Canonicalized via `URL.href` so it matches the OAuth store's key form. | +| `transport` | `http` (streamable-HTTP, the default) or `sse`. Unknown values fall back to `http`. | +| `autoConnect` | **CSRF gate.** Must equal the per-launch `MCP_INSPECTOR_API_TOKEN`. The token is random per launch and only known to whatever started the server, so a third-party-minted link cannot satisfy it — this is the same exposure surface as the existing `?MCP_INSPECTOR_API_TOKEN=` param. Without a match the link is ignored. | + +The deep link upserts a stable `deep-link` catalog row (so a reload reconnects to the same row instead of accumulating duplicates) and connects. Connection-level outcomes are surfaced on the `AppShell.Header` as a machine-readable contract, so a driver can `waitForSelector` and read the failure reason without scraping a transient toast: + +| Attribute | Where | Meaning | +| --- | --- | --- | +| `data-testid="connection-status"` | header | The element carrying the attributes below. | +| `data-status` | on `connection-status` | The live `ConnectionStatus` (`disconnected` → `connecting` → `connected` / `error`). Poll for `connected`. | +| `data-error-message` | on `connection-status` | Why the last connect failed (handshake error, OAuth-start failure, deep-link automation failure); absent when there is no error. | +| `data-deeplink` | on `connection-status` | `parsed` (a valid deep link drove this load), `rejected` (deep-link params present but the token/serverUrl gate failed), or `none`. Lets a driver distinguish "no deep link" from "rejected" — both otherwise leave `data-status` idle. | + +### Landing on a rendered app + +Three further params extend the deep link to pre-select — and optionally auto-open — an MCP App, so a driver reaches a rendered widget with zero clicks: + +``` +…&openApp=&appArgs=&autoOpen= +``` + +| Param | Meaning | +| --- | --- | +| `openApp` | The app-tool name. Once the connection is up and the tool appears in the app list, the inspector switches to the Apps tab and pre-selects it. | +| `appArgs` | `base64url(JSON)` object of form values. Merged **over** the tool's schema defaults (`collectSchemaDefaults`) so a required-with-default field isn't left blank — which would otherwise disable "Open App". Malformed / non-object values fall back to `{}`. | +| `autoOpen` | **Same CSRF gate as `autoConnect`** — must equal the session token. When set, "Open App" fires automatically (a tool call from a URL), so the token gate is mandatory. Without a match the app is pre-selected but not opened. | + +The app-side render lifecycle is observable through the [MCP Apps screen automation contract](#mcp-apps-screen-automation-contract) above (`data-app-status="ready"`), so a driver can `waitForSelector` the whole `connect → open → ready` chain deterministically. + ## Theme (`src/theme/`) Each customized Mantine component has a `Theme.ts` file (`Button.ts`, `Text.ts`, …, ~21 total) exporting a `Theme` constant; the barrel `index.ts` re-exports them and `theme.ts` assembles the `MantineProvider` theme. Theme files hold app-wide defaults and **variants** (flat CSS-in-JS); only pseudo-selectors, nested child selectors, keyframes, and native-HTML styling belong in `App.css`. Element components import from `@mantine/core` (never from `theme/`) — the theme layer is applied transparently by the provider. diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 255d71e31..350213f4c 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -154,6 +154,12 @@ import { } from "./components/groups/PendingClientRequestModal/PendingClientRequestModal"; import { buildExportFilename, downloadJsonFile } from "./lib/downloadFile"; import { INSPECTOR_SERVERS_TAB } from "./utils/inspectorTabs"; +import { + parseDeepLink, + deepLinkConfigEquals, + deepLinkParseStatus, +} from "./utils/deepLink"; +import type { DeepLink, DeepLinkParseStatus } from "./utils/deepLink"; import { applyOAuthResumeUi, buildTabUiSnapshot, @@ -1347,6 +1353,37 @@ function App() { serversRef.current = servers; }, [activeServer, activeServerId, servers]); + // Last connection-level error message, surfaced as `data-error-message` on + // the InspectorView header so an automated driver can read *why* a connect + // failed without scraping a transient toast. Cleared on the next connect + // attempt and on successful connection. + const [connectErrorMessage, setConnectErrorMessage] = useState< + string | undefined + >(undefined); + // Named writer so a single call site can be extended later (e.g. telemetry) + // and the intent (record a connection-level failure) stays explicit. + const recordConnectError = useCallback((message: string) => { + setConnectErrorMessage(message); + }, []); + + // Deep-link parameters parsed once from the initial URL. Security gating + // (auth-token match, http(s)-only serverUrl) happens inside `parseDeepLink`, + // so a `DeepLink` value here is already validated. The parse status is + // surfaced as `data-deeplink` so an automated driver can tell "no deep link" + // from "deep link present but rejected" — both leave `data-status` idle. + const [deepLink, deepLinkStatus] = useMemo< + [DeepLink | undefined, DeepLinkParseStatus] + >(() => { + /* v8 ignore next -- SSR guard: happy-dom always defines window in tests */ + if (typeof window === "undefined") return [undefined, "none"]; + const search = window.location.search; + const parsed = parseDeepLink(search, getAuthToken()); + return [parsed, deepLinkParseStatus(search, parsed)]; + }, []); + const deepLinkEnsureRef = useRef(false); + const deepLinkUpdateRef = useRef(false); + const deepLinkConnectRef = useRef(false); + const showReAuthBanner = useCallback( ( serverId: string, @@ -2338,7 +2375,11 @@ function App() { return; } - const target = servers.find((s) => s.id === id); + // Read from the ref so a caller that already awaited an + // addServer/updateServer in the same async tick (e.g. the deep-link + // auto-connect IIFE) sees the freshly-mutated list, not the stale array + // captured by this callback's closure. + const target = serversRef.current.find((s) => s.id === id); if (!target) return; // Always rebuild the InspectorClient on a (re)connect so the latest @@ -2354,6 +2395,9 @@ function App() { // the red border on the last-failed card is removed (#1621). If this // attempt also fails, the catch below re-sets it for this server. setFailedServerId(undefined); + // Clear the machine-readable connect error for the same reason; a fresh + // attempt starts from a clean `data-error-message`. + setConnectErrorMessage(undefined); connectStartRef.current = Date.now(); try { @@ -2432,6 +2476,7 @@ function App() { } const message = authErr instanceof Error ? authErr.message : String(authErr); + setConnectErrorMessage(message); notifications.show({ title: `OAuth authorization failed for "${target.name}"`, message, @@ -2446,6 +2491,7 @@ function App() { // "disconnected", and flag the card with a red border (#1621). setFailedServerId(id); const message = err instanceof Error ? err.message : String(err); + setConnectErrorMessage(message); notifications.show({ title: `Failed to connect to "${target.name}"`, message, @@ -2457,7 +2503,6 @@ function App() { activeServerId, connectionStatus, inspectorClient, - servers, setupClientForServer, prepareOAuthRedirect, finalizeExplicitDisconnect, @@ -2473,6 +2518,85 @@ function App() { } }, [inspectorClient, finalizeExplicitDisconnect]); + // Deep-link auto-connect (the URL-driven case of #1183). `useServers` + // hydrates asynchronously (initial `servers` is `[]`), so this effect runs in + // discrete phases keyed on what `servers` currently reflects, one per render: + // 1. ensure — no row yet: one-shot `addServer`. + // 2. update — row present but its persisted config differs from the deep + // link (a stale transport/url from an earlier load under the stable + // `deep-link` id): `updateServer`, then return so the effect re-runs. + // 3. connect — row present AND its config already matches: connect. + // Splitting update and connect across renders (rather than awaiting both in + // one closure) is what makes the connect correct: `onToggleConnection` reads + // the target from `serversRef`, which an earlier passive effect syncs from + // `servers` — so connecting only once `servers` reflects the updated config + // guarantees the client is built from the fresh transport, not the stale one. + // The OAuth callback path takes precedence; a deep link on `/oauth/callback` + // would be a misconfiguration, and the callback handler clears the URL. + useEffect(() => { + if (!deepLink) return; + if (window.location.pathname === OAUTH_CALLBACK_PATH) return; + + const existing = servers.find((s) => s.id === deepLink.serverId); + if (!existing) { + if (deepLinkEnsureRef.current) return; + deepLinkEnsureRef.current = true; + void addServer(deepLink.serverId, deepLink.serverConfig).catch((err) => { + const message = err instanceof Error ? err.message : String(err); + // A 409 ("already exists") means the row is on disk and hydration will + // surface it on a later render, so the connect phase still proceeds — + // swallow it. Any other failure (read-only catalog, backend 5xx) would + // otherwise leave the deep link permanently stuck at this guard with no + // signal, so record it on the machine-readable error surface. + if (!message.includes("already exists")) recordConnectError(message); + }); + return; + } + + if (!deepLinkConfigEquals(existing.config, deepLink.serverConfig)) { + if (deepLinkUpdateRef.current) return; + deepLinkUpdateRef.current = true; + void updateServer( + deepLink.serverId, + deepLink.serverId, + deepLink.serverConfig, + ).catch((err) => { + const message = err instanceof Error ? err.message : String(err); + recordConnectError(message); + }); + return; + } + + if (deepLinkConnectRef.current) return; + deepLinkConnectRef.current = true; + // Connect unless we're already *connected* to the deep-link server. Gating + // on `activeServerId` identity alone would skip the connect when a prior + // session restored `activeServerId` to the `deep-link` id while the socket + // is disconnected — a reload of the same deep-link URL would then silently + // never connect. `onToggleConnection` only disconnects when the id is the + // active one AND the status is connected, so this condition also avoids + // toggling a live connection off. + const alreadyConnected = + activeServerId === deepLink.serverId && connectionStatus === "connected"; + if (!alreadyConnected) { + void onToggleConnection(deepLink.serverId).catch((err) => { + // The toast fires from inside `onToggleConnection` for the common + // cases; this catch covers the rest (surfaced on `data-error-message`). + const message = err instanceof Error ? err.message : String(err); + recordConnectError(message); + }); + } + }, [ + deepLink, + servers, + activeServerId, + connectionStatus, + addServer, + updateServer, + onToggleConnection, + recordConnectError, + ]); + const onReauthenticateFromBanner = useCallback(() => { if (!reAuthBanner) return; const serverId = reAuthBanner.serverId; @@ -3691,11 +3815,14 @@ function App() { ) : null} { expect(screen.getByTitle("Ops Dashboard")).toBeInTheDocument(); }); + it("auto-opens the pre-selected app with its seeded form values when autoOpen is set (deep-link)", async () => { + const onOpenApp = vi.fn(); + renderWithMantine( + , + ); + // No click: the fire-once effect opens the seeded app with its form values. + await vi.waitFor(() => + expect(onOpenApp).toHaveBeenCalledWith("weather", { city: "Reykjavik" }), + ); + expect(onOpenApp).toHaveBeenCalledTimes(1); + // Renderer mounted; the Open App button is gone. + expect( + screen.queryByRole("button", { name: /Open App/ }), + ).not.toBeInTheDocument(); + expect(screen.getByTitle("Weather Widget")).toBeInTheDocument(); + }); + + it("does not auto-open when autoOpen is set but no app is pre-selected", async () => { + const onOpenApp = vi.fn(); + renderWithMantine(); + // Give the effect a chance to run; with no selection it must stay idle. + await Promise.resolve(); + expect(onOpenApp).not.toHaveBeenCalled(); + expect(screen.getByText("Select an app to view details")).toBeVisible(); + }); + it("invokes onOpenApp with form values when Open App is clicked", async () => { const user = userEvent.setup(); const onOpenApp = vi.fn(); diff --git a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx index dd85b02ae..0a38c2eae 100644 --- a/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx +++ b/clients/web/src/components/screens/AppsScreen/AppsScreen.tsx @@ -1,4 +1,4 @@ -import { useRef, useState, type Ref } from "react"; +import { useCallback, useEffect, useRef, useState, type Ref } from "react"; import { ActionIcon, Button, @@ -64,6 +64,14 @@ export interface AppsScreenProps { onCloseApp: () => void; /** Surfaces bridge/runtime failures from the renderer (e.g. no client). */ onError?: (err: Error) => void; + /** + * Deep-link auto-open (#1577): when true and an app is already pre-selected + * (the parent seeds `ui.selectedAppName` + `ui.formValues` from the URL), + * the screen fires "Open App" automatically — no explicit click. Token-gated + * upstream in `parseDeepLink` (the URL value must equal the session token), + * so a third-party link cannot auto-invoke a tool. Fires exactly once. + */ + autoOpen?: boolean; } // Selected app, its form values, and the sidebar search — controlled by the @@ -333,6 +341,7 @@ export function AppsScreen({ onOpenApp, onCloseApp, onError, + autoOpen = false, }: AppsScreenProps) { const { selectedAppName, formValues, search } = ui; const [running, setRunning] = useState(false); @@ -415,8 +424,9 @@ export function AppsScreen({ // Clear the message + log panels (and the reported height). Called when a run // ends or the selected app changes so a new run starts clean. `keepPartials` // is set for `handleOpen`, where the staged fragments are about to be consumed - // by the renderer and must not be cleared first. - function resetAppChannels(opts?: { keepPartials?: boolean }) { + // by the renderer and must not be cleared first. Memoized (stable setState + // calls only) so the deep-link auto-open effect's deps don't churn. + const resetAppChannels = useCallback((opts?: { keepPartials?: boolean }) => { setAppHeight(undefined); setMessages([]); setAppLogs([]); @@ -424,7 +434,7 @@ export function AppsScreen({ setAppStatus("idle"); setAppError(undefined); if (!opts?.keepPartials) setPartialStages([]); - } + }, []); // Capture the error locally (so it can be shown in the card and surfaced as // `data-app-error`) and forward it to the parent's onError. The renderer @@ -500,6 +510,36 @@ export function AppsScreen({ onCloseApp(); } + // Deep-link auto-open (#1577): the parent seeds `ui.selectedAppName` + + // `ui.formValues` from the URL and sets `autoOpen`; fire "Open App" here so + // the driver lands on a rendered widget with zero clicks. Ref-guarded to fire + // exactly once — a later manual close leaves `running` false without + // re-triggering. The open (running + channel resets + `onOpenApp`) is + // deferred one microtask past the synchronous effect body: it calls setState, + // which the set-state-in-effect lint rightly flags in general, but this is a + // ref-guarded run-once effect so the cascading-render concern doesn't apply + // (same pattern as App.tsx's deep-link connect effect). `resetAppChannels` + // changes identity each render, so the effect re-runs, but the ref guard + // makes every run after the first a no-op. + const autoOpenFiredRef = useRef(false); + useEffect(() => { + if (!autoOpen || autoOpenFiredRef.current) return; + if (!selectedTool || running) return; + autoOpenFiredRef.current = true; + void Promise.resolve().then(() => { + resetAppChannels({ keepPartials: true }); + setRunning(true); + onOpenApp(selectedTool.name, formValues); + }); + }, [ + autoOpen, + selectedTool, + running, + formValues, + onOpenApp, + resetAppChannels, + ]); + function handleBackToInput() { setRunning(false); setMaximized(false); diff --git a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx index 2cf66769c..2f494904b 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.test.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.test.tsx @@ -259,6 +259,42 @@ describe("InspectorView", () => { expect(onToggleConnection).toHaveBeenCalledWith("alpha"); }); + it("exposes the machine-readable connection-status header attributes for drivers", () => { + renderWithMantine( + , + ); + const header = screen.getByTestId("connection-status"); + expect(header).toHaveAttribute("data-status", "error"); + expect(header).toHaveAttribute( + "data-error-message", + "handshake failed: 500", + ); + expect(header).toHaveAttribute("data-deeplink", "rejected"); + }); + + it("omits the error-message attribute when no connect error is recorded", () => { + renderWithMantine( + , + ); + const header = screen.getByTestId("connection-status"); + expect(header).toHaveAttribute("data-status", "disconnected"); + expect(header).not.toHaveAttribute("data-error-message"); + expect(header).toHaveAttribute("data-deeplink", "none"); + }); + it("renders the connected header when connectionStatus + initializeResult are set", () => { renderWithMantine( { }); }); + it("deep-link openApp auto-switches to the Apps tab and pre-selects the app once connected", async () => { + const onSelectApp = vi.fn(); + renderWithMantine( + , + ); + // No click: the effect flips the lifted tab to Apps and seeds the selection. + expect(await screen.findByText("MCP Apps (1)")).toBeInTheDocument(); + await waitFor(() => expect(onSelectApp).toHaveBeenCalledWith("ops")); + }); + + it("deep-link appArgs are merged over the schema defaults into the pre-filled form", async () => { + const fieldedAppTool: Tool = { + name: "cohorts", + title: "Cohort Data", + inputSchema: { + type: "object", + properties: { + metric: { type: "string", default: "retention" }, + zip: { type: "string" }, + }, + }, + _meta: { ui: { resourceUri: "ui://apps/cohorts" } }, + }; + renderWithMantine( + , + ); + // The form is pre-filled: `zip` from appArgs, `metric` from the schema default. + expect(await screen.findByDisplayValue("10001")).toBeInTheDocument(); + expect(screen.getByDisplayValue("retention")).toBeInTheDocument(); + }); + + it("ignores a deep-link openApp whose tool is not an app (no tab switch)", async () => { + renderWithMantine( + , + ); + // The target app never appears, so the effect never switches tabs: the + // Apps screen is never activated even though the Apps tab is available. + const radios = await screen.findAllByRole("radio"); + expect(radios.map((r) => r.getAttribute("value"))).toContain("Apps"); + expect(screen.queryByText("MCP Apps (1)")).not.toBeInTheDocument(); + }); + it("snaps activeTab back to Servers when the Apps tab disappears after a refresh", async () => { const user = userEvent.setup({ delay: null }); const { rerender } = renderWithMantine( diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index 46e97d6ef..d8d335cd0 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -6,6 +6,7 @@ import { type ReactNode, type Ref, } from "react"; +import type { DeepLink, DeepLinkParseStatus } from "../../../utils/deepLink"; import { AppShell, Flex, @@ -88,6 +89,7 @@ import { MonitoringScreen } from "../../groups/MonitoringScreen/MonitoringScreen import { ResizeHandle } from "../../elements/ResizeHandle/ResizeHandle"; import { getServerType } from "@inspector/core/mcp/config.js"; import { INSPECTOR_SERVERS_TAB } from "../../../utils/inspectorTabs"; +import { collectSchemaDefaults } from "../../../utils/jsonUtils"; import { MONITOR_COLUMN_ANIM_MS } from "./monitorColumnAnimation"; const SORT_DEFAULT: SortDirection = "newest-first"; @@ -304,6 +306,21 @@ const MonitorColumnGroup = Group.withProps({ }); export interface InspectorViewProps { + /** + * Validated deep-link parameters from the page URL. When present and + * `openApp` is set, the parent switches to the Apps tab and pre-selects that + * app (with `appArgs` as the form values) once the connection is up and the + * app list contains it. The connect itself is driven by the parent. + */ + deepLink?: DeepLink; + /** + * Outcome of parsing the initial-URL deep link, surfaced as `data-deeplink` + * on the `connection-status` testid. Distinguishes "no deep link" from + * "rejected" (token mismatch / bad serverUrl) — both leave `data-status` + * idle, so an automated driver otherwise cannot tell them apart. + */ + deepLinkStatus?: DeepLinkParseStatus; + // Server list (static config; runtime connection state comes from the // separate fields below and is merged into each card by this component). servers: ServerEntry[]; @@ -324,6 +341,13 @@ export interface InspectorViewProps { */ erroredServerId?: string; connectionStatus: ConnectionStatus; + /** + * Last connection-level error message (handshake failure, OAuth start + * failure, deep-link automation failure). Surfaced as `data-error-message` + * on the header's `connection-status` testid so an automated driver can read + * *why* a connect failed without scraping a transient toast. + */ + connectErrorMessage?: string; initializeResult?: InitializeResult; latencyMs?: number; @@ -497,11 +521,14 @@ export interface InspectorViewProps { } export function InspectorView({ + deepLink, + deepLinkStatus, servers: serversInput, serverListWritable = true, activeServer, erroredServerId, connectionStatus, + connectErrorMessage, initializeResult, latencyMs, tools, @@ -841,6 +868,55 @@ export function InspectorView({ ? firstNonServersTab : SERVERS_TAB; + // Deep-link auto-open (#1577): once connected and the requested app appears in + // the app-tools list, switch to the Apps tab and pre-select it with the + // supplied form values. The `autoOpen` flag is forwarded to AppsScreen (which + // owns the running/iframe state) to fire the actual "Open App" — see its + // `autoOpen` prop. Guarded by a ref so it fires exactly once even though the + // effect re-runs as `appTools`/`availableTabs` settle. + const deepLinkOpenAppRef = useRef(false); + useEffect(() => { + if (!deepLink?.openApp) return; + if (deepLinkOpenAppRef.current) return; + if (connectionStatus !== "connected") return; + if (!availableTabs.includes("Apps")) return; + const target = appTools.find((t) => t.name === deepLink.openApp); + if (!target) return; + deepLinkOpenAppRef.current = true; + // Seed the schema's defaults THEN overlay the deep-link's appArgs. Without + // the defaults, a required field the form would display with its default + // value is absent from `formValues`, the schema-form's validity check + // fails, and Open App is silently disabled — an automated driver's click + // then no-ops and the iframe-wait spins forever. + const formValues = { + ...collectSchemaDefaults(target.inputSchema), + ...deepLink.appArgs, + }; + // Seed the selection directly rather than routing through + // AppsScreen.handleSelect. This deliberately bypasses handleSelect's + // no-input-app auto-launch: a deep link must never invoke a tool against + // the target server unless the token-gated `autoOpen` is set — even a + // no-input app waits for that explicit signal (or a manual "Open App" + // click), matching the security stance that a crafted URL alone can't fire + // a tool call. + onActiveTabChange("Apps"); + onAppsUiChange({ + ...appsUi, + selectedAppName: target.name, + formValues, + }); + onSelectApp(target.name); + }, [ + deepLink, + connectionStatus, + availableTabs, + appTools, + appsUi, + onActiveTabChange, + onAppsUiChange, + onSelectApp, + ]); + // The monitor tab shown in the column, clamped to what's available (e.g. a // switch to a stdio server drops Network). `?? LOGS_TAB` is a types-only // fallback: the column only renders while `monitorAvailable.length > 0`. @@ -1037,7 +1113,12 @@ export function InspectorView({ // Main-slot height clamp + overflow:hidden keep that scroll on the inner // ScrollArea regions only. - + {connectionStatus === "connected" && initializeResult ? ( diff --git a/clients/web/src/utils/deepLink.test.ts b/clients/web/src/utils/deepLink.test.ts new file mode 100644 index 000000000..14d32dd15 --- /dev/null +++ b/clients/web/src/utils/deepLink.test.ts @@ -0,0 +1,199 @@ +import { describe, it, expect } from "vitest"; +import { + parseDeepLink, + deepLinkConfigEquals, + deepLinkParseStatus, + DEEP_LINK_SERVER_ID, +} from "./deepLink"; + +const TOKEN = "tok-abc"; + +describe("parseDeepLink", () => { + it("returns undefined when no serverUrl param is present", () => { + expect(parseDeepLink("?autoConnect=" + TOKEN, TOKEN)).toBeUndefined(); + }); + + it("returns undefined when autoConnect is missing", () => { + expect( + parseDeepLink("?serverUrl=https%3A%2F%2Fexample.com%2Fmcp", TOKEN), + ).toBeUndefined(); + }); + + it("rejects when autoConnect does not match the session token (CSRF guard)", () => { + expect( + parseDeepLink( + "?serverUrl=https%3A%2F%2Fexample.com%2Fmcp&autoConnect=1", + TOKEN, + ), + ).toBeUndefined(); + }); + + it("rejects when there is no session token to compare against", () => { + expect( + parseDeepLink( + "?serverUrl=https%3A%2F%2Fexample.com%2Fmcp&autoConnect=" + TOKEN, + undefined, + ), + ).toBeUndefined(); + }); + + it("rejects non-http(s) serverUrl schemes", () => { + for (const url of [ + "javascript:alert(1)", + "file:///etc/passwd", + "data:text/html,