diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1baf42372..0fa8f0132 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -199,6 +199,30 @@ sidecar a given run lands on) rather than reimplementing execution. Provisioning and allocation follow Interchange's own contracts; workbench does not maintain a parallel scheduler. +## MCP connect + +Remote MCP servers connect through Plugins (curated presets and +add-by-URL). The connect-time probe and later tool calls share one +origin-pinned fetch: every first hop must be the stored origin or an +explicit extra origin for that pin — not a host-suffix match — and a 3xx +is never followed, even to an allowlisted origin. Canva is the one +shipped extra: the stored MCP origin may also first-hop the protocol +origin that is not the stored `apiBaseUrl`. + +OAuth presets that list advertised scopes send those scopes on RFC 7591 +dynamic client registration; presets that omit the list stay on the SDK's +protected-resource metadata fallback. When `/start` fails, the return +distinguishes `client_rejected` (the authorization server refused +Workbench as a client — including RFC 7591 `invalid_redirect_uri` and +sibling client-metadata codes, even when the SDK maps an unknown code +onto a generic server error) from `discovery_failed` (the authorization +server could not be reached). A successful callback re-probes with the +new token and may put that probe's tool count on the Plugins return so +the row can show it. + +Live Canva OAuth against Canva's own servers is not verified; this is +the shipped control flow, not a proven live handshake. + ## Related docs - [docs/GLOSSARY.md](docs/GLOSSARY.md) — product-term to platform-term diff --git a/IMPLEMENTATION.md b/IMPLEMENTATION.md index e13ae65a8..e00ff92b5 100644 --- a/IMPLEMENTATION.md +++ b/IMPLEMENTATION.md @@ -203,6 +203,39 @@ specialist's own 1:1 — never an invite into Myra's DM. A create-succeeded / mint-failed split is a completed tool result that names both halves, not a bare error. +## Canva MCP connect (shipped) + +Canva is the `canva` MCP preset (`packages/connections/src/mcp-presets.ts`): +`https://mcp.canva.com/mcp`, `connectionMode: "oauth"`, with the 16 +advertised PRM scopes space-joined onto RFC 7591 DCR `clientMetadata.scope` +(`createMcpOAuthProvider` in `packages/connections/src/mcp-oauth.ts`). +Other presets omit `oauthScopes` and stay on the SDK's SEP-835 PRM +fallback. + +Connect-time probe and credential fetch share +`mcpOriginPinnedFetch` (`packages/credential-providers/src/mcp-origin-pinned-fetch.ts`): +pin to the stored origin, extra first hop only +`https://mcp.canva.com` → `https://canva.ai` (not a host suffix), +`redirect: "manual"` so a 302 is never followed. `/start` classifies +DCR/client refusal as `client_rejected` versus unreachable discovery as +`discovery_failed` (`packages/connections/src/mcp-oauth-routes.ts`). +RFC 7591 `invalid_redirect_uri` (and `invalid_client_metadata`, +`invalid_client`, `unauthorized_client`) count as `client_rejected`; the +route clones 4xx/5xx JSON before the MCP SDK 1.30.0 maps unknown codes +onto `ServerError`. + +A successful OAuth callback probes with the new token and, on success, +appends `toolCount` to the Plugins return query. The Canva row +(`packages/plugins-ui/src/mcp-preset-cards.tsx`) shows that count when +it is a non-negative integer; otherwise the row stays "Connected". +`@corbits/mcp-tools` per-request timeout is two minutes +(`MCP_REQUEST_TIMEOUT_MS` in `packages/mcp-tools/src/mcp-client.ts`) — +above the SDK's 60s default, below a five-minute chat turn. + +These are unit-tested control-flow facts. Live Canva OAuth against +Canva's own servers is **not** verified; do not document a proven live +handshake. + ## Related docs - [README.md](README.md) — quickstart, local setup, repo layout, e2e detail @@ -223,3 +256,5 @@ names both halves, not a bare error. - Whether Pulumi stacks/config live in this repo or a separate infrastructure repo is not established in the docs reviewed for this pass. +- Live Canva MCP OAuth (DCR, redirect allowlist, and post-OAuth probe + against `mcp.canva.com` / `canva.ai`) is not verified as of CL-7083. diff --git a/PRODUCT.md b/PRODUCT.md index e3cce3846..1d852a47d 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -137,6 +137,20 @@ extend what an agent knows — both are installable, both are scoped to the bench or workbench that installs them, and neither requires touching platform internals. +The Plugins rail is how a person connects remote MCP servers: curated +preset cards plus an add-by-URL path. Canva is an OAuth preset — Connect +sends them through that app's sign-in, then back to Plugins. After a +successful OAuth return, the row can show how many tools the connect +probe found; a missing or non-integer count stays a bare "Connected". If +sign-in cannot start, Plugins distinguishes an unreachable authorization +server from the app rejecting Workbench as a client (redirect URL or +registration). Agent MCP tool calls are allowed two minutes so a slow +design tool can finish inside a chat turn. + +Live Canva OAuth against Canva's own servers is **not** verified as of +CL-7083. This documents the shipped connect path, not a proven live +handshake. + ## Workbench settings Each workbench has its own full-stage settings surface, not a dialog — @@ -217,6 +231,14 @@ user-facing surfaces use the rest of the product vocabulary above. generic `connections/pending` still wakes the asking agent. A leftover agent 401 after GitHub already succeeded is still a first-minute bug — see IMPLEMENTATION.md; do not document that it cannot happen. +- Connected/settle honesty (no stale Connect after success; settle never + posting as the signed-in user; no agent 401 after GitHub already + succeeded) stays **target** until CL-6737 and CL-6738 land — see + IMPLEMENTATION.md open questions; do not document those guarantees as + shipped. +- Live Canva MCP OAuth (sign-in, DCR, and post-OAuth probe against + Canva's own servers) is not verified; do not document a proven live + Canva handshake. - The precise boundary of what Insights surfaces to a non-admin bench member (all tenant activity vs. only their own) is not spelled out in `packages/insights`'s own docs as of this writing. diff --git a/apps/hub/src/launch-caches.test.ts b/apps/hub/src/launch-caches.test.ts index fa5e22969..20668d7f6 100644 --- a/apps/hub/src/launch-caches.test.ts +++ b/apps/hub/src/launch-caches.test.ts @@ -154,6 +154,24 @@ describe("createLaunchCaches: assetService reads", () => { expect(inner.listCalls.length).toBe(1); }); + test("lists an empty catalog when the package-registry has no resolvable main", async () => { + const heads = new Map(); + const inner = countingAssetService(heads); + const { repoStore } = countingRepoStore(heads); + const caches = createLaunchCaches({ + assetService: inner.assetService, + repoStore, + }); + + const listed = await caches.assetService.listAssetBlobs({ + assetId: ASSET_ID, + dir: "tarballs", + }); + + expect(listed).toEqual([]); + expect(inner.listCalls.length).toBe(0); + }); + test("delegates createAsset and populateAsset untouched", () => { const heads = new Map([[HEAD_REF, "sha-1"]]); const inner = countingAssetService(heads); diff --git a/apps/hub/src/launch-caches.ts b/apps/hub/src/launch-caches.ts index 9dcf7799b..0ca019d7c 100644 --- a/apps/hub/src/launch-caches.ts +++ b/apps/hub/src/launch-caches.ts @@ -158,7 +158,11 @@ export function createLaunchCaches(deps: { params: ListAssetBlobsParams, ): Promise { const sha = await resolvePackageRegistryHeadSha(params.assetId, params.ref); - if (sha === null) return assetService.listAssetBlobs(params); + // No resolvable `main` means no tarballs yet. Match the tarball REST + // list, which returns [] on this same not_found rather than failing + // the launch. Pins against a missing tarball still fail as unknown + // package. + if (sha === null) return []; const key = `${params.assetId}:${sha}:${params.dir}`; const cached = listCache.get(key); if (cached !== undefined) return cached; diff --git a/bun.lock b/bun.lock index cc062a7b7..6db572c5c 100644 --- a/bun.lock +++ b/bun.lock @@ -982,7 +982,7 @@ }, "packages/mcp-tools": { "name": "@corbits/mcp-tools", - "version": "0.0.8", + "version": "0.0.10", "dependencies": { "@intx/agent": "workspace:*", "@intx/types": "workspace:*", diff --git a/packages/connections/src/mcp-oauth-routes.test.ts b/packages/connections/src/mcp-oauth-routes.test.ts index 4515d86e5..90c6c75bd 100644 --- a/packages/connections/src/mcp-oauth-routes.test.ts +++ b/packages/connections/src/mcp-oauth-routes.test.ts @@ -14,6 +14,7 @@ import { createNoopCredentialCipher } from "@intx/crypto"; import type { RequireGrant, TenantEnv } from "@intx/hub-api"; import type { ApiCall } from "@workbench/hub-client"; import { createMcpOAuthRoutes } from "./mcp-oauth-routes"; +import { mcpPresetBySlug } from "./mcp-presets"; import type { McpProbeResult } from "./mcp-probe"; const TENANT = { @@ -66,17 +67,22 @@ function startStubAuthorizationServer( /** CL-6371 red-path fixture: a provider that never echoes `state` * back on the authorize redirect, even though we sent one. */ echoState?: boolean; + /** RFC 7591 registration failure: `/register` returns this instead of + * issuing a client (Canva-style DCR 4xx). */ + registrationError?: { status: number; body: unknown }; } = {}, ): { origin: string; resourcePath: string; stop: () => void; issuedCodes: Map; + registrationBodies: unknown[]; } { const issuedCodes = new Map< string, { codeChallenge: string; clientId: string } >(); + const registrationBodies: unknown[] = []; let nextClientId = 1; const clients = new Map(); @@ -105,12 +111,20 @@ function startStubAuthorizationServer( }); } if (url.pathname === "/register" && req.method === "POST") { - const body = (await req.json()) as { redirect_uris: string[] }; + const body: unknown = await req.json(); + registrationBodies.push(body); + if (tokenGrant.registrationError !== undefined) { + return Response.json(tokenGrant.registrationError.body, { + status: tokenGrant.registrationError.status, + }); + } + const redirectUris = (body as { redirect_uris: string[] }) + .redirect_uris; const clientId = `client_${nextClientId++}`; - clients.set(clientId, { redirectUris: body.redirect_uris }); + clients.set(clientId, { redirectUris }); return Response.json({ client_id: clientId, - redirect_uris: body.redirect_uris, + redirect_uris: redirectUris, token_endpoint_auth_method: "none", grant_types: ["authorization_code", "refresh_token"], response_types: ["code"], @@ -170,6 +184,7 @@ function startStubAuthorizationServer( resourcePath: `http://localhost:${server.port}/mcp`, stop: () => server.stop(true), issuedCodes, + registrationBodies, }; } @@ -329,11 +344,302 @@ describe("MCP OAuth connect flow", () => { expect(response.headers.get("set-cookie") ?? "").toContain( "workbench_mcp_oauth_exa=", ); + expect(as.registrationBodies).toHaveLength(1); + const dcrBody = as.registrationBodies[0]; + expect( + dcrBody !== null && typeof dcrBody === "object" && "scope" in dcrBody, + ).toBe(false); + } finally { + as.stop(); + } + }); + + test("preset Canva start posts the 16 space-joined scopes on the RFC 7591 DCR body", async () => { + const as = startStubAuthorizationServer(); + const originalFetch = globalThis.fetch; + const canvaOrigin = "https://mcp.canva.com"; + // `/canva/start` with no `?url=` discovers the preset origin. Rewrite + // that host onto the stub AS (and rewrite advertised origins back) so + // this stays a loopback test while still exercising the no-override + // path that actually joins `preset.oauthScopes`. + globalThis.fetch = Object.assign( + async (input: string | URL | Request, init?: RequestInit) => { + const href = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if (!href.startsWith(canvaOrigin)) { + return originalFetch(input, init); + } + const rewritten = href.replace(canvaOrigin, as.origin); + const response = + input instanceof Request + ? await originalFetch(new Request(rewritten, input), init) + : await originalFetch(rewritten, init); + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.includes("json")) { + return response; + } + const body = (await response.text()).replaceAll(as.origin, canvaOrigin); + return new Response(body, { + status: response.status, + headers: response.headers, + }); + }, + originalFetch, + ); + try { + const hub = fakeHub(); + const routes = createMcpOAuthRoutes({ + hubUrl: "http://hub.test", + requireGrant: allowAll, + log: () => {}, + credentialCipher: createNoopCredentialCipher(), + apiCall: hub.apiCall, + }); + const app = mountAs(routes); + + const response = await app.request("/canva/start", { + redirect: "manual", + }); + + expect(response.status).toBe(302); + const location = response.headers.get("location") ?? ""; + expect(location.startsWith(`${canvaOrigin}/authorize`)).toBe(true); + expect(as.registrationBodies).toHaveLength(1); + const canvaScopes = mcpPresetBySlug("canva")?.oauthScopes; + expect(canvaScopes).toHaveLength(16); + const dcrBody = as.registrationBodies[0]; + expect(dcrBody).toEqual( + expect.objectContaining({ + scope: canvaScopes?.join(" "), + }), + ); + } finally { + globalThis.fetch = originalFetch; + as.stop(); + } + }); + + test("start redirects client_rejected when DCR returns invalid_client_metadata", async () => { + const as = startStubAuthorizationServer({ + registrationError: { + status: 400, + body: { + error: "invalid_client_metadata", + error_description: "redirect URI is not allowlisted", + }, + }, + }); + const originalFetch = globalThis.fetch; + const canvaOrigin = "https://mcp.canva.com"; + globalThis.fetch = Object.assign( + async (input: string | URL | Request, init?: RequestInit) => { + const href = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if (!href.startsWith(canvaOrigin)) { + return originalFetch(input, init); + } + const rewritten = href.replace(canvaOrigin, as.origin); + const response = + input instanceof Request + ? await originalFetch(new Request(rewritten, input), init) + : await originalFetch(rewritten, init); + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.includes("json")) { + return response; + } + const body = (await response.text()).replaceAll(as.origin, canvaOrigin); + return new Response(body, { + status: response.status, + headers: response.headers, + }); + }, + originalFetch, + ); + try { + const hub = fakeHub(); + const routes = createMcpOAuthRoutes({ + hubUrl: "http://hub.test", + requireGrant: allowAll, + log: () => {}, + credentialCipher: createNoopCredentialCipher(), + apiCall: hub.apiCall, + }); + const app = mountAs(routes); + + const response = await app.request("/canva/start", { + redirect: "manual", + }); + + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe( + "/plugins?mcpOauth=canva&outcome=error&code=client_rejected", + ); + expect(as.registrationBodies).toHaveLength(1); } finally { + globalThis.fetch = originalFetch; as.stop(); } }); + test("start redirects client_rejected when DCR returns invalid_redirect_uri", async () => { + const as = startStubAuthorizationServer({ + registrationError: { + status: 400, + body: { + error: "invalid_redirect_uri", + error_description: "The redirection URI is not allowed.", + }, + }, + }); + const originalFetch = globalThis.fetch; + const canvaOrigin = "https://mcp.canva.com"; + globalThis.fetch = Object.assign( + async (input: string | URL | Request, init?: RequestInit) => { + const href = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if (!href.startsWith(canvaOrigin)) { + return originalFetch(input, init); + } + const rewritten = href.replace(canvaOrigin, as.origin); + const response = + input instanceof Request + ? await originalFetch(new Request(rewritten, input), init) + : await originalFetch(rewritten, init); + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.includes("json")) { + return response; + } + const body = (await response.text()).replaceAll(as.origin, canvaOrigin); + return new Response(body, { + status: response.status, + headers: response.headers, + }); + }, + originalFetch, + ); + try { + const hub = fakeHub(); + const routes = createMcpOAuthRoutes({ + hubUrl: "http://hub.test", + requireGrant: allowAll, + log: () => {}, + credentialCipher: createNoopCredentialCipher(), + apiCall: hub.apiCall, + }); + const app = mountAs(routes); + + const response = await app.request("/canva/start", { + redirect: "manual", + }); + + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe( + "/plugins?mcpOauth=canva&outcome=error&code=client_rejected", + ); + expect(as.registrationBodies).toHaveLength(1); + } finally { + globalThis.fetch = originalFetch; + as.stop(); + } + }); + + test("start redirects client_rejected when DCR returns invalid_redirect_uri with no description", async () => { + const as = startStubAuthorizationServer({ + registrationError: { + status: 400, + body: { + error: "invalid_redirect_uri", + }, + }, + }); + const originalFetch = globalThis.fetch; + const canvaOrigin = "https://mcp.canva.com"; + globalThis.fetch = Object.assign( + async (input: string | URL | Request, init?: RequestInit) => { + const href = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if (!href.startsWith(canvaOrigin)) { + return originalFetch(input, init); + } + const rewritten = href.replace(canvaOrigin, as.origin); + const response = + input instanceof Request + ? await originalFetch(new Request(rewritten, input), init) + : await originalFetch(rewritten, init); + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.includes("json")) { + return response; + } + const body = (await response.text()).replaceAll(as.origin, canvaOrigin); + return new Response(body, { + status: response.status, + headers: response.headers, + }); + }, + originalFetch, + ); + try { + const hub = fakeHub(); + const routes = createMcpOAuthRoutes({ + hubUrl: "http://hub.test", + requireGrant: allowAll, + log: () => {}, + credentialCipher: createNoopCredentialCipher(), + apiCall: hub.apiCall, + }); + const app = mountAs(routes); + + const response = await app.request("/canva/start", { + redirect: "manual", + }); + + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe( + "/plugins?mcpOauth=canva&outcome=error&code=client_rejected", + ); + expect(as.registrationBodies).toHaveLength(1); + } finally { + globalThis.fetch = originalFetch; + as.stop(); + } + }); + + test("start redirects discovery_failed when the authorization server is unreachable", async () => { + const hub = fakeHub(); + const routes = createMcpOAuthRoutes({ + hubUrl: "http://hub.test", + requireGrant: allowAll, + log: () => {}, + credentialCipher: createNoopCredentialCipher(), + apiCall: hub.apiCall, + }); + const app = mountAs(routes); + const response = await app.request( + `/canva/start?url=${encodeURIComponent("http://127.0.0.1:1/mcp")}`, + { redirect: "manual" }, + ); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe( + "/plugins?mcpOauth=canva&outcome=error&code=discovery_failed", + ); + }); + test("callback completes the token exchange and stores a bearer credential", async () => { const as = startStubAuthorizationServer(); try { diff --git a/packages/connections/src/mcp-oauth-routes.ts b/packages/connections/src/mcp-oauth-routes.ts index ce5a29328..2ea0f4dfa 100644 --- a/packages/connections/src/mcp-oauth-routes.ts +++ b/packages/connections/src/mcp-oauth-routes.ts @@ -80,6 +80,90 @@ function cookieName(slug: string): string { return `workbench_mcp_oauth_${slug}`; } +const CLIENT_REJECTED_OAUTH_CODES = new Set([ + "invalid_client_metadata", + "invalid_redirect_uri", + "invalid_client", + "unauthorized_client", +]); + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object"; +} + +function readString(value: unknown, key: string): string | undefined { + if (!isRecord(value)) return undefined; + const candidate = value[key]; + return typeof candidate === "string" && candidate.length > 0 + ? candidate + : undefined; +} + +/** Classify `/start` `auth()` throws: DCR/client rejection vs unreachable + * discovery. Prefer the RFC 7591 `error` captured from the HTTP body — + * SDK 1.30.0 maps unknown codes such as `invalid_redirect_uri` to + * `ServerError` (`errorCode` `server_error`) and drops the original. + * Then `errorCode`/`error` on the thrown object. Fall back to + * registration wording in the message so "redirect URI is not + * allowlisted" still counts when the body was not JSON. */ +function mcpOAuthStartErrorCode( + cause: unknown, + capturedCode: string | undefined, +): "discovery_failed" | "client_rejected" { + for (const code of [ + capturedCode, + readString(cause, "errorCode"), + readString(cause, "error"), + ]) { + if (code !== undefined && CLIENT_REJECTED_OAUTH_CODES.has(code)) { + return "client_rejected"; + } + } + + const message = cause instanceof Error ? cause.message : String(cause); + const lower = message.toLowerCase(); + for (const code of CLIENT_REJECTED_OAUTH_CODES) { + if (lower.includes(code)) { + return "client_rejected"; + } + } + + if ( + lower.includes("client metadata") || + lower.includes("redirect uri") || + lower.includes("redirect_uri") || + lower.includes("redirection uri") || + lower.includes("redirection_uri") || + lower.includes("client registration") + ) { + return "client_rejected"; + } + return "discovery_failed"; +} + +/** Clone 4xx/5xx JSON before the SDK's `parseErrorResponse` consumes the + * body and maps unknown RFC 7591 codes onto `ServerError`. */ +async function fetchCapturingOAuthError( + captured: { code: string | undefined }, + url: string | URL, + init?: RequestInit, +): Promise { + const response = await fetch(url, init); + if (response.status < 400) return response; + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.includes("json")) return response; + try { + const body: unknown = await response.clone().json(); + const code = readString(body, "error"); + if (code !== undefined) { + captured.code = code; + } + } catch { + // malformed JSON on an error response; classifier uses the thrown error + } + return response; +} + export type CreateMcpOAuthRoutesDeps = { hubUrl: string; requireGrant: RequireGrant; @@ -196,11 +280,21 @@ export function createMcpOAuthRoutes( callbackUrl, clientName: "Corbits Workbench", session, + ...(preset?.oauthScopes === undefined + ? {} + : { scope: preset.oauthScopes.join(" ") }), }); let result: Awaited>; + const capturedOAuthError: { code: string | undefined } = { + code: undefined, + }; try { - result = await auth(provider, { serverUrl: target.url }); + result = await auth(provider, { + serverUrl: target.url, + fetchFn: (url, init) => + fetchCapturingOAuthError(capturedOAuthError, url, init), + }); } catch (cause) { const message = cause instanceof Error ? cause.message : String(cause); deps.log(`mcp oauth start failed for "${target.slug}": ${message}`); @@ -208,7 +302,7 @@ export function createMcpOAuthRoutes( redirectPath(returnPath, { mcpOauth: target.slug, outcome: "error", - code: "discovery_failed", + code: mcpOAuthStartErrorCode(cause, capturedOAuthError.code), }), 302, ); @@ -344,10 +438,14 @@ export function createMcpOAuthRoutes( } : {}), }; + const callbackPreset = mcpPresetBySlug(payload.slug); const provider = createMcpOAuthProvider({ callbackUrl, clientName: "Corbits Workbench", session, + ...(callbackPreset?.oauthScopes === undefined + ? {} + : { scope: callbackPreset.oauthScopes.join(" ") }), }); let result: Awaited>; diff --git a/packages/connections/src/mcp-oauth.test.ts b/packages/connections/src/mcp-oauth.test.ts new file mode 100644 index 000000000..847b4c988 --- /dev/null +++ b/packages/connections/src/mcp-oauth.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, test } from "bun:test"; +import { createMcpOAuthProvider } from "./mcp-oauth"; + +describe("createMcpOAuthProvider", () => { + test("clientMetadata includes scope only when it is passed", () => { + const session = { state: "oauth-state" }; + const withScope = createMcpOAuthProvider({ + callbackUrl: "http://hub.test/callback", + clientName: "Corbits Workbench", + session, + scope: "profile:read asset:read", + }); + expect(withScope.clientMetadata).toEqual({ + client_name: "Corbits Workbench", + redirect_uris: ["http://hub.test/callback"], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + scope: "profile:read asset:read", + }); + + const withoutScope = createMcpOAuthProvider({ + callbackUrl: "http://hub.test/callback", + clientName: "Corbits Workbench", + session, + }); + expect("scope" in withoutScope.clientMetadata).toBe(false); + expect(withoutScope.clientMetadata).toEqual({ + client_name: "Corbits Workbench", + redirect_uris: ["http://hub.test/callback"], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }); + }); +}); diff --git a/packages/connections/src/mcp-oauth.ts b/packages/connections/src/mcp-oauth.ts index 2c239da52..53a9eceed 100644 --- a/packages/connections/src/mcp-oauth.ts +++ b/packages/connections/src/mcp-oauth.ts @@ -51,6 +51,7 @@ export function createMcpOAuthProvider(args: { readonly callbackUrl: string; readonly clientName: string; readonly session: McpOAuthSession; + readonly scope?: string; }): OAuthClientProvider { const { session } = args; let authorizationUrl: URL | undefined; @@ -66,6 +67,7 @@ export function createMcpOAuthProvider(args: { grant_types: ["authorization_code", "refresh_token"], response_types: ["code"], token_endpoint_auth_method: "none", + ...(args.scope === undefined ? {} : { scope: args.scope }), }; }, state(): string { diff --git a/packages/connections/src/mcp-presets.test.ts b/packages/connections/src/mcp-presets.test.ts index 63768e1b7..b757aae67 100644 --- a/packages/connections/src/mcp-presets.test.ts +++ b/packages/connections/src/mcp-presets.test.ts @@ -112,4 +112,29 @@ describe("MCP_PRESETS", () => { expect(mcpPresetBySlug("canva")?.connectionMode).toBe("oauth"); expect(mcpPresetBySlug("canva")?.icon).toBeUndefined(); }); + + test("Canva lists the 16 live PRM OAuth scopes; other presets omit oauthScopes", () => { + expect(mcpPresetBySlug("canva")?.oauthScopes).toEqual([ + "profile:read", + "design:meta:read", + "design:content:write", + "design:content:read", + "folder:read", + "folder:write", + "brandtemplate:content:read", + "brandtemplate:meta:read", + "brandtemplate:content:write", + "comment:write", + "comment:read", + "asset:read", + "asset:write", + "brandkit:read", + "help:answers:read", + "help:answers:write", + ]); + for (const preset of MCP_PRESETS) { + if (preset.slug === "canva") continue; + expect("oauthScopes" in preset).toBe(false); + } + }); }); diff --git a/packages/connections/src/mcp-presets.ts b/packages/connections/src/mcp-presets.ts index e208aa0bc..faea601e4 100644 --- a/packages/connections/src/mcp-presets.ts +++ b/packages/connections/src/mcp-presets.ts @@ -21,6 +21,10 @@ export type McpPreset = { * renders above the paste field — each step one action a person can * take, ending with what happens to the token. */ readonly tokenSteps?: readonly string[]; + /** Space-joined into RFC 7591 DCR `clientMetadata.scope` when this + * preset's OAuth connect runs. Omit the key — other presets stay on + * the SDK's SEP-835 PRM fallback. */ + readonly oauthScopes?: readonly string[]; }; /** @@ -140,6 +144,24 @@ export const MCP_PRESETS: readonly McpPreset[] = [ url: "https://mcp.canva.com/mcp", connectionMode: "oauth", docsUrl: "https://www.canva.dev/docs/mcp/", + oauthScopes: [ + "profile:read", + "design:meta:read", + "design:content:write", + "design:content:read", + "folder:read", + "folder:write", + "brandtemplate:content:read", + "brandtemplate:meta:read", + "brandtemplate:content:write", + "comment:write", + "comment:read", + "asset:read", + "asset:write", + "brandkit:read", + "help:answers:read", + "help:answers:write", + ], }, ]; diff --git a/packages/connections/src/mcp-probe.test.ts b/packages/connections/src/mcp-probe.test.ts index 1cf07e405..c5d7a8e20 100644 --- a/packages/connections/src/mcp-probe.test.ts +++ b/packages/connections/src/mcp-probe.test.ts @@ -1,7 +1,8 @@ // Exercises `probeMcpServer` against real HTTP servers on loopback ports // (never the real network) — a real MCP server, a plain 401 with no OAuth // metadata behind it, and a 401 that does advertise RFC 9728/8414 metadata -// (the OAuth-gated shape this probe is meant to recognize). +// (the OAuth-gated shape this probe is meant to recognize). Unpinned fetch +// follows a 302 to another origin; the probe must not. import { describe, expect, test } from "bun:test"; import { probeMcpServer } from "./mcp-probe"; @@ -84,4 +85,38 @@ describe("probeMcpServer", () => { stub.stop(); } }); + + test("a 302 to another origin does not succeed", async () => { + let destinationHits = 0; + const destination = Bun.serve({ + hostname: "localhost", + port: 0, + fetch: () => { + destinationHits += 1; + return new Response("ok"); + }, + }); + const redirector = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch: () => + new Response(null, { + status: 302, + headers: { + Location: `http://localhost:${String(destination.port)}/mcp`, + }, + }), + }); + try { + const result = await probeMcpServer( + `http://127.0.0.1:${String(redirector.port)}/mcp`, + undefined, + ); + expect(result.ok).toBe(false); + expect(destinationHits).toBe(0); + } finally { + redirector.stop(true); + destination.stop(true); + } + }); }); diff --git a/packages/connections/src/mcp-probe.ts b/packages/connections/src/mcp-probe.ts index 2d9e5b706..187d56e85 100644 --- a/packages/connections/src/mcp-probe.ts +++ b/packages/connections/src/mcp-probe.ts @@ -5,6 +5,7 @@ // second hand-rolled MCP client: the exact transport/session mechanics // `mcp_list_tools` uses at call time, run once at connect time to prove // the URL is a real MCP server before it is ever stored. +import { mcpOriginPinnedFetch } from "@corbits/credential-providers"; import { withMcpConnection, listMcpTools } from "@corbits/mcp-tools"; import { discoverOAuthServerInfo } from "@modelcontextprotocol/sdk/client/auth.js"; @@ -51,16 +52,16 @@ async function discoverOAuthRequirement( } } -/** Builds the bearer-authenticated fetch the probe (and nothing durable) - * uses: a plain `fetch` injecting `authorization` when a token was - * pasted, with no origin pinning — this is the one-shot pre-storage - * check, before a provider row (and its pinned origin) exists at all. */ -function probeFetch(token: string | undefined): typeof fetch { - if (token === undefined || token.length === 0) return fetch; - return ((input, init) => { - const headers = new Headers(init?.headers); - headers.set("authorization", `Bearer ${token}`); - return fetch(input as string | URL, { ...init, headers }); +/** Builds the origin-pinned fetch the probe (and nothing durable) uses: + * the same helper `mcp_call` later uses via the credential provider, pinned + * to this server's origin so a 3xx cannot leak a bearer (or a keyless + * handshake) off the URL the person pasted. Keyless probes pin too. */ +function probeFetch(url: string, token: string | undefined): typeof fetch { + const pinnedOrigin = new URL(url).origin; + return mcpOriginPinnedFetch({ + pinnedOrigin, + readToken: () => + token === undefined || token.length === 0 ? undefined : token, }) as typeof fetch; } @@ -85,7 +86,7 @@ export async function probeMcpServer( try { const tools = await withMcpConnection( - { url, fetchImpl: probeFetch(token) }, + { url, fetchImpl: probeFetch(url, token) }, (client) => listMcpTools(client), ); return { ok: true, toolCount: tools.length }; diff --git a/packages/connections/src/mcp-server-routes.test.ts b/packages/connections/src/mcp-server-routes.test.ts index 2e62d8ce4..9190646a5 100644 --- a/packages/connections/src/mcp-server-routes.test.ts +++ b/packages/connections/src/mcp-server-routes.test.ts @@ -403,6 +403,19 @@ describe("GET /presets", () => { expect(bySlug.get("exa")?.connected).toBe(true); expect(bySlug.get("granola")?.connected).toBe(false); }); + + test("does not expose oauthScopes on the public presets JSON", async () => { + const hub = fakeHub({}); + const app = buildApp({ apiCall: hub.apiCall }); + + const response = await app.request("/presets"); + expect(response.status).toBe(200); + const body = (await response.json()) as { data: Record[] }; + expect(body.data.length).toBeGreaterThan(0); + for (const preset of body.data) { + expect("oauthScopes" in preset).toBe(false); + } + }); }); describe("POST / with presetSlug", () => { diff --git a/packages/credential-providers/src/index.ts b/packages/credential-providers/src/index.ts index 906346c41..9be63ed93 100644 --- a/packages/credential-providers/src/index.ts +++ b/packages/credential-providers/src/index.ts @@ -27,3 +27,10 @@ export { MCP_STREAMABLE_HTTP_PROVIDER_KEY, } from "./mcp-streamable-http-provider"; export type { McpStreamableHttpCredentialProviderOptions } from "./mcp-streamable-http-provider"; + +export { + assertMcpPinnedTarget, + mcpOriginPinnedFetch, + resolveMcpTargetUrl, +} from "./mcp-origin-pinned-fetch"; +export type { McpOriginPinnedFetchArgs } from "./mcp-origin-pinned-fetch"; diff --git a/packages/credential-providers/src/mcp-origin-pinned-fetch.ts b/packages/credential-providers/src/mcp-origin-pinned-fetch.ts new file mode 100644 index 000000000..645b0cce0 --- /dev/null +++ b/packages/credential-providers/src/mcp-origin-pinned-fetch.ts @@ -0,0 +1,98 @@ +// Shared origin pin for MCP Streamable HTTP: the credential provider and +// the connect-time probe must refuse the same cross-origin first hops and +// never follow a 3xx (even to an allowlisted origin). Extra origins are +// an explicit map keyed by the pinned origin — not a host suffix. + +import type { FetchLike } from "./http-x-api-key-provider"; + +/** + * Additional first-hop origins a pinned MCP credential may call. Only + * `https://mcp.canva.com` → `https://canva.ai`: Canva's MCP protocol + * origin is not the stored `apiBaseUrl` origin. + */ +const MCP_PINNED_ORIGIN_EXTRAS: Readonly> = { + "https://mcp.canva.com": ["https://canva.ai"], +}; + +export interface McpOriginPinnedFetchArgs { + /** Origin the handle is pinned to (`new URL(context.origin).origin`). */ + pinnedOrigin: string; + /** Injectable `fetch`; defaults to the global `fetch`. */ + fetch?: FetchLike; + /** + * Secret for this request. Called per fetch so a rotation reaches the + * handle without a rebuild. `undefined` or empty omits `authorization` + * (keyless / sentinel already translated by the caller). + */ + readToken: () => string | undefined; +} + +/** + * Resolve the URL a request targets: a relative string resolves against + * the pinned origin, an absolute string or URL keeps its own origin, and + * a `Request` already carries an absolute URL. + */ +export function resolveMcpTargetUrl( + input: string | URL | Request, + pinnedOrigin: string, +): URL { + if (typeof input === "string") { + return new URL(input, pinnedOrigin); + } + if (input instanceof URL) { + return input; + } + return new URL(input.url); +} + +/** + * Throw if `target` is neither the pinned origin nor an explicit extra + * origin for that pin. Error text matches the mcp-streamable-http + * provider's historical refusal. + */ +export function assertMcpPinnedTarget(target: URL, pinnedOrigin: string): void { + if (target.origin === pinnedOrigin) return; + const extras = MCP_PINNED_ORIGIN_EXTRAS[pinnedOrigin]; + if (extras !== undefined && extras.includes(target.origin)) return; + throw new Error( + `mcp-streamable-http credential is pinned to ${pinnedOrigin}; refusing cross-origin request to ${target.origin}`, + ); +} + +function applyAuthorization(headers: Headers, token: string | undefined): void { + if (token === undefined || token.length === 0) { + headers.delete("authorization"); + return; + } + headers.set("authorization", `Bearer ${token}`); +} + +/** + * Fetch that origin-checks every request, injects Bearer when a token is + * present, and forces `redirect: "manual"` so a 3xx never sends a token + * (or a keyless handshake) to a foreign host. + */ +export function mcpOriginPinnedFetch( + args: McpOriginPinnedFetchArgs, +): FetchLike { + const fetchImpl: FetchLike = args.fetch ?? globalThis.fetch; + + return async ( + input: string | URL | Request, + init?: RequestInit, + ): Promise => { + const target = resolveMcpTargetUrl(input, args.pinnedOrigin); + assertMcpPinnedTarget(target, args.pinnedOrigin); + const token = args.readToken(); + + if (input instanceof Request) { + const headers = new Headers(input.headers); + applyAuthorization(headers, token); + return fetchImpl(new Request(input, { headers, redirect: "manual" })); + } + + const headers = new Headers(init?.headers); + applyAuthorization(headers, token); + return fetchImpl(target, { ...init, headers, redirect: "manual" }); + }; +} diff --git a/packages/credential-providers/src/mcp-streamable-http-provider.test.ts b/packages/credential-providers/src/mcp-streamable-http-provider.test.ts index b834fd242..0f5e31c83 100644 --- a/packages/credential-providers/src/mcp-streamable-http-provider.test.ts +++ b/packages/credential-providers/src/mcp-streamable-http-provider.test.ts @@ -2,7 +2,9 @@ // NO authorization header at all for the keyless sentinel (a public MCP // server like Exa accepts an absent header but 401s a bogus bearer), and // the same origin-pinning + manual-redirect protections its sibling -// providers enforce. +// providers enforce. Canva's MCP protocol origin `https://canva.ai` is an +// explicit extra first hop when the credential is pinned to +// `https://mcp.canva.com` — never a host-suffix, never followed via 3xx. import { describe, expect, test } from "bun:test"; import type { CredentialShapeContext } from "@intx/types"; @@ -12,14 +14,19 @@ import { MCP_STREAMABLE_HTTP_PROVIDER_KEY, } from "./mcp-streamable-http-provider"; -function contextWith(secret: string): CredentialShapeContext { +function contextWith( + secret: string, + origin = "https://mcp.example.test/mcp", +): CredentialShapeContext { return { - origin: "https://mcp.example.test/mcp", + origin, readCurrentMaterial: () => ({ secret }), } as CredentialShapeContext; } -function capturingFetch() { +function capturingFetch( + respond?: (input: string | URL | Request, init?: RequestInit) => Response, +) { const seen: { url: string; headers: Headers; redirect?: string }[] = []; const fetchImpl = async ( input: string | URL | Request, @@ -38,7 +45,7 @@ function capturingFetch() { ...(init?.redirect !== undefined ? { redirect: init.redirect } : {}), }); } - return new Response("ok"); + return respond?.(input, init) ?? new Response("ok"); }; return { seen, fetchImpl }; } @@ -95,4 +102,109 @@ describe(MCP_STREAMABLE_HTTP_PROVIDER_KEY, () => { expect(seen[0]?.headers.get("content-type")).toBe("application/json"); expect(seen[0]?.headers.get("authorization")).toBe("Bearer tok-9"); }); + + test("https://canva.ai is allowed as a first hop when pinned to https://mcp.canva.com", async () => { + const { seen, fetchImpl } = capturingFetch(); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + await shaped.fetch("https://canva.ai/mcp"); + expect(seen).toHaveLength(1); + expect(seen[0]?.url).toBe("https://canva.ai/mcp"); + expect(seen[0]?.headers.get("authorization")).toBe("Bearer tok-canva"); + expect(seen[0]?.redirect).toBe("manual"); + }); + + test("https://evil.example is refused when pinned to https://mcp.canva.com", async () => { + const { seen, fetchImpl } = capturingFetch(); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + await expect(shaped.fetch("https://evil.example/steal")).rejects.toThrow( + "mcp-streamable-http credential is pinned to https://mcp.canva.com; refusing cross-origin request to https://evil.example", + ); + expect(seen).toHaveLength(0); + }); + + test("https://notcanva.com is refused when pinned to https://mcp.canva.com", async () => { + const { seen, fetchImpl } = capturingFetch(); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + await expect(shaped.fetch("https://notcanva.com/mcp")).rejects.toThrow( + "mcp-streamable-http credential is pinned to https://mcp.canva.com; refusing cross-origin request to https://notcanva.com", + ); + expect(seen).toHaveLength(0); + }); + + test("unlisted Canva sibling https://media.canva.com is refused", async () => { + const { seen, fetchImpl } = capturingFetch(); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + await expect(shaped.fetch("https://media.canva.com/mcp")).rejects.toThrow( + "mcp-streamable-http credential is pinned to https://mcp.canva.com; refusing cross-origin request to https://media.canva.com", + ); + expect(seen).toHaveLength(0); + }); + + test("a 302 Location to https://canva.ai is not followed", async () => { + const { seen, fetchImpl } = capturingFetch( + () => + new Response(null, { + status: 302, + headers: { Location: "https://canva.ai/mcp" }, + }), + ); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + const response = await shaped.fetch("https://mcp.canva.com/mcp"); + expect(response.status).toBe(302); + expect(seen).toHaveLength(1); + expect(seen[0]?.url).toBe("https://mcp.canva.com/mcp"); + expect(seen[0]?.redirect).toBe("manual"); + }); + + test("a 302 Location off the allowlist is not followed", async () => { + const { seen, fetchImpl } = capturingFetch( + () => + new Response(null, { + status: 302, + headers: { Location: "https://evil.example/steal" }, + }), + ); + const provider = createMcpStreamableHttpCredentialProvider({ + fetch: fetchImpl, + }); + const shaped = provider.shape( + contextWith("tok-canva", "https://mcp.canva.com/mcp"), + ); + if (shaped.kind !== "http") throw new Error("expected http"); + const response = await shaped.fetch("https://mcp.canva.com/mcp"); + expect(response.status).toBe(302); + expect(seen).toHaveLength(1); + expect(seen[0]?.url).toBe("https://mcp.canva.com/mcp"); + expect(seen[0]?.redirect).toBe("manual"); + }); }); diff --git a/packages/credential-providers/src/mcp-streamable-http-provider.ts b/packages/credential-providers/src/mcp-streamable-http-provider.ts index 77cec5d65..0dacbbdf7 100644 --- a/packages/credential-providers/src/mcp-streamable-http-provider.ts +++ b/packages/credential-providers/src/mcp-streamable-http-provider.ts @@ -11,9 +11,9 @@ // // Every protection the sibling providers enforce is mirrored exactly: // the handle is pinned to the credential's origin at shape time, every -// request is re-checked against that origin, and every outbound request -// forces `redirect: "manual"` so a 3xx never sends a token to a foreign -// host. +// request is re-checked against that origin (plus an explicit extra-origin +// map), and every outbound request forces `redirect: "manual"` so a 3xx +// never sends a token to a foreign host. import type { CredentialProvider, @@ -22,6 +22,7 @@ import type { } from "@intx/types"; import type { FetchLike } from "./http-x-api-key-provider"; +import { mcpOriginPinnedFetch } from "./mcp-origin-pinned-fetch"; /** * The stored-secret sentinel for a keyless MCP-server connection. @@ -58,37 +59,17 @@ export function createMcpStreamableHttpCredentialProvider( return { kind: "http", - async fetch( - input: string | URL | Request, - init?: RequestInit, - ): Promise { - const target = resolveTargetUrl(input, pinnedOrigin); - if (target.origin !== pinnedOrigin) { - throw new Error( - `mcp-streamable-http credential is pinned to ${pinnedOrigin}; refusing cross-origin request to ${target.origin}`, - ); - } - - // Read the secret fresh on every call so a rotation of the - // underlying material cell reaches this handle without a - // rebuild. - const { secret } = context.readCurrentMaterial(); - const keyless = secret === MCP_NO_TOKEN_SENTINEL; - - if (input instanceof Request) { - const headers = new Headers(input.headers); - if (keyless) headers.delete("authorization"); - else headers.set("authorization", `Bearer ${secret}`); - return fetchImpl( - new Request(input, { headers, redirect: "manual" }), - ); - } - - const headers = new Headers(init?.headers); - if (keyless) headers.delete("authorization"); - else headers.set("authorization", `Bearer ${secret}`); - return fetchImpl(target, { ...init, headers, redirect: "manual" }); - }, + fetch: mcpOriginPinnedFetch({ + pinnedOrigin, + fetch: fetchImpl, + readToken: () => { + // Read the secret fresh on every call so a rotation of the + // underlying material cell reaches this handle without a + // rebuild. + const { secret } = context.readCurrentMaterial(); + return secret === MCP_NO_TOKEN_SENTINEL ? undefined : secret; + }, + }), dispose(): void { // An http handle allocates no resources; nothing to release. }, @@ -96,22 +77,3 @@ export function createMcpStreamableHttpCredentialProvider( }, }; } - -/** - * Resolve the URL a request targets, matching the sibling providers: a - * relative string resolves against the pinned origin, an absolute - * string or URL keeps its own origin (refused above if it differs), and - * a `Request` already carries an absolute URL. - */ -function resolveTargetUrl( - input: string | URL | Request, - pinnedOrigin: string, -): URL { - if (typeof input === "string") { - return new URL(input, pinnedOrigin); - } - if (input instanceof URL) { - return input; - } - return new URL(input.url); -} diff --git a/packages/mcp-tools/package.json b/packages/mcp-tools/package.json index 2b7aa04fe..3d147e906 100644 --- a/packages/mcp-tools/package.json +++ b/packages/mcp-tools/package.json @@ -2,7 +2,7 @@ "name": "@corbits/mcp-tools", "private": true, "description": "Generic MCP server integration: connect any Streamable HTTP MCP server through Plugins and its tools become reachable by any agent via mcp_list_servers/mcp_list_tools/mcp_call", - "version": "0.0.9", + "version": "0.0.10", "license": "LGPL-2.1-or-later", "type": "module", "exports": { diff --git a/packages/mcp-tools/src/index.ts b/packages/mcp-tools/src/index.ts index ec9379c81..eb891b542 100644 --- a/packages/mcp-tools/src/index.ts +++ b/packages/mcp-tools/src/index.ts @@ -4,6 +4,7 @@ export { withMcpConnection, MCP_CLIENT_NAME, MCP_CLIENT_VERSION, + MCP_REQUEST_TIMEOUT_MS, } from "./mcp-client"; export type { McpCallResult, diff --git a/packages/mcp-tools/src/mcp-client.test.ts b/packages/mcp-tools/src/mcp-client.test.ts new file mode 100644 index 000000000..0960a97e4 --- /dev/null +++ b/packages/mcp-tools/src/mcp-client.test.ts @@ -0,0 +1,44 @@ +import { expect, test } from "bun:test"; +import type { Client } from "@modelcontextprotocol/sdk/client/index.js"; + +import { + callMcpTool, + listMcpTools, + MCP_REQUEST_TIMEOUT_MS, +} from "./mcp-client"; + +test("MCP_REQUEST_TIMEOUT_MS is above the SDK 60s default and under a 5 minute chat turn", () => { + expect(MCP_REQUEST_TIMEOUT_MS).toBe(2 * 60 * 1000); +}); + +test("callMcpTool passes the request timeout as callTool options", async () => { + const options: unknown[] = []; + const client = { + callTool: async ( + _params: unknown, + _schema: unknown, + requestOptions: unknown, + ) => { + options.push(requestOptions); + return { content: [], isError: false }; + }, + } as unknown as Client; + + await callMcpTool(client, { name: "generate-design", arguments: {} }); + + expect(options).toEqual([{ timeout: MCP_REQUEST_TIMEOUT_MS }]); +}); + +test("listMcpTools passes the request timeout as listTools options", async () => { + const options: unknown[] = []; + const client = { + listTools: async (_params: unknown, requestOptions: unknown) => { + options.push(requestOptions); + return { tools: [] }; + }, + } as unknown as Client; + + await listMcpTools(client); + + expect(options).toEqual([{ timeout: MCP_REQUEST_TIMEOUT_MS }]); +}); diff --git a/packages/mcp-tools/src/mcp-client.ts b/packages/mcp-tools/src/mcp-client.ts index 31dbd88b0..908b1b914 100644 --- a/packages/mcp-tools/src/mcp-client.ts +++ b/packages/mcp-tools/src/mcp-client.ts @@ -19,6 +19,14 @@ import type { export const MCP_CLIENT_NAME = "corbits-workbench"; export const MCP_CLIENT_VERSION = "0.0.1"; +/** Per-request MCP timeout. The SDK default is 60s — the documented + * duration of long tools such as Canva `generate-design`. Chat turns + * are 5 minutes; two minutes sits in between so a slow tool can finish + * without a hung call outliving the turn. */ +export const MCP_REQUEST_TIMEOUT_MS = 2 * 60 * 1000; + +const MCP_REQUEST_OPTIONS = { timeout: MCP_REQUEST_TIMEOUT_MS } as const; + export interface McpToolAnnotations { readonly title?: string | undefined; readonly readOnlyHint?: boolean | undefined; @@ -74,7 +82,7 @@ export async function withMcpConnection( export async function listMcpTools( client: Client, ): Promise { - const result = await client.listTools(); + const result = await client.listTools(undefined, MCP_REQUEST_OPTIONS); return result.tools.map((tool) => { const base: McpToolInfo = { name: tool.name, @@ -96,10 +104,14 @@ export async function callMcpTool( client: Client, args: { name: string; arguments: Record }, ): Promise { - const result = await client.callTool({ - name: args.name, - arguments: args.arguments, - }); + const result = await client.callTool( + { + name: args.name, + arguments: args.arguments, + }, + undefined, + MCP_REQUEST_OPTIONS, + ); return { isError: result.isError === true, content: result.content, diff --git a/packages/plugins-ui/src/mcp-preset-cards.tsx b/packages/plugins-ui/src/mcp-preset-cards.tsx index 7433267e3..347d8a9c1 100644 --- a/packages/plugins-ui/src/mcp-preset-cards.tsx +++ b/packages/plugins-ui/src/mcp-preset-cards.tsx @@ -23,6 +23,8 @@ function messageOf(cause: unknown): string { const MCP_OAUTH_ERROR_COPY: Readonly> = { discovery_failed: "Couldn't reach that app's sign-in. Try connecting again.", + client_rejected: + "That app didn't accept Workbench as a client (redirect URL or registration). Try connecting again.", no_authorization_needed: "That app didn't start a sign-in. Try connecting again.", state_expired: @@ -47,6 +49,20 @@ function mcpOauthReturnError(slug: string): string | null { ); } +function mcpOauthConnectedReturn(): + { readonly slug: string; readonly toolCount: number } | undefined { + const params = new URLSearchParams(window.location.search); + if (params.get("outcome") !== "connected") return undefined; + const slug = params.get("mcpOauth"); + const raw = params.get("toolCount"); + if (slug === null || slug === "" || raw === null || raw === "") { + return undefined; + } + const toolCount = Number(raw); + if (!Number.isInteger(toolCount) || toolCount < 0) return undefined; + return { slug, toolCount }; +} + function McpPresetCard({ tenantId, preset, @@ -248,7 +264,12 @@ export function McpPresetCardsSection({ }) { const [presets, setPresets] = useState([]); const [toolCounts, setToolCounts] = useState>( - new Map(), + () => { + const returned = mcpOauthConnectedReturn(); + return returned === undefined + ? new Map() + : new Map([[returned.slug, returned.toolCount]]); + }, ); const [loadError, setLoadError] = useState(null); // Whether the presets fetch has resolved at least once — distinct from diff --git a/packages/plugins-ui/test/mcp-preset-cards.test.tsx b/packages/plugins-ui/test/mcp-preset-cards.test.tsx index 52f78222f..e0e6cc8cd 100644 --- a/packages/plugins-ui/test/mcp-preset-cards.test.tsx +++ b/packages/plugins-ui/test/mcp-preset-cards.test.tsx @@ -203,6 +203,132 @@ describe("McpPresetCardsSection", () => { ); }); + test("a client_rejected OAuth return names the registration failure, not unreachable sign-in", async () => { + window.history.replaceState( + null, + "", + "/?mcpOauth=canva&outcome=error&code=client_rejected", + ); + globalThis.fetch = (async () => + new Response( + JSON.stringify({ + data: [ + ...PRESETS, + { + slug: "canva", + displayName: "Canva", + description: "Design with Canva — via MCP.", + url: "https://mcp.canva.com/mcp", + connectionMode: "oauth", + docsUrl: "https://www.canva.com", + connected: false, + }, + ], + }), + )) as unknown as typeof fetch; + + const container = mountSection(); + await settle(); + + const canvaCard = container.querySelector( + '[data-plugin-slug="canva"]', + ) as HTMLElement; + expect(canvaCard.textContent).toContain( + "That app didn't accept Workbench as a client (redirect URL or registration). Try connecting again.", + ); + expect(canvaCard.textContent).not.toContain( + "Couldn't reach that app's sign-in. Try connecting again.", + ); + expect( + canvaCard.querySelector('[aria-label="Connect Canva"]'), + ).not.toBeNull(); + + const granolaCard = container.querySelector( + '[data-plugin-slug="granola"]', + ) as HTMLElement; + expect(granolaCard.textContent).not.toContain( + "That app didn't accept Workbench as a client (redirect URL or registration). Try connecting again.", + ); + }); + + test("an OAuth connected return shows the probe tool count on that preset row", async () => { + window.history.replaceState( + null, + "", + "/?mcpOauth=canva&outcome=connected&toolCount=40", + ); + globalThis.fetch = (async () => + new Response( + JSON.stringify({ + data: [ + ...PRESETS, + { + slug: "canva", + displayName: "Canva", + description: "Design with Canva — via MCP.", + url: "https://mcp.canva.com/mcp", + connectionMode: "oauth", + docsUrl: "https://www.canva.com", + connected: true, + }, + ], + }), + )) as unknown as typeof fetch; + + const container = mountSection(); + await settle(); + + const canvaCard = container.querySelector( + '[data-plugin-slug="canva"]', + ) as HTMLElement; + expect(canvaCard.textContent).toContain("40 tools"); + expect(canvaCard.textContent).not.toContain("Not connected"); + expect( + canvaCard.querySelector('[aria-label="Disconnect Canva"]'), + ).not.toBeNull(); + + const granolaCard = container.querySelector( + '[data-plugin-slug="granola"]', + ) as HTMLElement; + expect(granolaCard.textContent).not.toContain("40 tools"); + }); + + test("an OAuth connected return with a non-integer toolCount stays a bare Connected", async () => { + window.history.replaceState( + null, + "", + "/?mcpOauth=canva&outcome=connected&toolCount=abc", + ); + globalThis.fetch = (async () => + new Response( + JSON.stringify({ + data: [ + ...PRESETS, + { + slug: "canva", + displayName: "Canva", + description: "Design with Canva — via MCP.", + url: "https://mcp.canva.com/mcp", + connectionMode: "oauth", + docsUrl: "https://www.canva.com", + connected: true, + }, + ], + }), + )) as unknown as typeof fetch; + + const container = mountSection(); + await settle(); + + const canvaCard = container.querySelector( + '[data-plugin-slug="canva"]', + ) as HTMLElement; + expect(canvaCard.textContent).toContain("Connected"); + expect(canvaCard.textContent).not.toContain("abc"); + expect(canvaCard.textContent).not.toContain("NaN"); + expect(canvaCard.textContent).not.toContain("tools"); + }); + test("Disconnect's accessible name includes the preset display name (CL-6794)", async () => { globalThis.fetch = (async () => new Response( diff --git a/workflows/assistant/src/index.ts b/workflows/assistant/src/index.ts index 13b474fc6..3d6a6dd52 100644 --- a/workflows/assistant/src/index.ts +++ b/workflows/assistant/src/index.ts @@ -51,7 +51,7 @@ export const ASSISTANT_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ { name: "@corbits/connections-tools", version: "0.0.6" }, { name: "@corbits/catalog-tools", version: "0.0.1" }, { name: "@corbits/skills-tools", version: "0.0.6" }, - { name: "@corbits/mcp-tools", version: "0.0.9" }, + { name: "@corbits/mcp-tools", version: "0.0.10" }, { name: "@corbits/interaction-tools", version: "0.0.2" }, { name: "@corbits/manus-tools", version: "0.0.11" }, ]; diff --git a/workflows/attio-task-agent/src/index.ts b/workflows/attio-task-agent/src/index.ts index f46097e26..65232e88c 100644 --- a/workflows/attio-task-agent/src/index.ts +++ b/workflows/attio-task-agent/src/index.ts @@ -90,7 +90,7 @@ export const ATTIO_TASK_AGENT_SYSTEM_PROMPT = buildAttioTaskAgentSystemPrompt({ * deploy of this package itself. */ export const ATTIO_TASK_AGENT_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ - { name: "@corbits/mcp-tools", version: "0.0.9" }, + { name: "@corbits/mcp-tools", version: "0.0.10" }, ]; /** diff --git a/workflows/attio-task-agent/test/definition.test.ts b/workflows/attio-task-agent/test/definition.test.ts index 8c161a88c..b23076310 100644 --- a/workflows/attio-task-agent/test/definition.test.ts +++ b/workflows/attio-task-agent/test/definition.test.ts @@ -70,7 +70,7 @@ test("one MCP pin covers the CRM, past calls, and the web — the OG needed thre const agent = workStep(buildAttioTaskAgentWorkflow(INPUT)).agent; expect(agent.toolPackagePins).toEqual(ATTIO_TASK_AGENT_TOOL_PACKAGE_PINS); expect(ATTIO_TASK_AGENT_TOOL_PACKAGE_PINS).toEqual([ - { name: "@corbits/mcp-tools", version: "0.0.9" }, + { name: "@corbits/mcp-tools", version: "0.0.10" }, ]); }); diff --git a/workflows/exa-topic-watch/src/index.ts b/workflows/exa-topic-watch/src/index.ts index 98a802164..49cf7e033 100644 --- a/workflows/exa-topic-watch/src/index.ts +++ b/workflows/exa-topic-watch/src/index.ts @@ -110,7 +110,7 @@ export const EXA_TOPIC_WATCH_SYSTEM_PROMPT = [ * travels with the deploy of this package itself. */ export const EXA_TOPIC_WATCH_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ - { name: "@corbits/mcp-tools", version: "0.0.9" }, + { name: "@corbits/mcp-tools", version: "0.0.10" }, ]; /** diff --git a/workflows/exa-topic-watch/test/definition.test.ts b/workflows/exa-topic-watch/test/definition.test.ts index 23109a2e3..cf2436870 100644 --- a/workflows/exa-topic-watch/test/definition.test.ts +++ b/workflows/exa-topic-watch/test/definition.test.ts @@ -71,7 +71,7 @@ test("the step pins the MCP tools bundle, the one package a deploy here can reso const agent = digestStep(buildExaTopicWatchWorkflow(INPUT)).agent; expect(agent.toolPackagePins).toEqual(EXA_TOPIC_WATCH_TOOL_PACKAGE_PINS); expect(EXA_TOPIC_WATCH_TOOL_PACKAGE_PINS).toEqual([ - { name: "@corbits/mcp-tools", version: "0.0.9" }, + { name: "@corbits/mcp-tools", version: "0.0.10" }, ]); });