Skip to content

Commit 35b34e8

Browse files
Serve the Server Card at a single canonical path
The card is discovered via a single catalog link, so it must be served at exactly one canonical location. Register only the reserved relative path /server-card and drop the duplicate /mcp/server-card registration: the hosted edge strips the /mcp base path before forwarding, so the backend receives /server-card while the public URL stays https://api.githubcopilot.com/mcp/server-card. Update the tests to assert the single canonical path, guard that /mcp/server-card is no longer registered (404), and verify the static card route is not shadowed by the streamable MCP catch-all mount. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9143074 commit 35b34e8

2 files changed

Lines changed: 97 additions & 32 deletions

File tree

pkg/http/servercard/handler.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,17 @@ func NewHandler(cfg Config) *Handler {
2727
return &Handler{cfg: cfg}
2828
}
2929

30-
// RegisterRoutes mounts the Server Card handler at the reserved
31-
// `<streamable-http-url>/server-card` location. Because GitHub's hosted edge
32-
// strips the `/mcp` base path before forwarding, both `/server-card` and
33-
// `/mcp/server-card` are registered so the card is reachable either way. The
34-
// handler is registered for all methods (mirroring oauth.AuthHandler) so it
35-
// owns the path and answers non-GET requests itself rather than falling through
36-
// to the auth-gated MCP endpoint.
30+
// RegisterRoutes mounts the Server Card handler at the single reserved
31+
// `<streamable-http-url>/server-card` location. The handler is registered for
32+
// all methods (mirroring oauth.AuthHandler) so it owns the path and answers
33+
// non-GET requests itself rather than falling through to the auth-gated MCP
34+
// endpoint.
35+
//
36+
// The card is served at exactly one canonical location — the URL the MCP/AI
37+
// catalog links — so it is deliberately not also exposed under any alternate
38+
// path.
3739
func (h *Handler) RegisterRoutes(r chi.Router) {
3840
r.Handle(Path, h)
39-
r.Handle("/mcp"+Path, h)
4041
}
4142

4243
// ServeHTTP serves the Server Card as application/mcp-server-card+json.

pkg/http/servercard/handler_test.go

