feat(client): add an opt-in for sending scope on token refresh by claude[bot] · Pull Request #3085 · modelcontextprotocol/typescript-sdk

claude · 2026-10-23T15:44:35Z

Requested by Felix Weinberger · Slack thread

Some authorization servers reject a refresh-token request that carries no
scope (#2718: Microsoft Entra ID answers AADSTS90009 when the client and the
resource are the same app registration). Omitting it is allowed by RFC 6749
section 6 and other servers reject or narrow a refresh that does carry one, so
the default request does not change.

OAuthClientProvider.scopeForRefresh?(tokens) returns the scope auth()
sends on a refresh. Every caller of auth() takes the provider (both HTTP
transports, their step-up path, withOAuth), so one member reaches them all.
refreshAuthorization() takes an optional scope for hosts that call it
directly. An empty string sends nothing.
When a scope was sent and the response has none, the sent scope stays on the
returned tokens (RFC 6749 section 5.1: an omitted scope equals the requested
one). Without this, tokens => tokens.scope has nothing to return on the
second refresh. It never applies when no scope is sent.

A rejected refresh is handled as before: invalid_scope is thrown to the caller. An error thrown by the hook rejects auth().

Alternative to #2720, which sends the stored scope by default.

Generated by Claude Code

Some authorization servers reject a refresh-token request that carries no `scope` (#2718: Microsoft Entra ID answers AADSTS90009 when the client and the resource are the same app registration). Omitting it is allowed by RFC 6749 section 6 and other servers reject or narrow a refresh that does carry one, so the default request does not change. - `OAuthClientProvider.scopeForRefresh?(tokens)` returns the scope `auth()` sends on a refresh. Every caller of `auth()` takes the provider (both HTTP transports, their step-up path, `withOAuth`), so one member reaches them all. - `refreshAuthorization()` takes an optional `scope` for hosts that call it directly. An empty string sends nothing. - When a scope was sent and the response has none, the sent scope stays on the returned tokens (RFC 6749 section 5.1: an omitted scope equals the requested one). Without this, `tokens => tokens.scope` has nothing to return on the second refresh. It never applies when no scope is sent. A rejected refresh is handled as before: `invalid_scope` is thrown to the caller. An error thrown by the hook rejects `auth()`. Alternative to #2720, which sends the stored scope by default.

changeset-bot · 2026-10-23T15:44:45Z

🦋 Changeset detected

Latest commit: 4b559da

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Minor
@modelcontextprotocol/codemod	Minor
@modelcontextprotocol/core	Minor
@modelcontextprotocol/server-legacy	Minor
@modelcontextprotocol/server	Minor
@modelcontextprotocol/core-internal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pkg-pr-new · 2026-10-23T15:46:13Z

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3085

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3085

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3085

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3085

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3085

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3085

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3085

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3085

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3085

commit: 4b559da

claude

Findings marked 🟡 are optional suggestions and need no follow-up push.

claude · 2026-10-23T15:51:29Z

+    'client-auth:refresh:no-scope-by-default': {
+        source: 'sdk',
+        behavior:
+            'Unless the provider implements scopeForRefresh(), a refresh request carries grant_type, refresh_token, the resource indicator and client authentication only: no scope parameter, even when the stored tokens record a scope.',
+        transports: ['streamableHttp'],
+        note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.'
+    },
+    'client-auth:refresh:scope-opt-in': {


🟡 nit (optional): Maintainers of the shared e2e manifest get two unprefixed requirement ids whose wording names a TypeScript-only provider hook. client-auth:refresh:no-scope-by-default and client-auth:refresh:scope-opt-in both describe scopeForRefresh(), so the wording cannot be shared with the Python suite. Fix: give ids whose behaviour is defined by this SDK's own API the typescript: prefix, in the manifest and in the two verifies() calls in test/e2e/scenarios/client-auth.test.ts, or reword them API-neutrally; this covers the 2 sites listed. Same instruction at 2 sites (test/e2e/requirements.ts:2244, test/e2e/requirements.ts:2251). [also at: test/e2e/requirements.ts:2251 - nit: test/e2e/CLAUDE.md puts requirements that describe this SDK's own API surface under a typescript: slug and keeps unprefixed slugs for ids and wording shared with the Python interaction suite.]

Why this was flagged

No runtime trigger: this is a manifest naming convention. test/e2e/requirements.ts:2244 and :2251 add unprefixed ids with source: 'sdk' whose behavior text is written in terms of scopeForRefresh() / scopeForRefresh(tokens), a hook that exists only on this SDK's OAuthClientProvider (packages/client/src/client/auth.ts:400). test/e2e/CLAUDE.md:95 reserves unprefixed slugs for ids and wording shared with the Python interaction suite and puts requirements that describe this SDK's own API surface under typescript:. The closest existing entry, a provider hook requirement at test/e2e/requirements.ts:2204, is typescript:client-auth:state:verify. The base branch has neither id, so the mismatch is new. Precedent is mixed (client-auth:middleware:with-oauth at :2223 is unprefixed and names withOAuth), and no gate catches it, so the cost is a cross-SDK id that the Python suite cannot adopt verbatim.

Verification: Triggering condition: none at runtime; it is a manifest naming/taxonomy break that exists as soon as the two entries merge. The diff adds two unprefixed ids whose behavior text is defined by a TypeScript provider hook, at test/e2e/requirements.ts:2244-2247 and :2251-2254. Nothing in coverage.test.ts gates the prefix, so nothing fails.

_{Finding ID: f-1317a947}

claude · 2026-10-23T15:51:29Z

+}
+
+verifies('client-auth:refresh:no-scope-by-default', async (_args: TestArgs) => {
+    const world = createScopeRequiringWorld();


🟡 nit (optional): test/e2e/CLAUDE.md asks each verifies() body to be self-contained (build server inline, build client inline, assert). The two new scenarios take their MCP host, mock authorization server, fetch routing and stored tokens from a new module-level createScopeRequiringWorld() helper, so neither body shows the server it runs against. Fix: build the host and the scope-requiring token endpoint inside each verifies() body (as client-auth:refresh:transparent just above does), which covers the 2 sites listed. Same instruction at 2 sites (test/e2e/scenarios/client-auth.test.ts:1527, test/e2e/scenarios/client-auth.test.ts:1553).

Why this was flagged

Nothing fails at runtime. The instruction guards readability and independence of each scenario: a reader of one verifies() body should see the server, client and assertion without chasing a shared fixture, and a change made for one scenario should not silently alter another. Here the rule that the AS rejects a refresh without scope, the omission of scope/refresh_token from the response, and the token-expiry mechanism all live in the 37-line helper at lines 1489-1524, so the expected invalid_request in the first body and the 'two expiries' logic in the second are only understandable by reading the helper. Size is small: the helper is in the same file (not a shared fixture file, which the same line also forbids), it is used by exactly two adjacent bodies, and this file already has module-level helpers of the same kind (createAuthenticatedHost, createCombinedFetch, runFinishAuthScenario, createMigratingAuthorizationServer), so the author is following local precedent.

Verification: Instruction: test/e2e/CLAUDE.md at base 9adb8d5, section "Writing a test": "Self-contained: build server inline (factory), build client inline, wire(), assert. No shared fixture files." It is present as quoted and binds test/e2e/scenarios/client-auth.test.ts. Code: the added bodies at test/e2e/scenarios/client-auth.test.ts:1527 (const world = createScopeRequiringWorld(); in client-auth:refresh:no-scope-by-default) and :1554 (same call in client-auth:refresh:scope-opt-in) do not build the server inline.

_{Finding ID: f-25bff81a}

claude · 2026-10-23T15:51:29Z

    }
 });

