GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(oauth): preserve existing refresh_token when refresh response omits it by oedokumaci · Pull Request #2271 · modelcontextprotocol/python-sdk

Conversation

@oedokumaci

@oedokumaci oedokumaci commented Mar 11, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Fixes #2270

Per RFC 6749 Section 6, the authorization server MAY issue a new refresh token in the refresh response. When it doesn't, _handle_refresh_response overwrites the stored token as-is, discarding the existing refresh_token. After the first successful refresh, can_refresh_token() returns False and subsequent refreshes fail.

This preserves the existing refresh_token when the response omits one, using model_copy(update=...).

Also removes # pragma: no cover from _handle_refresh_response (added in a bulk coverage baseline, not intentional) and adds tests covering all branches: success with preservation, success with rotation, no prior tokens, non-200 failure, and invalid JSON.

Note: #2270 hasn't received the ready for work label yet. Submitting this early since the fix is small and self-contained. Happy to wait or adjust based on maintainer feedback.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@oedokumaci
fix(oauth): preserve existing refresh_token when refresh response omi…
ae5050b 
…ts it (modelcontextprotocol#2270)
@oedokumaci
oedokumaci force-pushed the fix/preserve-refresh-token branch from 6758180 to ae5050b Compare March 11, 2026 14:52
@oedokumaci oedokumaci mentioned this pull request Mar 11, 2026
fix(mcp): pass OAuthClientProvider to call_mcp_tool for automatic token refresh onyx-dot-app/onyx#9267
Closed
2 tasks
@maxisbey maxisbey mentioned this pull request Mar 24, 2026
fix: preserve existing refresh_token when server omits it in refresh response #2304
Closed
@mikeasick-fennec

mikeasick-fennec commented Apr 16, 2026

Copy link
Copy Markdown

Hit this in production against Zoho's MCP bridge (token endpoint at
mcp.zoho.com/baas/mcp/v1/oauth///token). Zoho uses long-lived
refresh tokens -- the refresh response returns only {access_token,
api_domain, token_type, expires_in} with no refresh_token field. This
is spec-compliant per RFC 6749 §6 (MAY issue a new one).

Exact failure mode described in #2270:

  1. First silent refresh succeeds. _handle_refresh_response replaces
    current_tokens with the parsed body, which has refresh_token=None.
  2. That refresh_token-less OAuthToken is persisted to storage.
  3. Next session (or next expiry): can_refresh_token() returns False,
    client falls through to interactive OAuth.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@maxisbey

maxisbey commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Thanks for the PR. This has since landed via #2946. Closing this as part of a wider backlog cleanup following the v2 launch. If it's still relevant, feel free to reopen.

AI Disclaimer
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@maxisbey maxisbey closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

_handle_refresh_response discards existing refresh_token when server omits it

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@oedokumaci @mikeasick-fennec @maxisbey
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.