Skip to content

test(mcp): cover SDK redirect compatibility - #42033

Merged
joshua-berri merged 3 commits into
mainfrom
litellm_mcp_redirect_regressions_7738
Sep 19, 2026
Merged

joshua-berri merged 3 commits into
mainfrom
litellm_mcp_redirect_regressions_7738

Conversation

@joshua-berri

@joshua-berri joshua-berri commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Existing redirect tests bypass the MCP SDK policy

How it solves it:

  • Exercises real SDK redirects through LiteLLM list and call
  • Proves rejected destinations receive no requests or credentials
  • Documents method-preserving redirects and configuring final endpoint URLs

User Flow

Before: redirect behavior works, but SDK-level regression coverage is missing

  1. Configure an MCP endpoint returning HTTP307 to another path on the same origin
  2. List tools and call add with arguments 2 and 3, receiving 5
  3. Configure an endpoint redirecting to another host and receive an explicit redirect rejection

After: the same behavior is protected by focused tests and documented

  1. Configure an MCP endpoint returning HTTP307 to another path on the same origin
  2. List tools and call add with arguments 2 and 3, receiving 5
  3. Configure an endpoint redirecting to another host and receive an explicit redirect rejection

Relevant issues

Follow-up coverage for merged #41718, preserving its SDK2 adapter implementation

Affected release

No new production behavior change

Linear ticket

Resolves LIT-7738

https://linear.app/litellm-ai/issue/LIT-7738/mcp-phase-0-mcp-httpx2-adapter-integration

Pre-Submission checklist

  • Added meaningful tests in the existing mapped file
  • All 192 mapped tests pass locally with branch coverage
  • Full CI-equivalent lint and budget gates pass locally
  • Scope isolated to tests and documentation
  • Required hosted CI and completed current-tip Codecov report
  • Greptile 5/5, Veria and Bugbot complete with no actionable findings

Screenshots / Proof of Fix

Controlled live HTTP upstream on 127.0.0.1:8091, using the same observer for each source build. /redirect/mcp returns307 to /ok/mcp; /xredirect/mcp returns307 to localhost:8091/ok/mcp, a different origin. Synthetic credential only, recorded as presence. The client uses actual network requests through LiteLLM and MCP SDK2. No LLM needed

The independent verifier runs uv run --no-sync python /home/ubuntu/lit7738/redirect_probe.py 8091 from each source checkout. Exact script, sanitized output, import paths and source hashes are retained with the ticket evidence

Before (7651d6b)

Same-origin redirect

  1. List tools and call add(2,3) through /redirect/mcp
  2. Observe tool add, result 5, and credential presence on redirected requests

Cross-origin redirect

  1. List tools and call add(2,3) through /xredirect/mcp in separate operations
  2. Observe the SDK redirect rejection, one origin request per operation, and zero destination requests

After (7cd96e9)

Same-origin redirect

  1. List tools and call add(2,3) through /redirect/mcp
  2. Observe tool add, result 5, and credential presence on redirected requests

Cross-origin redirect

  1. List tools and call add(2,3) through /xredirect/mcp in separate operations
  2. Observe the SDK redirect rejection, one origin request per operation, and zero destination requests

Independent verifier: all five live cases passed on both commits, and all 192 mapped tests passed at the final tip in 11.82s. Local negative controls fail when same-origin redirects are blocked or cross-origin requests are allowed

Type

Test and documentation

Caveats

Low

  • Characterization tests intentionally pass on the existing production behavior
  • Isolated negative controls fail when allowed redirects break or forbidden requests occur
  • No production executable lines changed, so changed-production-line coverage is inapplicable
  • Misc shard reports unrelated LoggingWorker shutdown warnings
  • Contributor agreement verification remains unavailable through current integrations

Final Attestation

  • Tests check meaningful tool results and denied network side effects
  • Current-tip hosted checks, coverage and required bot reviews complete

@joshua-berri
joshua-berri requested a review from a team September 19, 2026 22:55
@joshua-berri

Copy link
Copy Markdown
Contributor Author

@greptileai Please review 064b7d1 for SDK redirect coverage, meaningful credential assertions, test isolation, and the existing client helper reuse

@joshua-berri

Copy link
Copy Markdown
Contributor Author

bugbot run. Please review 064b7d1 for real SDK execution, zero destination requests, meaningful tool results, and bounded cleanup

@joshua-berri

Copy link
Copy Markdown
Contributor Author

@veria-ai Please review 064b7d1 for redirect credential isolation, rejected destination side effects, existing policy preservation, and focused test coverage

@codspeed

codspeed Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_mcp_redirect_regressions_7738 (7cd96e9) with main (5fc5afd)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because it changes only tests and README guidance, and the current tests preserve the relevant redirect and credential-boundary assertions.

Summary

This PR adds focused SDK-level regression coverage and operator guidance for MCP HTTP redirects.

  • Verifies that same-origin 307 redirects preserve POST requests, authentication, tool listing, and tool calls.
  • Verifies that cross-origin redirects raise MCPError without contacting the destination.
  • Removes dependence on third-party SDK error wording while retaining denial-side-effect assertions.
  • Documents supported redirect behavior and direct final-endpoint configuration.

Reviews (2) · Last reviewed commit: "test(mcp): assert redirect rejection wit..."

Comment thread litellm/experimental_mcp_client/Readme.md
Comment thread tests/test_litellm/experimental_mcp_client/test_mcp_client.py Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@joshua-berri

Copy link
Copy Markdown
Contributor Author

@greptileai Please review 7cd96e9: SDK wording dependency removed, denial side effects remain covered, and the quoted documentation rule exempts READMEs

@joshua-berri

Copy link
Copy Markdown
Contributor Author

bugbot run. Please review 7cd96e9: rejection assertions now tolerate SDK wording changes while preserving credential and zero destination request checks

@joshua-berri

Copy link
Copy Markdown
Contributor Author

@veria-ai Please review 7cd96e9 for credential isolation and zero rejected-destination traffic, with SDK error wording removed from regression assertions

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7cd96e9. Configure here.

@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri mateo-berri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks!

@joshua-berri
joshua-berri merged commit f49fd22 into main Sep 19, 2026
97 checks passed
@joshua-berri
joshua-berri deleted the litellm_mcp_redirect_regressions_7738 branch September 19, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants