scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-05 08:07:05 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 137

refactor(openapi-mcp): extract resolve_operation_params, add tests

Browse source 
- Hoist _resolve_ref and _resolve_param_list to module level in
  openapi_to_mcp_generator.py (were being redefined on every loop iteration)
- _resolve_ref now returns None for unresolvable $refs instead of
  the stub dict, preventing (None, None) from poisoning deduplication
- Add resolve_operation_params() as a shared helper that handles both
  $ref resolution and path-level param merging
- Replace duplicated inline logic in mcp_server_manager.py and
  rest_endpoints.py with calls to resolve_operation_params()
- Add TestResolveRef, TestResolveParamList, TestResolveOperationParams
  test classes covering $ref resolution, path-level merging, collision
  semantics, unresolvable ref filtering, and a GitHub-style spec fixture
This commit is contained in:
Ishaan Jaffer 2026-03-06 17:49:51 -08:00
parent af45006111
commit 02f74d5cea
4 changed files with 257 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -385,6 +385,7 @@ class MCPServerManager:
)
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
load_openapi_spec_async,
resolve_operation_params,
)
from litellm.proxy._experimental.mcp_server.tool_registry import (
global_mcp_tool_registry,
@ -450,32 +451,10 @@ class MCPServerManager:
operation = path_item[method]
# Merge path-level parameters into the operation (operation-level wins)
# and resolve any $ref parameters against components/parameters.
# Real-world specs (e.g. GitHub's) define shared params at the path level
# and use $ref instead of inline objects.
path_level_params = path_item.get("parameters", [])
op_level_params = operation.get("parameters", [])
def _resolve_ref(p: dict) -> dict:
ref = p.get("$ref", "")
if not ref.startswith("#/components/parameters/"):
return p
param_name = ref.split("/")[-1]
return components.get("parameters", {}).get(param_name, p)
# Build merged list: path-level first, then op-level overrides by name+in
resolved_path = [_resolve_ref(p) for p in path_level_params]
resolved_op = [_resolve_ref(p) for p in op_level_params]
op_keys = {(p.get("name"), p.get("in")) for p in resolved_op}
merged_params = [
p for p in resolved_path
if (p.get("name"), p.get("in")) not in op_keys
] + resolved_op
# Build a resolved copy of the operation for schema/function generation
resolved_operation = dict(operation)
resolved_operation["parameters"] = merged_params
# Resolve $ref params and merge path-level params into the operation.
resolved_operation = resolve_operation_params(
operation, path_item, components
)
# Generate tool name (without prefix initially)
operation_id = operation.get(

58
litellm/proxy/_experimental/mcp_server/openapi_to_mcp_generator.py
View file

@ -7,7 +7,7 @@ import contextvars
import json
import os
from pathlib import PurePosixPath
from typing import Any, Dict, Optional
from typing import Any, Dict, List, Optional
from urllib.parse import quote
from litellm._logging import verbose_logger
@ -115,6 +115,62 @@ def get_base_url(spec: Dict[str, Any], spec_path: Optional[str] = None) -> str:
return ""
def _resolve_ref(
param: Dict[str, Any], component_params: Dict[str, Any]
) -> Optional[Dict[str, Any]]:
"""Resolve a single parameter, following a $ref if present.
Returns the resolved param dict, or None if the $ref target is absent from
components (so callers can skip/filter it rather than propagating a stub
with name=None that would corrupt deduplication).
"""
ref = param.get("$ref", "")
if not ref.startswith("#/components/parameters/"):
return param
return component_params.get(ref.split("/")[-1])
def _resolve_param_list(
raw: List[Dict[str, Any]], component_params: Dict[str, Any]
) -> List[Dict[str, Any]]:
"""Resolve $refs in a parameter list, dropping any unresolvable entries."""
result = []
for p in raw:
resolved = _resolve_ref(p, component_params)
if resolved is not None and resolved.get("name"):
result.append(resolved)
return result
def resolve_operation_params(
operation: Dict[str, Any],
path_item: Dict[str, Any],
components: Dict[str, Any],
) -> Dict[str, Any]:
"""Return a copy of *operation* with fully-resolved, merged parameters.
Handles two common patterns in real-world OpenAPI specs:
1. **$ref parameters** — ``{"$ref": "#/components/parameters/per-page"}``
instead of inline objects. Each ref is resolved against
``components["parameters"]``; unresolvable refs are silently dropped so
they cannot corrupt the deduplication set with ``(None, None)`` keys.
2. **Path-level parameters** — params defined on the path item that apply
to every HTTP method on that path (e.g. ``owner``, ``repo``). They are
merged with the operation-level params; operation-level wins when the
same ``name`` + ``in`` combination appears in both.
"""
component_params = components.get("parameters", {})
path_level = _resolve_param_list(path_item.get("parameters", []), component_params)
op_level = _resolve_param_list(operation.get("parameters", []), component_params)
op_keys = {(p["name"], p.get("in")) for p in op_level}
merged = [p for p in path_level if (p["name"], p.get("in")) not in op_keys] + op_level
result = dict(operation)
result["parameters"] = merged
return result
def extract_parameters(operation: Dict[str, Any]) -> tuple:
"""Extract parameter names from OpenAPI operation."""
path_params = []

18
litellm/proxy/_experimental/mcp_server/rest_endpoints.py
View file

@ -666,6 +666,7 @@ if MCP_AVAILABLE:
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
build_input_schema,
load_openapi_spec_async,
resolve_operation_params,
)
try:
@ -679,22 +680,7 @@ if MCP_AVAILABLE:
if operation is None:
continue
# Resolve $ref params and merge path-level params (same logic as
# _register_openapi_tools) so large specs like GitHub's work correctly.
def _resolve_ref(p: dict) -> dict:
ref = p.get("$ref", "")
if ref.startswith("#/components/parameters/"):
param_name = ref.split("/")[-1]
return components.get("parameters", {}).get(param_name, p)
return p
path_level = [_resolve_ref(p) for p in path_item.get("parameters", [])]
op_level = [_resolve_ref(p) for p in operation.get("parameters", [])]
op_keys = {(p.get("name"), p.get("in")) for p in op_level}
merged = [p for p in path_level if (p.get("name"), p.get("in")) not in op_keys] + op_level
resolved_op = dict(operation)
resolved_op["parameters"] = merged
resolved_op = resolve_operation_params(operation, path_item, components)
op_id = operation.get("operationId", f"{method}_{path}")
summary = operation.get("summary", "")

194
tests/test_litellm/proxy/_experimental/mcp_server/test_openapi_to_mcp_generator.py
View file

@ -15,10 +15,13 @@ from unittest.mock import AsyncMock, patch
import pytest
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
_resolve_param_list,
_resolve_ref,
build_input_schema,
create_tool_function,
extract_parameters,
get_base_url,
resolve_operation_params,
)
GET_ASYNC_CLIENT_TARGET = (
@ -660,6 +663,195 @@ class TestGetBaseUrl:
"paths": {}
}
spec_path = "https://api.example.com/v2/docs/api/openapi.json"
base_url = get_base_url(spec, spec_path)
assert base_url == "https://api.example.com/v2/docs/api"
class TestResolveRef:
"""Test $ref resolution for individual parameters."""
def test_inline_param_returned_unchanged(self):
"""Inline params (no $ref) are returned as-is."""
param = {"name": "owner", "in": "path", "required": True}
component_params: dict = {}
result = _resolve_ref(param, component_params)
assert result is param
def test_ref_resolved_from_components(self):
"""A $ref pointing at components/parameters is resolved correctly."""
param = {"$ref": "#/components/parameters/per-page"}
component_params = {
"per-page": {"name": "per_page", "in": "query", "schema": {"type": "integer"}}
}
result = _resolve_ref(param, component_params)
assert result == {"name": "per_page", "in": "query", "schema": {"type": "integer"}}
def test_unresolvable_ref_returns_none(self):
"""A $ref whose target is absent from components returns None (not the stub)."""
param = {"$ref": "#/components/parameters/missing-param"}
component_params: dict = {}
result = _resolve_ref(param, component_params)
assert result is None
def test_non_component_ref_returned_unchanged(self):
"""A $ref that doesn't start with #/components/parameters/ is returned as-is."""
param = {"$ref": "#/definitions/SomeModel"}
result = _resolve_ref(param, {})
assert result is param
class TestResolveParamList:
"""Test batch $ref resolution with filtering."""
def test_all_inline_params_preserved(self):
"""All inline params with names are preserved."""
raw = [
{"name": "owner", "in": "path"},
{"name": "repo", "in": "path"},
]
result = _resolve_param_list(raw, {})
assert len(result) == 2
assert result[0]["name"] == "owner"
assert result[1]["name"] == "repo"
def test_refs_resolved(self):
"""$ref entries are resolved against component_params."""
raw = [
{"$ref": "#/components/parameters/per-page"},
{"name": "q", "in": "query"},
]
component_params = {
"per-page": {"name": "per_page", "in": "query", "schema": {"type": "integer"}}
}
result = _resolve_param_list(raw, component_params)
assert len(result) == 2
assert result[0]["name"] == "per_page"
assert result[1]["name"] == "q"
def test_unresolvable_refs_dropped(self):
"""Unresolvable $refs are silently dropped — no None or nameless entries."""
raw = [
{"$ref": "#/components/parameters/does-not-exist"},
{"name": "q", "in": "query"},
]
result = _resolve_param_list(raw, {})
assert len(result) == 1
assert result[0]["name"] == "q"
def test_nameless_entries_dropped(self):
"""Entries that resolve to dicts without a 'name' key are dropped."""
raw = [{"schema": {"type": "string"}}] # no "name" field
result = _resolve_param_list(raw, {})
assert result == []
class TestResolveOperationParams:
"""Test the shared resolve_operation_params helper."""
def test_inline_params_unchanged(self):
"""Simple inline operation with no $refs or path-level params is returned unchanged."""
operation = {
"operationId": "listRepos",
"parameters": [
{"name": "owner", "in": "path", "required": True},
{"name": "sort", "in": "query"},
],
}
path_item = {"get": operation}
result = resolve_operation_params(operation, path_item, {})
names = [p["name"] for p in result["parameters"]]
assert names == ["owner", "sort"]
def test_ref_params_resolved(self):
"""$ref parameters in the operation are resolved from components."""
operation = {
"parameters": [
{"$ref": "#/components/parameters/per-page"},
{"name": "q", "in": "query"},
]
}
path_item = {"get": operation}
components = {
"parameters": {
"per-page": {"name": "per_page", "in": "query", "schema": {"type": "integer"}}
}
}
result = resolve_operation_params(operation, path_item, components)
names = [p["name"] for p in result["parameters"]]
assert "per_page" in names
assert "q" in names
def test_path_level_params_merged(self):
"""Path-level parameters are merged into the operation parameters."""
path_level_params = [
{"name": "owner", "in": "path", "required": True},
{"name": "repo", "in": "path", "required": True},
]
operation = {
"parameters": [{"name": "sort", "in": "query"}]
}
path_item = {"parameters": path_level_params, "get": operation}
result = resolve_operation_params(operation, path_item, {})
names = [p["name"] for p in result["parameters"]]
assert "owner" in names
assert "repo" in names
assert "sort" in names
def test_operation_level_wins_on_collision(self):
"""When path-level and operation-level define the same name+in, operation wins."""
path_level_params = [
{"name": "per_page", "in": "query", "schema": {"type": "integer"}, "default": 30}
]
operation = {
"parameters": [
{"name": "per_page", "in": "query", "schema": {"type": "integer"}, "default": 100}
]
}
path_item = {"parameters": path_level_params, "get": operation}
result = resolve_operation_params(operation, path_item, {})
per_page_params = [p for p in result["parameters"] if p["name"] == "per_page"]
assert len(per_page_params) == 1
assert per_page_params[0].get("default") == 100 # operation-level value
def test_unresolvable_refs_silently_dropped(self):
"""Unresolvable $refs are dropped — they don't poison the result with (None, None) keys."""
operation = {
"parameters": [
{"$ref": "#/components/parameters/nonexistent"},
{"name": "q", "in": "query"},
]
}
path_item = {"get": operation}
result = resolve_operation_params(operation, path_item, {})
names = [p["name"] for p in result["parameters"]]
assert names == ["q"]
# Verify no None entries slipped through
assert all(p.get("name") is not None for p in result["parameters"])
def test_github_style_spec_structure(self):
"""Simulate a GitHub-style spec: path-level owner+repo refs, operation-level query params."""
component_params = {
"owner": {"name": "owner", "in": "path", "required": True, "schema": {"type": "string"}},
"repo": {"name": "repo", "in": "path", "required": True, "schema": {"type": "string"}},
"per-page": {"name": "per_page", "in": "query", "schema": {"type": "integer"}},
}
path_level_params = [
{"$ref": "#/components/parameters/owner"},
{"$ref": "#/components/parameters/repo"},
]
operation = {
"operationId": "repos/list-commits",
"parameters": [
{"$ref": "#/components/parameters/per-page"},
{"name": "sha", "in": "query"},
],
}
path_item = {"parameters": path_level_params, "get": operation}
result = resolve_operation_params(operation, path_item, {"parameters": component_params})
names = [p["name"] for p in result["parameters"]]
assert "owner" in names
assert "repo" in names
assert "per_page" in names
assert "sha" in names
assert len(names) == 4 # no duplicates