Lines changed: 88 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -129,37 +129,101 @@ func TestHandlerRegisterRoutes(t *testing.T) {
129129
r := chi.NewRouter()
130130
NewHandler(Config{}).RegisterRoutes(r)
131131

132-
for _, path := range []string{Path, "/mcp" + Path} {
133-
t.Run(path, func(t *testing.T) {
134-
t.Parallel()
132+
t.Run("GET serves the card at the canonical path", func(t *testing.T) {
133+
t.Parallel()
135134

136-
req := httptest.NewRequest(http.MethodGet, path, nil)
137-
rec := httptest.NewRecorder()
138-
r.ServeHTTP(rec, req)
135+
req := httptest.NewRequest(http.MethodGet, Path, nil)
136+
rec := httptest.NewRecorder()
137+
r.ServeHTTP(rec, req)
139138

140-
res := rec.Result()
141-
defer res.Body.Close()
139+
res := rec.Result()
140+
defer res.Body.Close()
142141

143-
assert.Equal(t, http.StatusOK, res.StatusCode)
144-
assert.Equal(t, MediaType, res.Header.Get(headers.ContentTypeHeader))
145-
})
142+
assert.Equal(t, http.StatusOK, res.StatusCode)
143+
assert.Equal(t, MediaType, res.Header.Get(headers.ContentTypeHeader))
144+
})
146145

147-
t.Run(path+" POST owned by card handler", func(t *testing.T) {
148-
t.Parallel()
146+
t.Run("POST owned by card handler", func(t *testing.T) {
147+
t.Parallel()
149148

150-
// The handler is registered for all methods so non-GET requests are
151-
// answered here (405) rather than falling through to another route.
152-
req := httptest.NewRequest(http.MethodPost, path, nil)
153-
rec := httptest.NewRecorder()
154-
r.ServeHTTP(rec, req)
149+
// The handler is registered for all methods so non-GET requests are
150+
// answered here (405) rather than falling through to another route.
151+
req := httptest.NewRequest(http.MethodPost, Path, nil)
152+
rec := httptest.NewRecorder()
153+
r.ServeHTTP(rec, req)
155154

156-
res := rec.Result()
157-
defer res.Body.Close()
155+
res := rec.Result()
156+
defer res.Body.Close()
158157

159-
assert.Equal(t, http.StatusMethodNotAllowed, res.StatusCode)
160-
assert.Equal(t, "*", res.Header.Get("Access-Control-Allow-Origin"))
161-
})
162-
}
158+
assert.Equal(t, http.StatusMethodNotAllowed, res.StatusCode)
159+
assert.Equal(t, "*", res.Header.Get("Access-Control-Allow-Origin"))
160+
})
161+
162+
t.Run("card is served at exactly one path", func(t *testing.T) {
163+
t.Parallel()
164+
165+
// The card must be discoverable at a single canonical location only;
166+
// no alternate path (e.g. /mcp/server-card) is registered.
167+
req := httptest.NewRequest(http.MethodGet, "/mcp"+Path, nil)
168+
rec := httptest.NewRecorder()
169+
r.ServeHTTP(rec, req)
170+
171+
res := rec.Result()
172+
defer res.Body.Close()
173+
174+
assert.Equal(t, http.StatusNotFound, res.StatusCode)
175+
})
176+
}
177+
178+
// TestHandlerRegisterRoutesNotShadowedByCatchAll mirrors the production wiring
179+
// (pkg/http/server.go), where the streamable MCP endpoint is mounted as a
180+
// catch-all at "/" (pkg/http/handler.go: r.Mount("/", h)). The card's static
181+
// route must take precedence over that wildcard mount so the card — and not the
182+
// auth-gated MCP endpoint — answers GET and non-GET requests at the card path.
183+
func TestHandlerRegisterRoutesNotShadowedByCatchAll(t *testing.T) {
184+
t.Parallel()
185+
186+
const mcpStatus = http.StatusUnauthorized // sentinel for the auth-gated MCP endpoint
187+
188+
r := chi.NewRouter()
189+
r.Group(func(r chi.Router) {
190+
r.Mount("/", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
191+
w.WriteHeader(mcpStatus)
192+
}))
193+
})
194+
r.Group(func(r chi.Router) {
195+
NewHandler(Config{}).RegisterRoutes(r)
196+
})
197+
198+
t.Run("GET is owned by the card handler", func(t *testing.T) {
199+
t.Parallel()
200+
201+
req := httptest.NewRequest(http.MethodGet, Path, nil)
202+
rec := httptest.NewRecorder()
203+
r.ServeHTTP(rec, req)
204+
205+
res := rec.Result()
206+
defer res.Body.Close()
207+
208+
assert.Equal(t, http.StatusOK, res.StatusCode)
209+
assert.Equal(t, MediaType, res.Header.Get(headers.ContentTypeHeader))
210+
})
211+
212+
t.Run("non-GET is owned by the card handler, not the catch-all", func(t *testing.T) {
213+
t.Parallel()
214+
215+
// A POST must get the card handler's 405, not the MCP catch-all's
216+
// sentinel status — proving the static route is not shadowed.
217+
req := httptest.NewRequest(http.MethodPost, Path, nil)
218+
rec := httptest.NewRecorder()
219+
r.ServeHTTP(rec, req)
220+
221+
res := rec.Result()
222+
defer res.Body.Close()
223+
224+
assert.Equal(t, http.StatusMethodNotAllowed, res.StatusCode)
225+
assert.NotEqual(t, mcpStatus, res.StatusCode)
226+
})
163227
}
164228

165229
func TestHandlerETagConditionalRequests(t *testing.T) {

0 commit comments

Comments
 (0)