Repository navigation
feat(mcp): track MCP tool versions, changelogs and deprecations - #44245
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
|
6742c15 to
16c3c0a
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
16c3c0a to
9ef19ee
Compare
|
@greptileai please re-review at 9ef19ee (single commit, all four findings addressed) |
9ef19ee to
06f07ab
Compare
|
@greptileai please re-review at 06f07ab (same change as 9ef19ee plus prettier formatting of MCPToolVersionsPanel.tsx) |
06f07ab to
7dc6351
Compare
|
@greptileai please re-review at 7dc6351 (single commit). All seven findings from the 06f07ab review are addressed: pin from the panel forwards the tools viewer's x-mcp-* headers; deprecation read and update run in one writer transaction; the history test seam is a module-level helper instead of a class patch; new tests are typed with Final bindings and use mock call history for ordering; documentation stripping is recursive at schema positions only, so nested description/title edits are non-breaking while property names are never stripped |
7dc6351 to
b9f5bd9
Compare
|
@greptileai please re-review at b9f5bd9 (single commit). Addresses "Schema comparisons fail type checks": a typed |
b9f5bd9 to
bdfbae3
Compare
83bcc0d to
8012935
Compare
|
bugbot run |
8012935 to
19d0965
Compare
|
bugbot run |
19d0965 to
0091cc4
Compare
|
@greptileai please review the new head 0091cc4, rebased onto current main (the previous 5/5 on 19d0965 is no longer an ancestor). The rebase resolved conflicts with the catalog consistency change by keeping |
|
bugbot run |
0091cc4 to
5b95c5d
Compare
|
@greptileai please review the new head 5b95c5d. Since 0091cc4 the only change is |
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
5b95c5d to
c16b055
Compare
|
@greptileai please review the new head c16b055. Since 5b95c5d the only change is the test rewrite for the P2 on test style: |
|
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 c16b055. Configure here.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an admin re-pins a server whose upstream tools changed, and the old tool definitions are gone
After: the same re-pin records a new version and the page shows what changed and what is deprecated
Linear ticket
Resolves LIT-9138
Root cause
The pin path stored only the latest tool list in one JSON column on the server row and replaced it on every pin, so earlier definitions, what changed between them, and any deprecation metadata had nowhere to live.
Classification rules (
tool_versioning.py): description-only edits, new optional params, params becoming optional and param description edits are non-breaking; removed params, new required params, type or schema changes, newly required params (including names listed only inrequired), top-level schema changes and tool removal are breaking; top-level schemadescriptionandtitleedits are non-breaking. A tool that comes back after removal gets a new version marked "Tool restored", breaking if its schema also changed. Unchanged tools record nothing, and unpinning records nothing.Files changed
schema.prisma,litellm/proxy/schema.prisma,litellm-proxy-extras/litellm_proxy_extras/schema.prismaLiteLLM_MCPToolVersionmodel, unique on (server_id, tool_name, version)litellm-proxy-extras/.../20261002000000_mcp_tool_versions/migration.sqlCREATE TABLE/INDEX IF NOT EXISTSlitellm/types/mcp_server/mcp_server_manager.pyMCPToolVersion,MCPToolChange,MCPToolChangeKind, pin and deprecation request modelslitellm/proxy/_experimental/mcp_server/tool_versioning.pydescription/titleare ignored at every nested schema position (never as property names), so documentation-only edits are non-breakinglitellm/proxy/_experimental/mcp_server/db.pydeprecated_at; best-effort cleanup on server deletelitellm/integrations/otel/model/spans.pyLiteLLM_MCPToolVersionas a renderable Prisma span tablelitellm/repositories/table_repositories.pyMCPToolVersionRepositorylitellm/proxy/management_endpoints/mcp_management_endpoints.py{"changelog"}body;GET .../tool-versionsfiltered by the caller's key/team tool permissions; admin-onlyPUT/DELETE .../tools/{tool}/versions/{n}/deprecation; 409 on a version-number conflictlitellm/proxy/_lazy_openapi_snapshot.jsonui/litellm-dashboard/src/components/networking.tsxui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPToolVersionsPanel.tsxui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_tools.tsxcanManageVersionsis set, passing the samex-mcp-*headers the viewer uses for tools/listui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsxcanManageVersionsto the tools viewerui/litellm-dashboard/src/components/mcp_tools/types.tsxcanManageVersionsprop on the viewerui/litellm-dashboard/src/lib/http/schema.d.tsnpm run gen:apiPre-Submission checklist
Screenshots / Proof of Fix
Shared setup: real Postgres (
lit9138), proxy on :4000 withPYTHONPATHpointed at each checkout, a local HTTP MCP upstream on :8765 servingcatalog.json.catalog_v1.jsonhasget_forecast(city, days:int),get_alerts(region),get_radar(city).catalog_v2.jsonchangesdaysto string and adds requiredunitsonget_forecast, adds optionalseverityand a new description onget_alerts, and dropsget_radar. The same files are used on both sides. Build marker:tool_versioning.plan_tool_versionsis ABSENT on the base checkout and present on the head checkout.Before (merge base)
POST /v1/mcp/server/$S/pinreturns 200 with three toolsPOST /v1/mcp/server/$S/pin -d '{"changelog": "Weather API 2.0"}'returns 200 with two tools; v1 definitions are gone frompinned_toolsGET /v1/mcp/server/$S/tool-versionsreturns 404{"detail":"Not Found"}PUT /v1/mcp/server/$S/tools/get_forecast/versions/1/deprecation -d '{"sunset_date": "2026-12-31T00:00:00Z"}'returns 404After (first round, 6742c15)
{"changelog": "Weather API 2.0"}returns 200GET /v1/mcp/server/$S/tool-versionsreturns 200:PUT .../tools/get_forecast/versions/1/deprecation -d '{"sunset_date": "2026-12-31T00:00:00Z", "deprecation_note": "Use v2 with units"}'returns 200 withdeprecated_atset,sunset_date2026-12-31, the notedeprecated_atunchanged, sunset updated, omitted note clearedPUT .../versions/9/deprecation: 404MCP tool 'get_forecast' version 9 not found for server '...'Admin access required to update MCP tool version deprecation.get_radarv3non_breaking"Tool restored",get_forecastv3 breaking (type back to integer,unitsremoved)DELETE .../deprecation: all three deprecation fields nullget_forecasthistory, deprecated v3 with sunset 2027-01-15 (see the After screenshots under User Flow)Handler branches exercised: pin with no body, pin with changelog body, pin with unchanged catalog (no rows), pin with removed tool, pin with restored tool, concurrent pins (lock path), unpin; tool-versions GET as admin and as a key without server access; deprecation PUT as admin (new and existing), non-admin, missing version; deprecation DELETE. Not exercised live: a config-only server (
GETreturns[], covered by unit test) and the 409 version-conflict branch, which the lock now makes unreachable between pins (covered by unit test).Tests: classifier and planner rules, exact
create_manypayload with JSON-serializedinput_schema/changes, lock acquired before the history read and held until after the snapshot update, endpoint auth and 404/403/409 shapes, whitespace-only changelog becomes null, deprecation set/preserve/clear, cleanup failure does not fail server delete, dashboard badges, history, admin-only controls, loading/empty/error states, sunset payload${date}T00:00:00Z. Core tests were mutation-checked. The two removed assert lines intest_mcp_management_endpoints.pywere replaced with the same assertion pluschangelog=None.After (review round, 06f07ab)
Proxy restarted on the head checkout (
MARKER_plan_tool_versionspresent), same fixtures and server."required": ["token"]toget_radarwithout atokenproperty; pin returns 200 andget_radarv6 is recordedbreakingwithParameter "token" is now requiredget_alertsversionPUT .../tools/get_radar/versions/5/deprecationset, then edited from the UI:deprecated_atstays2026-10-02T21:14:46.307Z, note updatedGET .../tool-versionswith a virtual key whose MCP permissions allow onlyget_forecast: onlyget_forecastrows; master key: all three toolsTests added this round: pin runs lock, lookup, history read, insert and snapshot update on the transaction client in that order and never on the regular client, publish happens once after commit and not when the update fails; required-only parameter changes both ways; schema description and title changes; history filtered for allowed
["tool_a"],[]andNone; dashboard edit prefill, payload and non-admin hiding. Mutants killed: insert routed back through the regular client, publish moved inside the transaction, required-only names dropped from the comparison, history filter removed.After (review round 2, 7dc6351)
Same proxy and fixture, database
lit9138confirmed in the proxy log and/health/readinessbefore any write.filter.properties.term.descriptioninsideget_alertsand pinning recorded v8 asnon_breakingwith the single changeParameter "filter" description changedget_forecastv3: two PUTs returned 200 anddeprecated_atstayed2026-10-02T20:12:46.665000Zget_forecastgot 5 rows, allget_forecast; the master key got 17 rows across the three toolsget_alertsv9 after the fixture was restoredTests added this round: deprecation lookup and update both go through the transaction client; nested child description,
items.descriptionand$defsdescription edits are non-breaking while nested type and$defstype edits stay breaking, and removing a property literally namedtitleis breaking; pin forwards the viewer'sx-mcp-*headers. Mutants killed: deprecation lookup on the regular client, non-recursive documentation stripping, stripping keys inside apropertiesmap.After (review round 3, bdfbae3)
$schema-only change toget_radarpinned as v8breakingwithInput schema changed; restoring it pinned v9breakingget_radarv9 with different notes both returned 200 withdeprecated_at2026-10-02T22:29:51.931000Z, and history shows that one timestampTests added this round: a draft-only
$schemachange is breaking and planned as a new version; deprecation takes the per-server lock on the transaction before lookup and update. Mutant killed: deprecation without the lock.After (rebase onto main, c1508c1 and 5f05c78)
Fresh Postgres database
lit9138_head(all 200 migrations applied, confirmed in the proxy log), proxy on :4000 withPYTHONPATHat this checkout, same v1 and v2 catalogs served by a local HTTP MCP upstream on :8765{"changelog": "Weather API 2.0"}: 200, thenGET .../tool-versionsreturns 6 rows:get_forecastv2 breaking (daysinteger to string, new requiredunits),get_alertsv2 non_breaking (description, optionalseverity),get_radarv2 removed, plus three v1 initial rowsget_forecastv1 with sunset 2026-12-31: 200; editing the sunset to 2027-01-15 keepsdeprecated_atat2026-10-05T09:08:13.596000ZAdmin access required to update MCP tool version deprecation.The 5f05c78 delta only registers
LiteLLM_MCPToolVersionin the OTel Prisma span table list, which a test added on main requires for every Prisma model. The dashboard part of the diff is byte-identical to bdfbae3, so the UI screenshots above still applyAfter (review round 4, 31470b0)
Same proxy and database as the rebase run above, restarted on the fixed build
GET /v1/mcp/server/weather_head/tool-versions(the server's name) on 5f05c78: 200 with 0 rows, while the same server by id returned 62026-12-31T00:00:00) now shows as 2026-12-31 in Asia/Tokyo, both on the badge and in the edit prefill. Covered by a vitest that setsTZ=Asia/Tokyoand fails on the old formatterDELETE /v1/mcp/server/{id}on the fixed build: 202, the server's 6 history rows are gone, and its tool-versions route now returns 404. If removing the history fails, the server is kept rather than left with orphaned history a reused id could pick upAfter (review round 5, 2b37e3997b)
Same proxy and database, restarted on the fixed build, new server
weather_r5PUT /v1/mcp/server/weather_r5/tools/get_forecast/versions/1/deprecation(the server's name, not its id): 200, and the row stored under the server's id carries the note.DELETEon the same name path: 200 and the row has no deprecation leftAfter (review round 6, 83bcc0d)
New server
weather_r6on the same proxy and databasecreated_atwas moved forward on the database while the proxy still held the old row: history by name returned 0 rows instead of the newer server's history. Restoring the timestamp brought back the 6 rowsDELETE /v1/mcp/server/{id}: 202, and its tool-versions route then returns 404History is read on the writer under the server's pin lock and only if the server row there is still the one the caller was authorized against
After (review round 7, 19d0965)
New server
weather_r7on the same proxy and database, toolget_alertsversion 1, deprecation called by server namePUT .../deprecationwith a sunset date and note: 200, row holdsuse v2with a sunset date.DELETE: 200, row clearedcreated_atmoved forward on the database while the proxy still held the old row,PUTreturned 404 and the row stayed cleared.DELETEalso returned 404 and left a note written on the newer row in placePUTreturned 200 again. No tracebacks in the proxy logDeprecation writes now run the same writer side identity check as history reads, under the same pin lock
After (rebase onto main, 0091cc4)
Rebased onto main 2c67ae9. The PR's own delta from 19d0965 is two lines of kind:
_resolve_and_authorize_mcp_servernow runs under@catalog_operationlikefetch_mcp_serveron main, and the four new models extendLiteLLMBaseModellike the rest of the file on main. The dashboard diff is byte-identical to 19d0965, so the UI screenshots above still applySame local HTTP MCP upstream and catalogs in both arms, fresh Postgres per arm. Marker:
tool_versioning.pyABSENT on the base worktree, present on headBase (main 2c67ae9):
get_alerts,get_forecast,get_radar. Pin v2 with{"changelog": "Weather API 2.0"}: 200 with two toolsGET .../tool-versions: 404{"detail":"Not Found"}.PUT .../tools/get_forecast/versions/1/deprecation: 404Head (0091cc4):
get_alerts,get_forecastGET .../tool-versions: 200, 6 rowsget_forecastv2 breaking, changelogWeather API 2.0:Parameter "days" type changed from "integer" to "string"(breaking),Added required parameter "units"(breaking)get_alertsv2 non_breaking:Description changed,Added optional parameter "severity"get_radarv2 removed:Tool removed(breaking)initialwith no changelogPUT .../get_forecast/versions/1/deprecationsunset 2026-12-31 and note: 200 withdeprecated_atset. A second PUT with sunset 2027-01-15: samedeprecated_at, new sunset and noteMCP tool 'get_forecast' version 9 not found .... Non-admin key: 403Admin access required to update MCP tool version deprecation.DELETE .../deprecation: 200,deprecated_at,sunset_date,deprecation_noteall nullmcp_pin:{id}advisory lock, the request authorizes against the current server row and waits on the lock, psql moves the row'screated_atforward and commitsPUTwith notestale write: 404 after 3.0s, version row unchanged (Use v2 with units, sunset 2026-12-31)DELETE: 404 after 3.0s, the deprecation on the row is left in placePUT200 after 3.1s and the row holdscurrent write, sunset 2027-06-30Authorization on this head reads the server row from the database at request start, so moving
created_atbefore sending the request (the round 6 and 7 harness) is no longer stale: on head that request authorizes against the moved row and returns 200. The lock-held race above is what opens the window between authorization and the writeAfter (review round 8, 5b95c5d)
Greptile P1 on 0091cc4: the history route authorized inside a catalog snapshot but looked up the key's tool grants against the live catalog, so a rename between the two could drop a name-keyed restriction.
fetch_mcp_server_tool_versionsnow runs under@catalog_operationitself, like tool listing. The new test recordscatalog.current()at authorization and at the grant lookup and asserts the same snapshot; with the decorator removed the grant lookup seesNoneand the test failsProxy restarted on 5b95c5d, new server
weather_r8on the same databasedeprecated_at), version 9 404, non-admin 403, clear: same results as the 0091cc4 runget_forecaston this server: history 200 with 2 rows, allget_forecast. Master key: 6 rows across the three toolsPUT404 after 3.1s andDELETE404 after 3.0s with the version row unchanged, controlPUT200 after 3.1s writescurrent writeReview round 9, c16b055
Greptile P2 on 5b95c5d asked for
Finalbindings and typed callbacks in the new test. The test now records both snapshots throughFinalMagicMockrecorders and typed callbacks with the signatures ofauthorize_mcp_serverand_get_allowed_tool_names_for_server. Test-only change,git diff 5b95c5d102 c16b055209 -- . ':!tests'is empty, so the round 8 live run covers the production code on this head. Mutation check repeated: without the decorator the grant lookup recordsNoneand the test failsTaxonomy audit (A-BB)
@catalog_operationon_resolve_and_authorize_mcp_serveronly makes server lookup and theallow_catalog_viewregistry check read the same consistent catalog snapshot asfetch_mcp_server; a failed refresh surfaces as main's existing 503, never as wider access. History GET still filters withtool_is_granted(Noneunrestricted,[]nothing) and deprecation writes stay admin-only with the writer-side identity check. No new credential or header handlingNoneunrestricted,[]nothing); writes are admin-only with the existing 403 shapeOptional, noAny, no baredictin the diff_experimental/outbundle is left to the scheduled build-artifact job, as for other dashboard PRsMCPToolVersionfield has a column in all three schema files;prisma migrate diffagainst the migrated DB is emptyx-mcp-*headers the tools viewer already sends for tools/list, and the server reads them through the same extractor as tools/listdeprecated_atorfallback on the stored valueType
New Feature
Caveats (if any)
Medium
Low
Link to Devin session: https://app.devin.ai/sessions/b6d7b76b141849dab34e4f89f5f937f9
Open in Devin Desktop: https://app.devin.ai/desktop/session/b6d7b76b141849dab34e4f89f5f937f9?variant=devin
Requested by: @yassin-berriai
Note
Medium Risk
Changes the pin and delete code paths and adds new DB writes under advisory locks; impact is limited to MCP admin flows but mistakes could affect pinned catalogs and history consistency.
Overview
Adds version history for admin-pinned MCP tool catalogs: each pin can record per-tool versions with breaking/non-breaking change summaries, an optional changelog, and later deprecation metadata (sunset date and note).
Persistence and concurrency: New
LiteLLM_MCPToolVersiontable (migration + Prisma). Pinning, history reads, deprecation updates, and server delete all use a per-server Postgres advisory lock and writer transactions so version rows stay aligned withpinned_toolsand are removed with the server row.API:
POST .../pinaccepts optionalchangelog; newGET .../tool-versions(filtered by key/team tool grants); adminPUT/DELETE .../versions/{n}/deprecation. Concurrent version inserts surface as 409 on pin.Logic: New
tool_versioning.pydiffs pinned tool descriptions and JSON input schemas to planinitial/non_breaking/breaking/removedversions.Dashboard: Tool versions panel on the MCP Tools tab (history, pin with changelog, deprecate/edit/clear), reusing the viewer’s
x-mcp-*headers for pin.Reviewed by Cursor Bugbot for commit c16b055. Bugbot is set up for automated code reviews on this repo. Configure here.