fix(oauth): keep the server's path in the fallback authorization-server URL (#2110)

[cliffhall]

Closes#2110

The defect

When RFC 9728 protected-resource metadata names no authorization server, the MCP server URL stands in as one. core/auth/discovery.ts built that URL with:

newURL("/",serverUrl)

which discards the path — exactly the part discovery needs when the server is not hosted at the domain root. The SDK's buildDiscoveryUrls derives the path-scoped well-known locations (/.well-known/oauth-authorization-server{path}, /.well-known/openid-configuration{path}, {path}/.well-known/openid-configuration) from that pathname, so an origin-only value can only ever probe the domain root. For the reporter's server — https://misc.poodll.com/mod/minilesson/mcp.php — every probe 404s and the flow dies before authorization is ever reached.

The fix

Keep the path, and strip query/fragment (an RFC 8414 issuer identifier carries neither, and the SDK compares the discovered issuer against this URL).

A straight swap would have regressed the case the origin-only value did serve: a server that merely lives under a path while publishing its metadata at the domain root. So the corrected URL becomes the first of two candidates rather than the only answer:

• getAuthorizationServerUrlCandidates() — the path-scoped URL first, the bare origin second. When protected-resource metadata does name an authorization server there is exactly one candidate; the server told us where to look, and probing past it would be guessing.
• discoverAuthorizationServerMetadataForServer() — walks them. A candidate that throws (a 5xx, or an RFC 8414 §3.3 issuer mismatch, both of which the SDK raises rather than returning undefined) is not fatal while another candidate remains; if every candidate fails the first error is rethrown, so callers keep the original diagnosis instead of a bare undefined.

Both in-repo consumers go through the walk: discoverScopes() and the CIMD pre-registration probe (core/auth/cimd.ts). EMA's resourceContext keeps the single-URL getAuthorizationServerUrl(), which now preserves the path too — for a path-hosted resource the full URL is the more correct audience.

Scope — what this PR does not fix

The issue reports three defects. Only the first has a copy in this repo; the other two are in the pinned SDK (@modelcontextprotocol/client@2.0.0) and are not reachable from here:

Reported bug	Where it lives	Status
1. Fallback AS URL drops the path	core/auth/discovery.tsand the SDK's discoverOAuthServerInfo	Fixed here (our copy). The SDK copy is inside auth() and is not overridable.
2. /.well-known/openid-configuration forces the full OIDC schema, rejecting a plain OAuth 2.0 AS	SDK OpenIdProviderDiscoveryMetadataSchema	Not ours — belongs upstream.
3. Protected-resource discovery has no append-form fallback	SDK discoverMetadataWithFallback	Not ours. Note the append form is an OIDC-ism, not RFC 9728 §3.1, which specifies the insert form the SDK already tries. Largely moot in practice since #2071: a server advertising resource_metadata on its 401 gets that URL used verbatim.

Tests

clients/web/src/test/integration/auth/discovery.test.ts gains a path-hosted MCP servers (#2110) block: path preserved, query/fragment stripped, candidate ordering (path-hosted → 2, root-hosted → 1, metadata-named → 1), and the walk's five outcomes — first candidate wins without probing further, falls through on undefined, survives a throwing candidate, rethrows the first error when all fail, and forwards the caller's fetchFn to every candidate.

npm run ci passes. Coverage: discovery.ts 100% lines / 95.45% branches, cimd.ts unchanged at 92.85%.

No UI surface changed, so there are no screenshots to attach.

🤖 Generated with Claude Code

https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev

[Copilot]

Pull request overview

Fixes OAuth metadata discovery for MCP servers hosted below the domain root.

Changes:

• Preserves server paths and tries path-scoped discovery before the origin.
• Applies candidate discovery to scope and CIMD flows.
• Adds regression tests and architectural documentation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
core/auth/discovery.ts	Adds path-preserving candidate discovery.
core/auth/cimd.ts	Uses candidate discovery for CIMD probing.
clients/web/src/test/integration/auth/discovery.test.ts	Tests path-hosted discovery behavior.
AGENTS.md	Documents the authorization-server URL policy.

---

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

[cliffhall]

Copilot review round 1 — both comments addressed (2bb0b62)

Mirroring the inline replies here, since they get folded away once the fix is pushed.

1. refreshStoredAuthToken still used the single-URL fallback — real regression, fixed.

Copilot is right, and this was the one consumer I left behind. clients/cli/src/cli.ts consumed getAuthorizationServerUrl(serverUrl) and made a single SDK discovery call, so preserving the path there without the walk would have broken exactly the compatibility case the walk exists to keep: a path-hosted MCP server whose authorization metadata is published at the domain root.

Two details shaped the fix:

• It needs which candidate answered, not just the metadata — that URL is also the base the refreshAuthorization token request is made against. So the walk is now exposed as discoverAuthorizationServerMetadataFromCandidates(candidates, discover) returning { authorizationServerUrl, metadata }, and discoverAuthorizationServerMetadataForServer is a thin wrapper over it.
• The injected discover test seam is retained, which is why the walk takes the discovery function as a parameter rather than closing over the SDK's.

When no candidate answers, authServerUrl stays the path-scoped candidate and metadata stays undefined, preserving the existing behavior that test already pins.

Three regression tests in clients/cli/__tests__/stored-auth.test.ts (its SERVER fixture is already path-hosted, https://api.example/mcp): path-scoped candidate answers first with no second probe; the root-metadata case, asserting the second probe hits the origin and that both authServerUrl and metadata come from it; and the all-candidates-silent case.

2. Unjustified as unknown as in the new test — fixed.

Replaced with vi.fn<typeof fetch>(). The cast was only there because the assertion needs the same reference back out of toHaveBeenCalledWith, which the typed form gives just as well.

npm run ci passes. Coverage: discovery.ts 100% lines / 95.45% branches, cli.ts 99.57% / 92.82%, cimd.ts unchanged.

AGENTS.md updated to state the invariant the miss came from: every consumer of the fallback must walk, and the ...FromCandidates form exists for the one that injects its own discovery function.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.