fix: complete MCP discovery flow and setup guides#9
Conversation
Co-authored-by: Codex <codex@openai.com>
# Conflicts: # app/.well-known/oauth-authorization-server/api/auth/route.ts # app/api/mcp/route.ts # tests/mcp-oauth.test.ts
|
Working on this PR… |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
enkii · code review
Summary
This PR completes the MCP discovery flow and replaces the old one-off MCP token snippet with a tabbed connection guide for Claude Web, Claude Code, Codex, and generic HTTP MCP clients. It also adds discovery and token-guide tests. Overall the implementation is clean and well-tested. There is one P2 UX issue: the "Copied" indicator bleeds across the Claude Code and Codex tabs due to duplicate label keys used for copy-state tracking. Fix that, and this is safe to merge.
Mergeability Score: 4/5
Likely mergeable after reviewing the flagged low-risk comments.
| </div> | ||
| <CodeBlock | ||
| label="Claude Web values" | ||
| value={claudeWebBlock(appUrl)} |
There was a problem hiding this comment.
The CodeBlock component uses its label prop as the copy-state key, and both the "Claude Code" and "Codex" tabs use identical labels: "macOS / Linux" and "Windows PowerShell". Because copiedKey is a single shared state, copying a snippet in one tab (e.g., the Claude Code macOS command) and then switching to the Codex tab shows a stale "Copied" indicator on the Codex macOS command — misleading the user into thinking they already copied a value they haven't.
Fix: give CodeBlock an explicit copyKey prop, or prefix the labels with the tab name (e.g. "claude-code-macos", "codex-macos").
There was a problem hiding this comment.
enkii · security review
Summary
Security surface reviewed: MCP client connection guides, token settings UI, and MCP OAuth discovery routes. The PR adds UI-only connection snippets for Claude Web/Code/Codex/generic MCP clients and passes env.APP_URL (server-side, Zod-validated) to the client component — replacing the old window.location.origin usage, which is a net security improvement. Token values are base64url-encoded and rendered through React's JSX escaping, eliminating injection risk. No new trust boundaries, no authentication/authorization changes, no exploitable vulnerabilities found. Safe to merge.
Mergeability Score: 5/5
Safe to merge from this review's perspective.
Co-authored-by: Codex <codex@openai.com>
|
Working on this PR… |
There was a problem hiding this comment.
enkii · code review
Summary
This PR completes the MCP connector discovery flow and replaces the one-off token snippet with a rich, tabbed connection guide covering Claude Web, Claude Code, Codex, and generic MCP clients. The OAuth discovery routes are correctly wired (protected resource metadata at /.well-known/oauth-protected-resource, path-aware auth server metadata at /.well-known/oauth-authorization-server/api/auth), and GET/DELETE on /api/mcp correctly challenge unauthenticated requests. The connection-guides module produces syntactically valid commands for all four client targets. The existing P2 finding about copied-state bleeding across tabs has been addressed via the copyKey prop. No new issues found — safe to merge.
Mergeability Score: 5/5
Safe to merge from this review's perspective.
There was a problem hiding this comment.
enkii · security review
Summary
Security review: no exploitable vulnerabilities found. The PR adds MCP client connection guides (pure string-formatting functions) and UI tabs to the service tokens settings page. The only data crossing from server to client is env.APP_URL (the app's public URL, not a secret) and server-generated service tokens (base64url-encoded random bytes, safe for display). No user-controlled input reaches a dangerous sink. The old window.location.origin approach is replaced with the canonical server-side APP_URL, which fixes a SSR display bug with no security impact. Safe to merge.
Mergeability Score: 5/5
Safe to merge from this review's perspective.
1 participant
Summary
Complete the Claude connector discovery flow and replace the one-off MCP token snippet with connector-specific setup guides.
What's Included
Tests
Test Plan
Local Review Findings