Skip to content

Add MCP server card endpoint#60

Merged
AnnatarHe merged 1 commit intomasterfrom
codex/serve-mcp-server-card
Apr 19, 2026
Merged

Add MCP server card endpoint#60
AnnatarHe merged 1 commit intomasterfrom
codex/serve-mcp-server-card

Conversation

@AnnatarHe
Copy link
Copy Markdown
Member

Summary

  • Add /.well-known/mcp/server-card.json as a prerendered Astro endpoint
  • Publish SEP-1649 server card metadata with serverInfo, transport endpoint, and capabilities
  • Set JSON response headers for discovery and caching

Testing

  • pnpm astro check
  • pnpm build

@AnnatarHe AnnatarHe temporarily deployed to codex/serve-mcp-server-card - asynctalk-website PR #60 April 19, 2026 03:51 — with Render Destroyed
@AnnatarHe AnnatarHe merged commit 223984b into master Apr 19, 2026
1 check passed
@AnnatarHe AnnatarHe deleted the codex/serve-mcp-server-card branch April 19, 2026 03:51
@claude
Copy link
Copy Markdown

claude Bot commented Apr 19, 2026

Code Review

Thanks for adding this endpoint. The implementation is clean and follows Astro's API route conventions well. Below are my observations, grouped by severity.


Issues to Address

1. The /mcp transport endpoint does not exist

src/pages/.well-known/mcp/server-card.json.ts, line 18:

endpoint: "/mcp",

The server card advertises a streamable-http transport at /mcp, but no such route exists anywhere in src/pages/. This makes the capability advertisement misleading — any MCP client that discovers this card and tries to connect to /mcp will receive a 404. Either:

  • Implement the /mcp server route (as a separate PR), or
  • Remove the transport block until the actual endpoint is built, or
  • Add a comment documenting that this is intentionally forward-looking

2. capabilities object declares tools, prompts, and resources but none are implemented

Lines 21–24 list all three capability categories, each as an empty object. According to the MCP Server Card spec, advertising a capability implies the server supports it. An empty {} for tools still signals "I have tools" to a compliant client. If no tools/prompts/resources are currently provided, either omit the keys that aren't implemented yet or use the presence flags the spec recommends.


Minor Issues

3. protocolVersion: "2025-06-18" is a future date

Today is 2026-04-19, but the MCP protocol version string 2025-06-18 refers to an MCP spec revision date. Make sure this matches a real published revision of the MCP spec that this endpoint is actually compatible with (the 2024-11-05 or 2025-03-26 revisions are published; 2025-06-18 should be verified).

4. version field in serverInfo is hardcoded to "2.0.0"

This duplicates the version in package.json. If the package version changes, this field will drift. Consider importing it:

import { version } from "../../../../package.json";
// ...
serverInfo: { version },

Astro supports JSON imports with TypeScript strict mode out of the box, and this ensures the two stay in sync.

5. CORS header is very permissive

"Access-Control-Allow-Origin": "*",

A wildcard CORS header on a discovery endpoint is fine and probably intentional here (it is a public, read-only JSON file), but it is worth documenting the reason in a comment so the next reviewer does not wonder whether it was accidental.


Positive Notes

  • Using prerender = true is the right call for a static JSON file; no server runtime required.
  • Response headers are well-chosen: Content-Type with charset, Cache-Control with a reasonable 1-hour TTL, and X-Content-Type-Options: nosniff.
  • The description field is correctly in Chinese, consistent with the project's content guidelines.
  • JSON.stringify(serverCard, null, 2) gives human-readable output, which helps with debugging and curl-based discovery.
  • The documentationUrl points to the live site rather than a dead internal URL.

Summary

The main blocker is that the declared transport endpoint (/mcp) does not exist, so the server card cannot actually be used for MCP discovery today. The capabilities object also over-declares what is implemented. Everything else is cosmetic or low-priority. I'd suggest either scoping this PR to the server card only (with transport removed or clearly marked as placeholder) and tracking the actual MCP server implementation separately, or holding the PR until /mcp is also implemented.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 347db0d47e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

documentationUrl: "https://asynctalk.com",
transport: {
type: "streamable-http",
endpoint: "/mcp",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Point transport endpoint to a live MCP route

The server card advertises transport.endpoint as /mcp, but this change does not add any /mcp handler or rewrite in the repo (I checked with a repo-wide rg -uu -n "\bmcp\b|server-card", which only finds this new file). In deployments built from this code, clients that discover the card will attempt to connect to /mcp and receive a 404, so discovery now directs users to a non-functional MCP endpoint.

Useful? React with 👍 / 👎.

export const GET: APIRoute = () => {
return new Response(JSON.stringify(serverCard, null, 2), {
headers: {
"Access-Control-Allow-Origin": "*",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add full CORS preflight headers for discovery

The endpoint only returns Access-Control-Allow-Origin, but omits explicit Access-Control-Allow-Methods/Access-Control-Allow-Headers. Cross-origin browser clients that include non-simple discovery headers will trigger a preflight request, and without those allow-lists the browser can block access to the server card, undermining HTTP-based discovery in exactly the browser environments this endpoint is intended to support.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant