FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add MCP Server Card (SEP-2127) types + handler by SamMorrowDrums · Pull Request #2768 · github/github-mcp-server · GitHub

Add MCP Server Card (SEP-2127) types + handler - #2768

Merged
SamMorrowDrums merged 1 commit into
mainfrom
sammorrowdrums-server-card-handler
Sep 2, 2026
Merged

Add MCP Server Card (SEP-2127) types + handler#2768
SamMorrowDrums merged 1 commit into
mainfrom
sammorrowdrums-server-card-handler

Conversation

SamMorrowDrums commented Jun 25, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator

What & why

Makes the GitHub MCP Server discoverable via an MCP Server Card. Adds a new OSS-owned package pkg/http/servercard with:

  • Go types matching the current v1 Server Card schema in modelcontextprotocol/experimental-ext-server-card (schema.json + docs/discovery.md).
  • A constructor (NewServerCard) for the GitHub MCP Server's card, reusing the identity fields from the registry server.json so both documents describe the same server.
  • A public, no-auth, read-only http.Handler that serves the card at the reserved /server-card location (public URL: transport URL + /server-card, e.g. https://api.githubcopilot.com/mcp/server-card).

Ownership: this package owns the Server Card implementation and all stable GitHub MCP identity/metadata, but the card is mounted and hosted only by the remote deployment (github/github-mcp-server-remote), which supplies the environment-specific remote URL and reuses this serving logic verbatim. The standalone OSS server does not advertise the card, so self-hosted binaries never falsely claim the dotcom remote.

Note: SEP-2127 is still open/in review — this tracks the current canonical v1 schema + discovery rules in the experimental extension repo, not an accepted core extension.

Card shape (remote-only, minimal)

The card is deliberately remote-only and minimal — it advertises identity + a single streamable-http remote and omits tools/resources/prompts and installable packages (those stay in the registry server.json). This is the exact JSON emitted for github.com:

{
  "$schema": "https://static.modelcontextprotocol.io/schemas/v1/server-card.schema.json",
  "name": "io.github.github/github-mcp-server",
  "version": "<build version>",
  "description": "Connect AI assistants to GitHub - manage repos, issues, PRs, and workflows through natural language.",
  "title": "GitHub",
  "websiteUrl": "https://github.com/github/github-mcp-server",
  "repository": { "url": "https://github.com/github/github-mcp-server", "source": "github", "id": "942771284" },
  "remotes": [
    { "type": "streamable-http", "url": "https://api.githubcopilot.com/mcp/" }
  ]
}

The name is locked to io.github.github/github-mcp-server, matching the registry server.json. It is the stable Server Card / registry server identity; the AI Catalog identifier is assigned independently and is not derived from the card name.

Serving behavior (per discovery.md)

  • Media type application/mcp-server-card+json with RFC 9110 Accept negotiation, including quality values — an explicit q=0 on the most specific matching media range rejects with 406 Not Acceptable. Accept and If-None-Match are parsed as RFC 9110 list values across repeated header field-lines; Accept honors quoted commas and media-range parameters, while entity-tag lists split on quoted commas without backslash escaping (an opaque-tag treats a backslash literally).
  • CORS — the mandated four headers, applied uniformly to GET/HEAD/OPTIONS/304:
    • Access-Control-Allow-Origin: *
    • Access-Control-Allow-Methods: GET
    • Access-Control-Allow-Headers: Content-Type, If-None-Match
    • Access-Control-Expose-Headers: ETag
  • Cache-Control: public, max-age=3600.
  • Vary: Accept, X-Forwarded-Host — appended (not set) so it composes with deployment-middleware values. Because the response is publicly cacheable and the multi-tenant remote derives its URL from the trusted X-Forwarded-Host, both are declared cache-key inputs so a shared cache cannot serve one tenant's card to another.
  • Strong ETag (SHA-256 of the exact served body, quoted lowercase hex) with If-None-Match → 304 Not Modified (empty body). Weak comparison per RFC 9110. See upstream ETag proposal experimental-ext-server-card#33.

RegisterRoutes mounts the card at the single reserved /server-card path. A composition test guards that route against being shadowed by an MCP catch-all mount (r.Mount("/", h)), so the remote can register both on one router.

Reuse by the hosted/remote deployment

The hosted deployment is multi-tenant, so the remote URL (and therefore the card body + ETag) varies per request. To keep header/ETag logic byte-for-byte identical across OSS and the remote, the handler exposes:

  • Config.RemoteURLFunc func(*http.Request) string — derive the per-request remote URL.
  • ServeCard(w, r, card *ServerCard) — write the full response (Content-Type, CORS, Cache-Control, ETag, If-None-Match/304) for an already-built card.

Deliberately omitted (kept minimal)

  • Installable packages — remote-only card; packages live in the registry server.json.
  • supportedProtocolVersions — the go-sdk does not export the versions it negotiates, so it cannot be advertised accurately from the runtime; a hand-maintained list would drift.
  • Icons — title / description / repository / websiteUrl are sufficient for discovery; icon data-URIs added code without contract value.
  • Per-remote auth metadata — the hosted server advertises auth via OAuth protected-resource-metadata discovery; duplicating it on the card would risk drift and cannot capture every accepted mode. The v1 schema only requires type + url on a remote.

Validation

  • go build ./..., go test -race ./... (full suite), ./script/lint (0 issues) — all green.
  • Diff kept to 800 additions across 4 files (package + tests only); no vendored schema copy is checked in, and the standalone OSS server is untouched.
  • Release-quality history: rebased onto current main (== v1.11.0) and squashed into a single commit.

Refs: github/copilot-mcp-core#1855 · epic github/copilot-mcp-core#1853

SamMorrowDrums force-pushed the sammorrowdrums-server-card-handler branch 2 times, most recently from eaa898f to 06a878c Compare August 26, 2026 10:34
SamMorrowDrums requested a balanced review from Copilot August 26, 2026 10:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

Adds MCP Server Card discovery support for the GitHub MCP Server.

Changes:

  • Defines Server Card types and metadata.
  • Adds HTTP serving, CORS, caching, and ETag handling.
  • Integrates and tests the public /server-card route.
Show a summary per file
File Description
pkg/http/servercard/card.go Defines card types and construction.
pkg/http/servercard/card_test.go Tests card metadata and serialization.
pkg/http/servercard/handler.go Implements HTTP serving and negotiation.
pkg/http/servercard/handler_test.go Tests handler behavior and caching.
pkg/http/server.go Registers the card endpoint.
pkg/http/server_test.go Tests router and CORS isolation.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 6/6 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread pkg/http/servercard/handler.go Outdated
Comment thread pkg/http/servercard/handler.go Outdated
SamMorrowDrums requested a balanced review from Copilot August 26, 2026 10:39
SamMorrowDrums marked this pull request as ready for review August 26, 2026 10:40
SamMorrowDrums requested a review from a team as a code owner August 26, 2026 10:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Review details

Suppressed comments (2)

pkg/http/servercard/handler.go:181

  • The Accept parser discards every parameter, so Accept: application/mcp-server-card+json;q=0 (and application/*;q=0) is treated as acceptable even though a zero quality value explicitly rejects that representation. Parse the media ranges and their quality weights, including specificity precedence, and return 406 when the effective quality for MediaType is zero.
		if i := strings.IndexByte(mediaRange, ';'); i >= 0 {
			mediaRange = strings.TrimSpace(mediaRange[:i])
		}
		switch strings.ToLower(mediaRange) {
		case MediaType, "*/*", "application/*":
			return true

pkg/http/servercard/handler.go:63

  • Because the handler selects between 200 and 406 based on Accept and the 200 response is publicly cacheable, it needs Vary: Accept. Without it, a shared cache may replay a cached card to a request that explicitly rejects this media type (or otherwise reuse the negotiated response incorrectly).
	if !acceptsCard(r.Header.Get(headers.AcceptHeader)) {
  • Files reviewed: 6/6 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread pkg/http/servercard/card.go Outdated
Comment thread pkg/http/server.go Outdated
SamMorrowDrums force-pushed the sammorrowdrums-server-card-handler branch 3 times, most recently from e33dc64 to 5a4ca43 Compare August 26, 2026 11:01
SamMorrowDrums requested a balanced review from Copilot August 26, 2026 11:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Review details

Suppressed comments (2)

pkg/http/servercard/handler.go:112

  • This has the same repeated-field issue as Accept: If-None-Match is list-valued, but Header.Get examines only its first field line. If a matching validator is supplied on a later line, this returns 200 instead of the required 304. Combine Header.Values before applying the weak comparison.
	if ifNoneMatchSatisfied(r.Header.Get("If-None-Match"), etag) {

pkg/http/servercard/handler.go:204

  • Discarding every non-q parameter is not RFC 9110 media-range matching. For example, Accept: application/mcp-server-card+json;profile=x constrains the acceptable representation to that parameter, but this parameterless response returns 200; a parameterized exact q=0 can also incorrectly override an acceptable wildcard. Parse media parameters and only treat a range as matching when its representation parameters match the emitted Content-Type, with regression tests for both cases.
// parseMediaRange splits one Accept media range into its lowercased media type
// and quality value. The quality defaults to 1.0 when no valid q parameter is
// present; only the q parameter is interpreted and other parameters are ignored.
func parseMediaRange(part string) (mediaType string, quality float64) {
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread pkg/http/servercard/handler.go Outdated
SamMorrowDrums force-pushed the sammorrowdrums-server-card-handler branch 2 times, most recently from 0abcada to ad12429 Compare August 28, 2026 10:46
SamMorrowDrums requested a balanced review from Copilot September 1, 2026 12:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🟡 Changes recommended

Accept parsing mishandles quoted commas and can ignore an explicit q=0.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)
Severity Finding
pkg/​http/​servercard/​handler.go — strings.SplitSeq(accept, &quot;,&quot;) does not parse a valid HTTP list when a media parameter contains a…

Comment thread pkg/http/servercard/handler.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🟡 Changes recommended

Accept parameter matching and destructive Vary handling can produce incorrect cache and negotiation behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 3

New issues introduced by this change (2)
Severity Finding
pkg/​http/​servercard/​handler.go — Using Set here discards any existing Vary values added by the remote deployment's middleware.…
pkg/​http/​servercard/​handler.go — Accept media parameters before q participate in matching under RFC 9110; they cannot be…
Pre-existing issues (1)
Severity Finding
pkg/​http/​servercard/​handler.go — strings.SplitSeq(accept, &quot;,&quot;) does not parse a valid HTTP list when a media parameter contains a… View comment

Comment thread pkg/http/servercard/handler.go Outdated
Comment thread pkg/http/servercard/handler.go Outdated
SamMorrowDrums force-pushed the sammorrowdrums-server-card-handler branch from 10707f9 to 172dc95 Compare September 2, 2026 10:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🟡 Changes recommended

Conditional ETag matching mishandles valid backslashes, and the stated diff size is inaccurate.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 1

New issues introduced by this change (2)
Severity Finding
pkg/​http/​servercard/​handler.go — splitList applies quoted-string backslash escaping, but RFC 9110 entity-tags use opaque-tag,…
pkg/​http/​servercard/​card_test.go — The validation summary says this PR contains 780 additions across these four files, but the…
Issues resolved since last review (3)
Severity Finding
pkg/​http/​servercard/​handler.go — Accept media parameters before q participate in matching under RFC 9110; they cannot be… View resolved comment
pkg/​http/​servercard/​handler.go — Using Set here discards any existing Vary values added by the remote deployment's middleware.… View resolved comment
pkg/​http/​servercard/​handler.go — strings.SplitSeq(accept, &quot;,&quot;) does not parse a valid HTTP list when a media parameter contains a… View resolved comment

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🔵 Needs a closer look

Accept extensions named q can override an earlier explicit zero-quality refusal.

Review tier: Balanced
Findings: 1 · 1

Pre-existing issues (2)
Severity Finding
pkg/​http/​servercard/​handler.go — splitList applies quoted-string backslash escaping, but RFC 9110 entity-tags use opaque-tag,… View comment
pkg/​http/​servercard/​card_test.go — The validation summary says this PR contains 780 additions across these four files, but the… View comment
Suppressed comments (1)

pkg/http/servercard/handler.go:208

  • After the first q delimiter, later segments are accept extensions, but this branch still treats an extension named q as another weight. Consequently, Accept: application/mcp-server-card+json;q=0;q=1 overwrites the refusal and returns 200. Only recognize q before seenQ becomes true, and add this case to the negotiation table.
		if strings.EqualFold(strings.TrimSpace(name), "q") {

Define the GitHub MCP Server's Server Card (SEP-2127, in review) and a
public no-auth HTTP handler that serves it at the canonical /server-card
backend path. OSS owns all stable identity/metadata and serving behavior;
the remote server only supplies an environment-specific remote URL.

Negotiate Accept and If-None-Match as RFC 9110 list values across repeated
header field-lines. Accept honors quoted commas, media-range parameters, and
q-values (q=0 rejects); entity-tag lists split on quoted commas without
backslash escaping, since an opaque-tag treats a backslash literally. Vary
lists Accept and X-Forwarded-Host and is appended rather than set: it
composes with values added by deployment middleware and, since the response
is publicly cacheable and multi-tenant deployments derive the remote URL from
the trusted X-Forwarded-Host, it keeps a shared cache from serving one
tenant's card to another.

Refs github/copilot-mcp-core#1855, epic github/copilot-mcp-core#1853
Spec: modelcontextprotocol/experimental-ext-server-card

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4a9f522f-6942-4b77-98a4-b2d42f19625d

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot review overview

🟢 Approval recommended

The implementation matches the stated remote-only design and is thoroughly covered by focused tests.

Review tier: Balanced
Findings: 1

Pre-existing issues (1)
Severity Finding
pkg/​http/​servercard/​card_test.go — The validation summary says this PR contains 780 additions across these four files, but the… View comment
Issues resolved since last review (1)
Severity Finding
pkg/​http/​servercard/​handler.go — splitList applies quoted-string backslash escaping, but RFC 9110 entity-tags use opaque-tag,… View resolved comment

SamMorrowDrums merged commit bd47e63 into main Sep 2, 2026
20 checks passed
SamMorrowDrums deleted the sammorrowdrums-server-card-handler branch September 2, 2026 13:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

2 participants


Back | FazBrowse Home | New Git URL