fix(client): preserve Request semantics in createProxyFetch#1208
Open
shaun0927 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Open
fix(client): preserve Request semantics in createProxyFetch#1208shaun0927 wants to merge 2 commits intomodelcontextprotocol:mainfrom
shaun0927 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
The proxy auth wrapper handled (url, init) correctly but dropped method, headers, and body when callers supplied a real Request object. That made wrapper behavior depend on call shape and could break Request-based OAuth or proxy flows. This change reads method, headers, and body from Request inputs, lets an explicit init override them, and adds a focused regression test covering the Request path while preserving the existing URL+init behavior. Constraint: Keep the fix narrow and V1-compatible with no transport/proxy refactor Rejected: Broader fetch wrapper redesign | out of scope for a correctness bug fix Confidence: high Scope-risk: narrow Reversibility: clean Directive: Preserve parity between Request input and (url, init) input in future proxyFetch changes Tested: Targeted Node 22 source-backed reproduction before and after fix; added regression test in client/src/lib/__tests__/proxyFetch.test.ts Not-tested: Full workspace Jest/CI run in this local environment
The original fix branch already repaired Request preservation in createProxyFetch, but the test suite only locked the basic Request passthrough case. This follow-up adds coverage for the override rule the implementation promises: explicit init values should still win over the Request's method, headers, and body. It also keeps the small formatting adjustment in proxyFetch.ts consistent with the surrounding style. Constraint: Keep follow-up scope limited to merge-readiness hardening for PR modelcontextprotocol#1208 Rejected: Broader semantic changes around bodyUsed handling | no reviewer request and would expand scope beyond the validated bug Confidence: high Scope-risk: narrow Reversibility: clean Directive: Preserve both Request passthrough and explicit init override semantics in future proxyFetch edits Tested: Targeted Node 22 harness validating override semantics against current source Not-tested: Full workspace Jest/CI run in this local environment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
createProxyFetch()currently preserves request semantics when it is called as(url, init), but not when the caller passes a realRequestobject. In theRequestcase, the proxy payload keepsrequest.urlbut drops the originalmethod,headers, andbody.This patch preserves those fields for
Requestinputs, still lets an explicitinitoverride them, and adds focused regression coverage for both the passthrough and override cases.Type of Change
Changes Made
method,headers, andbodyfromRequestinputs insideclient/src/lib/proxyFetch.ts(url, init)behaviorinitcontinue to override the values derived from theRequestclient/src/lib/__tests__/proxyFetch.test.tsthat cover:Requestpassthrough caseinit-overrides-Requestprecedence caseRelated Issues
Fixes #1207
Testing
Test Results and/or Instructions
Added focused regression tests for the
Requestinput path inclient/src/lib/__tests__/proxyFetch.test.ts.Local verification in this environment used targeted Node 22 harnesses against the checked-out source to confirm:
(url, init)still forwardsmethod,headers, andbodynew Request(...)now forwards the same fields instead of sending an emptyinitRequestand an explicitinitare provided, the explicitinitvalues winReviewers can test the change by running the client unit tests, or by comparing the serialized
POST /fetchpayload before and after this patch whencreateProxyFetch()is called with aRequest.Checklist
npm run prettier-fix)Breaking Changes
None.
Additional Context
This keeps
createProxyFetch()closer to normal fetch-wrapper expectations: callers should not get different proxy behavior solely because they constructed aRequestobject instead of passing(url, init)separately.