+/** An authorization server that rejects a refresh without `scope`, and an MCP host whose access tokens can be expired. */


🟡 nit (optional): CLAUDE.md principle 8 keeps JSDoc to the public API. The added /** An authorization server that rejects a refresh without scope, ... */ block documents createScopeRequiringWorld(), a file-private test helper. Fix: use a one-line // comment (or none) on helpers that are not part of the published API; the JSDoc added in packages/client/src/client/auth.ts on scopeForRefresh and the scope option is on public API and is fine.

Why this was flagged

Nothing fails at runtime. The instruction keeps doc-comment blocks reserved for the published surface so that JSDoc signals 'public contract' and internal code carries only brief inline notes. This is a single one-line /** */ on a test-local function, so the practical cost is negligible; it is a wording/layout matter only. The same file already has JSDoc on other private helpers (lines 807, 2863, 2925), so this follows local precedent.

Verification: Instruction: root CLAUDE.md at base 9adb8d5, principle 8, verbatim as quoted: "Brief comments — Keep inline comments to one line and JSDoc to the public API; the reasoning goes in the commit message or PR description." It is a root-level file, so it binds test/e2e/scenarios/client-auth.test.ts.

_{Finding ID: f-01c04ab5}

… scope

claude

Code review found no new issues

No new issues were found in this update; 4 findings from earlier reviews are still open above.

