Repository navigation
refactor(mcp): extract upstream preparation and support modern clients - #44232
Conversation
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
bugbot run |
|
@greptileai please re-review a740975. The modern SSE persistence finding is fixed with shared validation and passing create/partial-update persistence regressions. |
|
@greptileai please re-review ca7f23d. Both transport-validation findings now have passing sequential and deterministic two-worker persistence regressions. |
|
bugbot run |
There was a problem hiding this comment.
✅ 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 ca7f23d. Configure here.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: adding a modern MCP upstream fails validation
http://localhost:18402/v1/mcp/serverwith a server URL,transport: "http", andmcp_info.protocol_version: "2026-07-28"2025-11-25instead permits listing tools and callingadd(2,3), returning5After: the configured modern upstream serves tools successfully
http://localhost:18402/v1/mcp/serverwith a server URL,transport: "http", andmcp_info.protocol_version: "2026-07-28"/mcp-rest/tools/list?server_id=<id>, then POST/mcp-rest/tools/callwith that ID,name: "add", andarguments: {"a":2,"b":3}; receive HTTP 200 and5Linear ticket
Resolves LIT-7745
Implementation
upstream.pyowns credential selection, header precedence, SigV4, stdio configuration and client preparation. It reuses the existing credential provider and error mapping; the manager retains metadata discovery and request-context captureMCPServerTargetCatalogcomposes the existing lookup and authorization helpers. Credential management and temporary OAuth lookup use the contract while preserving canonical server identity, caller permissions and endpoint-specific errorsExplicit
2026-07-28upstreams use the SDK's discovery/adoption path on HTTP and stdio. Legacy revisions retain initialization. Downstream advertisement stays unchanged; modern SSE is rejected before creation or update. Partial updates validate the stored configuration plus explicitly submitted fields. The same compatibility rule guards request models, config/runtime models, and the public client. Protocol/transport writes also re-read and validate under a database row lock in the existing write transaction, so concurrent partial edits cannot combine into an invalid saved server. Pooling, worker affinity and catalog freshness remain outside this changePre-Submission checklist
Verification
At
ca7f23dad688fae60ddf32033284d9c7f3242051, refreshed mapped unit suites passed: 1,806 manager/auth/resolver/consumer tests (one existing skip), 411 client tests, and 130 request-type tests. After an explicit type-annotation correction, 621 management/request-type tests and 26 protocol client cases passed again.The refreshed canonical MCP integration run passed all 386 selected tests, excluding only the previously classified unrelated SSE end-user REST authorization case. The earlier full run had 384 passes and that one failure. All five new persistence/concurrency regressions and eight modern gateway transport cases passed. Final-commit live source replay confirms default/explicit modern SSE creates return 422 with no saved server; incompatible partial updates return 400 without changing the stored configuration. Valid edits and legacy/modern tool calls still succeed.
Whole-product and configured test-tree Ruff, formatting, strict-rule, type-discipline, test-quality, canonical basedpyright budget, E2E types, import checks and schema synchronization passed locally.
Changed executable-line coverage is 181/182 (99.45%). Validation helpers have 100% line and branch coverage (12/12 branches). Overall touched-function branch coverage is 132/144 (91.67%); remaining gaps include unchanged OAuth guards/cache invalidation and client cleanup paths, plus closed-union fallthroughs. The sole missed changed line remains the defensive SDK revision consistency check unreachable with the locked SDK after supported-version validation. Completed current-tip Codecov report: 98.38% patch coverage; repository-wide coverage 80.25%. Its two extra missing request-model lines are blank lines (1686 and 1781), a hosted mapping artifact, not executable gaps.
Independent Devin MCP verification passed at final
ca7f23dad688fae60ddf32033284d9c7f3242051, against original baseline2c9a9711712895a7e08ba27fca357291f4b86290, sequential pre-repairc479a4af2b0297e750f568db4cecf7f5eae871dd, and concurrency pre-repaira74097531fe31e5d67c1daf3b05d8aeeec5d87a0. Sources were clean; processes were restarted from their measured source paths. The shared enterprise directory used for the immediate race baseline was byte-identical between baseline and final.Independent results: original gateway create/list/call and HTTP/stdio credential/lifecycle cases passed; sequential invalid writes were rejected without persistence. Six locked-row race repetitions saved invalid configurations before repair; all six final repetitions produced one accepted edit and one 400, retained valid configuration, and served list/call successfully. The initial incorrectly timed lock harness was discarded and is not evidence.
pytest -n 4on request types, MCP management endpoints, public client and partial-update files: 1,089 passed. The additional database helper file passed seven tests. The five focused live create/update/concurrency tests passed. These controlled peers do not claim real OAuth consent or AWS authorization.Greptile reviewed this commit at 5/5 and confirmed both findings resolved; Veria and Cursor Bugbot found no issues. The current-tip Codecov report is complete and passing. All 102 completed non-skipped hosted checks pass, including MCP integration, OAuth, lint/types and Codecov. The separate schema-migration job is still running; no schema or migration files changed.
Screenshots / Proof of Fix
Run the repository math MCP server at
http://127.0.0.1:18090/mcpand a source-built proxy athttp://localhost:18402. SetLOCAL_MASTER_KEYto the local test proxy's master key. Use the same requests on each source build, deleting each created server after its case. Response excerpts below show the relevant fieldsFor the legacy control, use
2025-11-25andprooflegacyin the creation body.SERVER_IDis the ID returned by each successful creationBefore (2c9a971)
Legacy upstream
add{"content":[{"type":"text","text":"5"}],"isError":false}Modern upstream
literal_errorunderbody.mcp_infoAfter (ca7f23d)
Legacy upstream
70216994-66df-44c4-8a90-3253aa138ecaadd{"content":[{"type":"text","text":"5"}],"isError":false}Modern upstream
8723fd94-4b8e-4bec-9853-72ecb6633c6aadd{"content":[{"type":"text","text":"5"}],"isError":false}Transport-validation repair
On pre-repair
c479a4af2b0297e750f568db4cecf7f5eae871dd, modern SSE creation returned 201 and incompatible transport-only/protocol-only updates returned 202. Matching requests on finalca7f23dad688fae60ddf32033284d9c7f3242051return 422/400 before saving. GET confirms rejected creates are absent and rejected updates retain HTTP; a subsequentadd(2,3)still returns5. Metadata-only edits, reset to auto/SSE, and a combined modern/HTTP update remain accepted.The concurrent-edit follow-up also reproduces a two-worker race at
a74097531fe31e5d67c1daf3b05d8aeeec5d87a0: protocol-only and transport-only requests both pass a stale pre-check and save an invalid combination (202/202 or 202/500). At the final commit, one succeeds and the other returns 400; the stored combination remains valid. Both plain edits and a simultaneous rename are covered. All 298 affected database unit tests pass; changed database functions have full line and branch coverage.Type
Refactoring
New Feature
Caveats (if any)
Low
autoretains existing behaviorFinal Attestation