diff --git a/public/js/dodo-settings.js b/public/js/dodo-settings.js index 9f899c4..15b956f 100644 --- a/public/js/dodo-settings.js +++ b/public/js/dodo-settings.js @@ -255,6 +255,12 @@ function renderIntegrations(){ // Match catalog entries to OAuth-connected servers by hostname. const oauthByHostname=new Map(); oauthServers.forEach(s=>{const h=getHostname(s.url);if(h)oauthByHostname.set(h,s);}); + // Track which oauthServers we've rendered as part of a catalog entry so + // the leftover loop below can pick up the orphans. Without this, a + // hub-DO record for an MCP URL that's no longer in the catalog (e.g. a + // failed cf-portal attempt left behind after the catalog entry was + // removed) would never render → user has no UI handle to clear it. + const renderedOAuthServerIds=new Set(); const connected=[],suggestions=[]; const hasGithubToken=secretKeys.includes("github_token"); mcpCatalog.forEach(cat=>{ @@ -271,6 +277,7 @@ function renderIntegrations(){ for(const h of catHosts){if(oauthByHostname.has(h)){oauthServer=oauthByHostname.get(h);break;}} if(oauthServer){ connected.push(renderOAuthCard(cat,oauthServer)); + renderedOAuthServerIds.add(oauthServer.id); }else{ suggestions.push(renderOAuthCard(cat,null)); } @@ -298,6 +305,16 @@ function renderIntegrations(){ connected.push(renderIntegCard(cfg.name,cfg.url||"Custom integration",null,cfg)); } }); + // Render any orphaned OAuth servers — entries in the per-user hub DO + // whose URL doesn't match any current catalog entry. Usually leftovers + // from a failed OAuth attempt where the catalog entry has since been + // removed, or one-off MCP servers added directly via /api/mcp/start-auth. + // Surfacing them gives the user a "Clear" button instead of letting the + // zombie linger. + oauthServers.forEach(s=>{ + if(renderedOAuthServerIds.has(s.id))return; + connected.push(renderOrphanedOAuthCard(s)); + }); const cards=[...connected,...suggestions]; $("integrations-list").innerHTML=cards.length?cards.join(""):'
No integrations
'; } @@ -346,10 +363,12 @@ async function refreshIntegrationToken(id,displayName){ }catch(e){toast("Refresh failed: "+(e.message||e),"error")} } -// OAuth-MCP catalog entries render with different actions: "Connect with OAuth" -// when not yet connected (opens the provider in a popup), and "Disconnect" -// when already connected. The actual OAuth state is managed by the Agents SDK -// in the user's hub DO, not in mcp_configs. +// OAuth-MCP catalog entries render with different actions based on the +// server's current state in the per-user hub DO. The Agents SDK transitions +// between authenticating → connecting → discovering → ready (success path) +// or → failed (terminal error). For terminal states the only sensible +// action is to clear the entry; offering "Refresh" on a failed server +// retries against the same broken config and confuses users. function renderOAuthCard(cat,server){ const desc=cat.description||""; let statusHtml,actionsHtml; @@ -362,7 +381,10 @@ function renderOAuthCard(cat,server){ :server.state; const color=server.state==="ready"?"var(--text-success)":server.state==="failed"?"var(--text-error,#b91c1c)":"var(--text-subtle)"; statusHtml=`${esc(stateLabel)}`; - actionsHtml=``; + const isReady=server.state==="ready"; + const refreshBtn=isReady?``:""; + const removeLabel=isReady?"Disconnect":"Clear"; + actionsHtml=`${refreshBtn}`; }else{ statusHtml=`Not connected`; actionsHtml=``; @@ -370,6 +392,22 @@ function renderOAuthCard(cat,server){ return `
${esc(cat.name)}
${esc(desc)}
${statusHtml}
${actionsHtml}
`; } +// Render an OAuth server that has no matching catalog entry — either a +// custom MCP added via `/api/mcp/start-auth`, or a leftover from a failed +// attempt whose catalog entry has since been removed. Always offer a +// "Clear" action so the user can wipe it; never offer "Refresh" since we +// don't know what the original setup looked like. +function renderOrphanedOAuthCard(server){ + const stateLabel=server.state==="ready"?`Connected — ${server.toolCount} tool${server.toolCount!==1?'s':''}` + :server.state==="authenticating"?"Authenticating… (no longer reachable)" + :server.state==="failed"?(server.error?`Failed: ${server.error}`:"Failed") + :server.state; + const color=server.state==="ready"?"var(--text-success)":server.state==="failed"?"var(--text-error,#b91c1c)":"var(--text-subtle)"; + const displayName=server.name||"OAuth MCP"; + const desc=`${server.url} — orphaned entry, not in catalog`; + return `
${esc(displayName)}
${esc(desc)}
${esc(stateLabel)}
`; +} + // Kick off the OAuth dance for a catalog entry. The server returns either // {authUrl} (popup needed) or {message:"Connected"} (already authenticated). // The popup polls /api/mcp/oauth-servers after closing to detect the new diff --git a/src/mcp-catalog.ts b/src/mcp-catalog.ts index 255d872..74c277b 100644 --- a/src/mcp-catalog.ts +++ b/src/mcp-catalog.ts @@ -25,17 +25,15 @@ const CORE_MCP_CATALOG: McpCatalogEntry[] = [ auth_type: "static_headers", knownHosts: [], }, - { - id: "github", - name: "GitHub", - description: - "Structured tools for issues, PRs, actions, and code scanning (beyond basic git operations)", - url: "https://api.githubcopilot.com/mcp/", - setupGuide: - "Use the remote server at https://api.githubcopilot.com/mcp/ with OAuth, or deploy locally with a PAT", - auth_type: "oauth", - knownHosts: ["api.githubcopilot.com"], - }, + // NOTE: GitHub Copilot's MCP server at https://api.githubcopilot.com/mcp/ + // was tested and is intentionally NOT in this catalog as an OAuth entry. + // Its OAuth provider does NOT support Dynamic Client Registration — + // attempting to connect via the SDK-managed OAuth path errors with + // "Incompatible auth server: does not support dynamic client registration". + // To use GitHub MCP, add a github_token secret (Personal Access Token or + // GitHub App installation token); the existing static-headers path picks + // it up automatically and the UI hides the GitHub catalog entry when one + // is present. ]; const DEFAULT_CLOUDFLARE_REMOTE_MCP_CATALOG: McpCatalogEntry[] = [ diff --git a/test/mcp-catalog-unit.test.ts b/test/mcp-catalog-unit.test.ts index 45768d0..54ae54c 100644 --- a/test/mcp-catalog-unit.test.ts +++ b/test/mcp-catalog-unit.test.ts @@ -23,11 +23,14 @@ describe("MCP_CATALOG", () => { expect(entry?.knownHosts).toContain("browser.mcp.cloudflare.com"); }); - it("includes github as an OAuth catalog entry", () => { + it("does NOT include github as an OAuth catalog entry — GitHub's MCP server doesn't support DCR", () => { + // Documented in mcp-catalog.ts. api.githubcopilot.com returns + // "Incompatible auth server: does not support dynamic client + // registration" when the SDK-managed OAuth path attempts DCR. To use + // GitHub MCP, add a github_token secret — the static-headers path + // already handles it and the UI hides the suggestion when present. const entry = MCP_CATALOG.find((e) => e.id === "github"); - expect(entry).toBeDefined(); - expect(entry?.auth_type).toBe("oauth"); - expect(entry?.knownHosts).toContain("api.githubcopilot.com"); + expect(entry).toBeUndefined(); }); it("every catalog entry has either auth_type or is the self-MCP", () => { diff --git a/test/mcp-config.test.ts b/test/mcp-config.test.ts index 6c47c98..9a55f5e 100644 --- a/test/mcp-config.test.ts +++ b/test/mcp-config.test.ts @@ -182,22 +182,29 @@ describe("Approved MCP catalog admin operations", () => { }); describe("MCP Catalog", () => { - it("returns catalog with at least 3 entries", async () => { + it("returns catalog with the expected default entries", async () => { const res = await fetchJson("/api/mcp-catalog"); expect(res.status).toBe(200); const catalog = (await res.json()) as Array<{ id: string; name: string; description: string; url: string; setupGuide: string }>; expect(Array.isArray(catalog)).toBe(true); - // Catalog seed is config-driven (PR #48). Three entries land by default — - // dodo-self, github, browser-rendering — and admins can add more via + // Catalog seed is config-driven. The current defaults are + // dodo-self + browser-rendering. cf-portal is omitted (loopback-only + // OAuth — see mcp-catalog.ts comment) and github is omitted (its MCP + // server doesn't support DCR). Admins can add more via // /api/admin/approved-mcps. - expect(catalog.length).toBeGreaterThanOrEqual(3); + expect(catalog.length).toBeGreaterThanOrEqual(2); // Verify known entries exist const ids = catalog.map((c) => c.id); expect(ids).toContain("dodo-self"); - expect(ids).toContain("github"); expect(ids).toContain("browser-rendering"); + // And confirm the catalog entries removed for compatibility reasons + // are indeed gone, so the next person to add them doesn't have to + // re-read all the audit history. + expect(ids).not.toContain("github"); + expect(ids).not.toContain("cf-portal"); + // Verify each entry has required fields for (const entry of catalog) { expect(entry.id).toBeTruthy();