Still open from earlier reviews (4):

🔴 packages/client/src/client/auth.ts:1409 — Clients that opt in to scopeForRefresh can get stuck: once the server answers a refresh with invalid_scope, every later…
Also unresolved: 3 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

claude

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (3):

Unresolved: 3 minor or pre-existing.

Resolves conflicts with #3085 (opt-in scope on token refresh) in refreshAuthorization's result merge and its tests. The merge keeps this branch's hardened spread order, in which a present-but-undefined refresh_token from the parsed response can never clobber the preserved refresh token, and adds #3085's fallback that keeps the sent scope when the response has none. The refreshAuthorization test blocks from both sides are retained. One #3085 assertion is adjusted: the test that pins no scope being sent on refresh by default also pinned the saved payload without the stored scope, which is the pre-existing behavior this branch changes on review (a refresh response that omits scope asserts an unchanged grant, so the stored scope is preserved rather than erased). The request-body assertion that is the test's subject is unchanged. The two behaviors compose: no scope is sent by default, a sent scope is saved when the response omits one, and otherwise the stored scope survives. Also shrinks the comment above the scope-preservation merge to one line, per the inline-comment convention. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KXnNnp3fxQYR5HF9BUVhYP

claude Bot requested a review from a team as a code owner October 23, 2026 15:44

claude Bot commented Oct 23, 2026

View reviewed changes

fix(client): keep the scope sent when a refresh response has an empty…

0f4f96b

… scope

claude Bot commented Oct 23, 2026

View reviewed changes

felixweinberger approved these changes Oct 23, 2026

View reviewed changes

Merge branch 'main' into feat/refresh-scope-opt-in

164ac1a

claude Bot commented Oct 23, 2026

View reviewed changes

Merge branch 'main' into feat/refresh-scope-opt-in

4b559da

felixweinberger merged commit a633756 into main Oct 23, 2026
19 checks passed

felixweinberger deleted the feat/refresh-scope-opt-in branch October 23, 2026 17:25

github-actions Bot mentioned this pull request Oct 23, 2026

Version Packages #2959

Merged

This was referenced Oct 23, 2026

[v2] Incompatibility with refresh_tokens from Azure when client_id=resource_id #2718

Closed

fix(client): send the scope parameter on refresh-token requests #2720

Closed

[v1.x] fix(client): send the scope parameter on refresh-token requests #2937

Closed

claude Bot mentioned this pull request Oct 23, 2026

fix(auth): treat null optional fields in token responses as absent #2462

Open

Navigation Menu

Repository navigation

feat(client): add an opt-in for sending scope on token refresh - #3085

feat(client): add an opt-in for sending scope on token refresh#3085
felixweinberger merged 4 commits into
mainfrom
feat/refresh-scope-opt-in

claude Bot commented Oct 23, 2026

Uh oh!

changeset-bot Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Oct 23, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot Oct 23, 2026

Uh oh!

claude Bot Oct 23, 2026

Uh oh!

claude Bot Oct 23, 2026

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

claude Bot commented Oct 23, 2026

Uh oh!

changeset-bot Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pkg-pr-new Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot Bot commented Oct 23, 2026 •

edited

Loading

pkg-pr-new Bot commented Oct 23, 2026 •

edited

Loading