fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: preserve transient OAuth discovery HTTP errors - #1071

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors
Jul 28, 2026
Merged

fix: preserve transient OAuth discovery HTTP errors#1071
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
tsarlandie-oai:codex/oauth-discovery-transient-http-errors

Conversation

@tsarlandie-oai

@tsarlandie-oaitsarlandie-oai commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1069.

OAuth metadata discovery now preserves transport failures and HTTP 5xx responses, but HTTP 408 Request Timeout, 425 Too Early, and 429 Too Many Requests still fall through as though a metadata endpoint were absent. As a result, a temporary timeout, early-data rejection, or rate limit can cause discovery to continue to another endpoint or report a legacy fallback instead of surfacing the operational failure.

This change classifies HTTP 408, 425, and 429 alongside 5xx in the existing discovery_request helper. The resulting metadata error preserves both the actual response status and the failing discovery URL.

Compatibility

  • Preserve HTTP 401 challenge handling.
  • Preserve HTTP 404 and 405 discovery fallback, with separate regression cases for both.
  • Preserve the existing HTTP 5xx behavior.
  • Do not introduce automatic retries, new dependencies, or public API changes.

Regression coverage

Add nine parameterized cases covering HTTP 408, 425, and 429 at each of:

  1. The protected resource endpoint.
  2. The protected resource metadata endpoint.
  3. The authorization server metadata endpoint.

Each case verifies that discovery reports the correct failing URL and status and does not issue additional discovery requests. The regression was first reproduced against upstream main before applying the production fix.

Validation

  • cargo +1.96 test -p rmcp --lib --all-features (482 tests passing).
  • cargo +1.96 clippy --all-targets --all-features -- -D warnings.
  • cargo +nightly-2025-09-18 fmt --all -- --check.

@github-actionsgithub-actionsBot added T-core Core library changes T-transport Transport layer changes labels Jul 28, 2026
@tsarlandie-oai
tsarlandie-oai marked this pull request as ready for review July 28, 2026 16:58
@tsarlandie-oai
tsarlandie-oai requested a review from a team as a code ownerJuly 28, 2026 16:58
DaleSeo
DaleSeo previously approved these changes Jul 28, 2026

@DaleSeoDaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tightening the discovery error boundary!

Comment threadcrates/rmcp/src/transport/auth.rs Outdated
if status.is_server_error()
|| matches!(
status,
StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would 425 would make the transient classification more complete?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

added

@tsarlandie-oai
tsarlandie-oaiforce-pushed the codex/oauth-discovery-transient-http-errors branch from 7d80f6d to bf9e7c5CompareJuly 28, 2026 18:53
@DaleSeo
DaleSeo merged commit 82a6c48 into modelcontextprotocol:mainJul 28, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-transportTransport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@tsarlandie-oai@DaleSeo