proto(mcp): per-user env-var fields demo flow (UI mockup) - #28399
Closed
mateo-berri wants to merge 18 commits into
Closed
proto(mcp): per-user env-var fields demo flow (UI mockup)#28399mateo-berri wants to merge 18 commits into
mateo-berri wants to merge 18 commits into
Conversation
* feat(gemini): add gemini-3.1-flash-lite model cost map entries Co-authored-by: Cursor <cursoragent@cursor.com> * Update model_prices_and_context_window.json * Update source URL for model pricing information * Sync source URL for gemini-3.1-flash-lite in backup JSON * fix(model_cost_map): add mistral/ministral-8b-2512 entry Mistral rotated the 'mistral/mistral-tiny' alias to return 'ministral-8b-2512' as the response model, which is not in the cost map. This caused test_completion_mistral_api and test_completion_mistral_api_modified_input to fail in completion_cost lookup. Add the entry mirroring the existing openrouter/mistralai/ministral-8b-2512 pricing. * test(cost_calculator): assert output_cost_per_reasoning_token for gemini-3.1-flash-lite * fix(tests): backfill local backup entries into runtime model_cost litellm.model_cost is loaded from LITELLM_MODEL_COST_MAP_URL (pinned to main) at import time, so any pricing entries added to the in-tree backup on this branch aren't visible at test runtime until they also land on main. The Mistral cassette currently returns model=ministral-8b-2512 and the cost-calculator lookup in test_completion_mistral_api / test_completion_mistral_api_modified_input fails despite the entry existing in the local backup. Backfill missing backup entries into litellm.model_cost in the local_testing conftest so these lookups succeed against the cassette state the branch is being tested with. * fix(tests): guard conftest backfill against empty local cost map --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
…d double-seed (#27854) * fix(spend_counter): seed Redis counter via SET NX to prevent cross-pod double-seed Symptom ------- Customers on multi-pod deployments see team `spend` jump to ~2x (or N x the pod count) shortly after a Redis cache miss / TTL expiry, triggering spurious "Budget Crossed" alerts and blocked requests until the value is manually reset. Root cause ---------- `SpendCounterReseed.coalesced` warmed the primary spend counter by calling `redis.async_increment(key, value=db_spend, refresh_ttl=True)`, which lowers to Redis `INCRBYFLOAT`. That is additive, not idempotent. The per-counter `asyncio.Lock` only coalesces seeders inside one process. With N pods sharing one Redis, on a cold key (cold start, TTL expiry, manual delete) every pod independently passes its lock + Redis re-check, reads the same `db_spend`, and issues `INCRBYFLOAT db_spend`. Final value: N x db_spend. Fix --- Use `redis.async_set_cache(key, value=db_spend, nx=True)` for the seed. SET NX is atomic across pods: exactly one writer initializes the key; losers read the winner's value via `async_get_cache`. This is the same idiom already used by `coalesced_window` in the same file, so the two seed paths are now consistent. Per-request deltas continue to use `INCRBYFLOAT` (correct - additive behaviour is what we want for increments, not for initial seed). Verification ------------ Live two-process repro against the same Postgres + Redis (DB spend = 506): Unpatched: 4/4 runs -> Redis counter = ~1012 (~2 x db_spend) Patched: 12/12 runs -> Redis counter = ~506 Unit tests (`test_proxy_server.py`): - New `test_primary_spend_counter_redis_concurrent_seed_does_not_double_seed` patches `_get_lock` to return a fresh lock per caller (otherwise the per-process lock masks the race), races two `coalesced` calls, and asserts final = 506 with exactly one of two SET NX attempts winning. - 4 existing tests updated for the new seed contract (SET NX for the seed, INCRBYFLOAT only for the per-request delta). - Full `spend_counter or reseed or budget` slice: 22 passed. Co-authored-by: Cursor <cursoragent@cursor.com> * test(spend_counter): make SET NX mock atomic so loser branch is exercised Greptile flagged that `redis_set_cache` in test_primary_spend_counter_redis_concurrent_seed_does_not_double_seed placed `await asyncio.sleep(0)` AFTER the NX membership check. Both concurrent tasks observed an empty `redis_store`, passed the guard, and both returned True - so the loser branch (else: read back winner's value) was never exercised. Fix the mock to model real atomic Redis SET NX: - Yield BEFORE the membership check so two concurrent callers interleave the way real SET NX does (first to resume runs check + write atomically and wins; second resumes after the key exists and loses). - Track set_cache return values; assert sorted([loser, winner]) so we know exactly one task wins and one loses. - Track async_get_cache calls that happen AFTER at least one SET NX has completed; assert at least one such read - that is the loser-path fallback (`current_value = float(cached)` when seeded is False). Verified by temporarily reverting the mock to the old order: the test now fails with `expected exactly one SET NX winner and one loser, got [True, True]`, exactly the failure mode Greptile described. No production code change. Co-authored-by: Cursor <cursoragent@cursor.com> * test(spend_counter): mock async_set_cache to populate redis_store in concurrent read+write test `test_concurrent_read_and_write_paths_share_one_db_query` mocks `async_increment` to populate the in-memory `redis_store`, but did not mock `async_set_cache`. After the SET-NX seed change in `coalesced()`, the seed step writes via `async_set_cache(nx=True)` (default AsyncMock, no `redis_store` write), so the simulated Redis stays empty after the first reseed. The second `get_current_spend` then sees a clean Redis miss, re-enters the DB read path, and the test fails with `expected 1 DB query, got 2`. Fix: add a `redis_set_cache` side_effect that updates `redis_store` on `nx=True` (and rejects when the key already exists), matching the pattern used by the four sibling tests fixed in this branch's first commit. Pre-existing assertions are unchanged. Full `tests/test_litellm/proxy/test_proxy_server.py`: 158 passed. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…28339) * fix(proxy): normalize batch file IDs before ManagedObjectTable write Run post_call_success_hook before update_batch_in_database on retrieve/cancel, and ensure_batch_response_managed_file_ids so file_object never stores raw provider output_file_id or error_file_id. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(proxy): address Greptile review on batch file ID normalization Remove redundant resolve_* calls after update_batch_in_database and rename loop variable to avoid shadowing hidden_params unified_file_id. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tests): add mistral/ministral-8b-2512 to cost map and backfill in conftest Mistral rotated the 'mistral/mistral-tiny' alias to return 'ministral-8b-2512' as the response model, which was missing from the cost map. This caused test_completion_mistral_api and test_completion_mistral_api_modified_input to fail in litellm.completion_cost lookup. - Add mistral/ministral-8b-2512 entry to both the in-tree model_prices_and_context_window.json and the bundled litellm/model_prices_and_context_window_backup.json (mirrors the existing openrouter/mistralai/ministral-8b-2512 pricing). - litellm.model_cost is loaded at import time from the URL pinned to main, so the new backup entry isn't visible at test runtime until it also lands on main. Backfill any entries missing from the remote-fetched map into litellm.model_cost in the local_testing conftest so cost-calculator lookups succeed on this branch. * fix(tests): drop unnecessary del of conftest backfill loop vars * fix: resolve batch response file IDs even when status unchanged The status-unchanged early return in update_batch_in_database was skipping ensure_batch_response_managed_file_ids, leaving raw provider input_file_id (and other raw IDs) in the user-facing response when polling an in-progress batch. Move the in-place file ID normalization above the early return so the response always carries unified managed IDs while still skipping the DB write when nothing changed. Co-authored-by: Yassin Kortam <yassin@berri.ai> * test(batches): cover ensure_batch_response_managed_file_ids branches Add tests for the previously-uncovered paths in ensure_batch_response_managed_file_ids: error_file_id normalization, swallowed conversion errors, UserAPIKeyAuth fallback from db_batch_object, model_name resolution from unified_file_id, and early returns when managed_files_obj, model_id, or auth context are missing. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Yassin Kortam <yassin@berri.ai> Co-authored-by: Claude <noreply@anthropic.com>
…27921) * fix(router): use forwarded model_id for native Azure container IDs in _init_containers_api_endpoints Azure code-interpreter containers return provider-native IDs (cntr_ + hex) that carry no LiteLLM routing payload, so _decode_container_id returns model_id=None. The router was falling through to call the handler directly, bypassing _ageneric_api_call_with_fallbacks and leaving api_base=None for Azure deployments. Fall back to the model_id forwarded from the proxy ownership check so deployment credentials are always applied. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(azure-containers): strip /openai/responses path from api_base in AzureContainerConfig.get_complete_url When a deployment's api_base is the responses endpoint URL (e.g. .../openai/responses?api-version=...), AzureContainerConfig was appending /openai/containers on top of it, producing the broken path .../openai/responses/openai/containers. Azure returns 404 for that URL while the correct path is .../openai/containers. Strip any /openai/responses suffix from api_base before constructing the containers URL so the resource root is always used as the starting point. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(azure-containers): prefer api-version from api_base URL over deployment's api_version The deployment's api_version (e.g. 2024-08-01-preview) targets the chat/responses API and is too old for the containers API, which requires 2025-04-01-preview. The responses endpoint api_base already carries the correct api-version in its query string. Extract it and use it for the containers URL, overriding the stale deployment-level version. Fixes DELETE and file-upload operations returning 404 due to wrong api-version. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(containers): pass params=None instead of params={} to httpx to preserve api-version httpx erases a URL's query-string when params={} (empty dict) is passed, silently stripping ?api-version=2025-04-01-preview from every container POST/DELETE request. Azure's GET endpoints tolerate a missing api-version; POST (upload) and DELETE are strict, so those returned 404. Fix: use `params or None` in container_handler._async_handle and llm_http_handler.async_container_delete_handler (and all sibling container handlers) so that an empty params dict falls back to None, leaving httpx to preserve the URL's existing query string intact. Adds a regression test that directly documents the httpx behaviour. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(router): remove elif model_id branch from _init_containers_api_endpoints Two reviewer findings addressed: 1. Truncated comment on the model_id fallback line — now complete. 2. Security: the elif branch that fired when container_id was absent allowed any authenticated caller to supply model_id in a POST /v1/containers body and route the request through an arbitrary deployment UUID, bypassing the model-level access checks that only validate `model`. Removed the elif branch; operations without container_id (create, list) route by the caller-supplied `model` field as before. model_id forwarding is kept only inside the container_id block, where the proxy ownership check has already validated the container before forwarding the deployment ID. Adds a regression test pinning the security boundary: no-container-id path calls original_function directly even when model_id is in kwargs. Co-authored-by: Cursor <cursoragent@cursor.com> * test(containers): validate proxy-to-router model_id forwarding for managed IDs Add test_regression_get_container_forwarding_params_sets_model_id_for_managed_id to verify that get_container_forwarding_params (the proxy-side half of the Azure routing fix) correctly extracts and forwards model_id from a LiteLLM-managed encoded container ID. This closes the gap identified by Greptile P1: the previous regression test only injected model_id as a direct kwarg, validating the router in isolation. The new test exercises the actual proxy-to-router data flow through ownership.get_container_forwarding_params, confirming that kwargs["model_id"] is populated before _init_containers_api_endpoints is reached. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(azure-containers): tighten endpoint-path strip to endswith match Use path.endswith() instead of path.find() for _AZURE_ENDPOINT_PATHS so the suffix strip only fires when api_base actually ends with one of the endpoint-specific path suffixes. This is the more precise check greptile flagged on the original find()-based implementation. * Fix sync container handler to preserve URL query string Mirror the async path fix: pass None instead of an empty params dict so httpx does not strip the URL's existing query string (e.g. ?api-version=...), which is required for Azure container routing. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(azure-containers): strip trailing slash before endpoint suffix match Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(containers): recover model_id from stored encoded id for native Azure container IDs get_container_forwarding_params previously only set model_id when the user-supplied container_id was a LiteLLM-managed encoded id. For native upstream IDs (e.g. Azure 'cntr_<hex>') the decode fails and model_id was never forwarded — making the router-side fallback in _init_containers_api_endpoints unreachable in production. Fall back to the stored 'unified_object_id' on the ownership row, which is the encoded form captured at create time when the router selected a specific deployment. Decoding that yields the deployment model_id and restores router-based credential application (api_base, api_key) for retrieve/delete and container-file operations on native IDs. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Yassin Kortam <yassin@berri.ai>
When a new filter is applied to spend logs, React Query's keepPreviousData left stale rows on screen for 10–15s with no indication that a fetch was in progress. The previous custom isFilteringResults flag was removed in the #25847 toolbar refactor and only partially restored on the Fetch button. Use React Query's isPlaceholderData to discriminate a real filter change (queryKey changed, data not yet arrived) from a same-key live-tail refetch, and feed it into the existing isLoading prop on the toolbar pagination text and the table body. Live-tail polls still keep previous rows without flicker. Co-authored-by: Ryan <ryan@Ryans-MBP.localdomain>
* chore(e2e): migrate runner to uv, add All Proxy Models key test Switches the local e2e runner (run_e2e.sh) from poetry to uv to match the rest of the repo and CI. Adds a Playwright test for creating an admin key with no team selected (all-proxy-models flow), a SLOWMO env hook for headed debugging, and a MIGRATION_TRACKING.md doc that maps the manual UI QA checklist to e2e tests so future migration work has a single source of truth. * chore(e2e): address greptile feedback - Remove MIGRATION_TRACKING.md (docs belong in litellm-docs repo) - playwright.config.ts: fall back to 0 when SLOWMO is non-numeric (parseInt returns NaN, which Playwright accepts silently) - run_e2e.sh: add --frozen to uv sync for CI determinism
* feat(ui): team allowed_passthrough_routes create parity + edit load fix Add the Allowed Pass Through Routes selector to the create-team modal (previously only on the edit form), and fix the edit form silently dropping the field: it lives under team metadata, so initialValues must read info.metadata.allowed_passthrough_routes — otherwise the selector renders empty and saving wipes admin-set routes. Both selectors are gated to premium proxy admins, mirroring the server-side gate. Resolves LIT-3019 * fix(ui): persist team allowed_passthrough_routes edits on save The edit form loaded the selector but the save path never wrote it back: allowed_passthrough_routes stayed in the raw metadata JSON textarea and parsedMetadata (from that textarea) always won, so selector edits were silently discarded. Strip it from the textarea initialValues and overlay values.allowed_passthrough_routes into updateData.metadata, mirroring how guardrails is handled. Resolves LIT-3019 * fix(ui): preserve team passthrough routes for non-proxy-admins on save Only proxy admins may set allowed_passthrough_routes (server-side gate). For non-proxy-admins, write the team's stored value back into metadata instead of the form value, so saving an unrelated setting can't silently wipe routes; omit the key entirely when the team never had any. Resolves LIT-3019
…8227) * fix(mcp): JWT on tools/list, REST server_id resolution, tool_server_mismatch Sign outbound MCP JWTs for list_mcp_tools and inject headers on the tools/list path. Resolve server_id on /mcp-rest/tools/call and return 403 tool_server_mismatch when the tool does not belong to the requested server. Default missing arguments to {}. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): restrict list JWTs to mcp:tools/list and default REST arguments to {} - List-only JWTs (call_type=list_mcp_tools) no longer carry the broad mcp:tools/call scope. _build_scope() now emits only mcp:tools/list when no tool name is provided, mirroring the existing least-privilege rule that tool-call JWTs omit mcp:tools/list. - REST /tools/call now defaults a missing 'arguments' field to {} so execute_mcp_tool() and downstream **arguments / .keys() calls don't receive None and crash with TypeError/AttributeError. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(mcp): validate tool/server in call_tool; skip JWT signer when not configured or static auth present Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(mcp): align tests and mypy with user_api_key_auth on tools/list Update mocks for the new _get_tools_from_server parameter, mock server registry in REST access-denied test, and narrow static_headers for mypy. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(test): accept user_api_key_auth in get_tools_from_mcp_servers mock The side_effect for the all-servers case did not accept the new kwarg, so tools/list returned an empty list. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): fail fast for unknown tools when server mapping exists Server-name fallback in call_tool must not open an upstream session when the tool is absent from a populated mapping. Update the HTTP transport test to register a known tool before asserting not-found behavior. Co-authored-by: Cursor <cursoragent@cursor.com> * fix mypy * Fix mypy * fix(mcp): preserve tools/call scope on missing tool name; pass user_api_key_auth in list_tools Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(mcp): match alias/server_name in _resolve_mcp_server_for_tool_call The registry lookup in _resolve_mcp_server_for_tool_call previously only compared candidate.name against the provided server_name, but tool name prefixes can be derived from a server's alias or server_name (see get_server_prefix). When the tool→server mapping is empty/stale (cold start, dynamic tools), the lookup would fail for alias-configured servers even though get_mcp_server_by_name (used by the REST path) matches alias, server_name, and name. Match the same priority of identifiers in both the registry pass and the unprefixed fallback so the MCP protocol call_tool path is consistent with the REST path. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(mcp): reuse proxy_logging DualCache in inject_mcp_jwt_headers_for_upstream Instead of allocating a fresh DualCache() on every tools/list invocation, prefer the shared proxy_logging_obj.internal_usage_cache.dual_cache when available. The cache argument is currently unused by MCPJWTSigner, but sharing the proxy's cache avoids per-call allocation overhead and matches the cache identity used elsewhere in the proxy hook plumbing — so any future per-request state stored in cache will survive across list calls. Co-authored-by: Claude <noreply@anthropic.com> * fix(mcp): return 403 ip_filtering for IP-restricted servers in tools/call name lookup Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(test): accept user_api_key_auth kwarg in list_tools mocks The proxy-infra job was failing on four TestMCPServerManager tests because the mock_get_tools_from_server stubs did not accept the new user_api_key_auth keyword argument that list_tools now forwards to _get_tools_from_server. Add the kwarg to each stub so list_tools can call through cleanly. Co-authored-by: Claude <claude@anthropic.com> * fix(mcp): skip JWT injection when per-user mcp_auth_header is set MCPClient._get_auth_headers() applies extra_headers AFTER writing Authorization from auth_value, so an injected JWT silently overwrites the user's per-server OAuth token. Guard the JWT signer with 'not mcp_auth_header' so per-user OAuth (and any dict-form per-user auth) takes precedence, mirroring the existing static_headers guard. Adds a regression test that the signer's inject helper is not called when mcp_auth_header is supplied. * fix(mcp): skip JWT injection when extra_headers already has Authorization When a server uses per-user OAuth tokens, the resolved token is passed into _get_tools_from_server via extra_headers. The JWT injection guard only checked mcp_auth_header and the server's static headers, so the signer would silently overwrite the user's OAuth Authorization header. Add a check for an existing Authorization entry in extra_headers so caller-supplied per-user OAuth tokens take precedence over JWT signing. Co-authored-by: Yassin Kortam <yassin@berri.ai> * test(mcp): cover JWT signer + tool-call resolution branches Adds unit tests for the new MCPServerManager helpers (_resolve_mcp_server_for_tool_call, _resolve_oauth2_headers_for_tool_call) and the new MCPJWTSigner paths (_build_scope call_type branches and inject_mcp_jwt_headers_for_upstream). Brings patch coverage above the auto target without changing behavior. Co-authored-by: Claude <claude@anthropic.com> * fix(mcp): retry tool-server lookup with prefixed name in REST mismatch check When the REST /mcp-rest/tools/call path sends a raw tool name plus requested_server_id, _get_mcp_server_from_tool_name(name) can return None if the mapping only stores the prefixed form. That bypassed the tool_server_mismatch 403 guard and let the call fall through to trusting requested_server. Retry the lookup with every known prefix of the requested server so the mismatch check fires whenever the tool is actually registered. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(mcp): always reject unknown tools in server-name fallback Defense-in-depth: _resolve_mcp_server_for_tool_call previously skipped the unknown-tool check whenever the per-server mapping had no entries yet (cold start, OAuth2 lazy listing, or upstream listing failure), allowing arbitrary tool names to reach upstream servers. Tighten the check so the server-name fallback always rejects tool names not present in the mapping. Callers must call list_tools first (standard MCP flow) before tools/call can resolve. Removes the now-unused _mapping_has_tools_for_server helper and adds an explicit empty-mapping rejection test alongside the existing populated-mapping rejection test. Co-authored-by: Sameer Kankute <sameer@berri.ai> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Yassin Kortam <yassin@berri.ai> Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Claude (greptile subagent) <claude-greptile-bot@anthropic.com>
…May 2026) (#28153) * feat(interactions): migrate to Google Interactions API steps schema (May 2026) Default to Api-Revision: 2026-05-20 (new `steps` schema). Add `litellm.use_legacy_interactions_schema` global flag that sends Api-Revision: 2026-05-07 for operators who need the legacy `outputs` schema until June 8, 2026. - Inject Api-Revision header in GoogleAIStudioInteractionsConfig.validate_environment() - Auto-coalesce response_mime_type → response_format and image_config migration on new schema - Add steps field to InteractionsAPIResponse and InteractionsAPIStreamingResponse - Add StepStart/StepDelta/StepStop/InteractionCreated/etc. SSE event types - Update streaming completion detection to handle interaction.completed event - Bridge transformer populates both outputs and steps fields - Bridge streaming iterator emits new-schema events by default Co-authored-by: Cursor <cursoragent@cursor.com> * fix(interactions): address greptile review feedback - Avoid mutating caller's generation_config dict by shallow-copying before popping image_config, preventing silent failures on retries - Skip schema key in response_format when response_format is None to avoid sending schema: null to the Google Interactions API - Remove delta field from step.stop events (new schema only); the StepStop model has no delta field and sending it duplicates already- streamed text and breaks spec-conformant clients Co-authored-by: Cursor <cursoragent@cursor.com> * fix(proxy): parse use_legacy_interactions_schema string values safely bool("false") returns True in Python, so quoted YAML values like "false" or "False" silently activated the legacy Interactions API schema. Match the env-var parsing pattern in litellm/__init__.py by treating string inputs as true only when they equal "true" (case insensitive). Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix(interactions): only set object/id/delta on step.stop for legacy schema StepStop (new schema) has no object, id, or delta fields. Setting them unconditionally caused spec-breaking extra fields on new-schema step.stop events in all four construction sites (sync/async × main-loop/StopIteration). Legacy content.stop still receives id, object, and delta unchanged. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(interactions): stabilize streaming bridge schema, dict aliasing, and lost first delta - Capture use_legacy_interactions_schema once at iterator construction so all events emitted by a single stream use a consistent schema, even if the global flag is mutated mid-stream. - Check for the buffered interaction.complete/completed event before the finished check in __next__/__anext__ so the final completion event (which carries the full collected text in steps) is not dropped after self.finished is set. - Copy text content entries before appending to both outputs and the steps content list to avoid shared mutable dict aliasing between the two response fields. Co-authored-by: Yassin Kortam <yassin@berri.ai> * fix tests * fix greptile review * fix(interactions): address Greptile P1 review on schema coalescing and legacy deltas Skip response_mime_type merge when response_format is already a list, avoid in-place list mutation on image_config append, and restore delta.type on legacy content.delta events. Co-authored-by: Cursor <cursoragent@cursor.com> * style(interactions): black-format gemini transformation.py Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Yassin Kortam <yassin@berri.ai> Co-authored-by: Claude <noreply@anthropic.com>
* test(ui-e2e): add admin key creation with a specific proxy model Adds Playwright coverage for creating a key (no team) scoped to a single proxy model, complementing the existing All-Proxy-Models test. Uses a DOM-dispatched click on the antd dropdown option since the popup animation can render the option outside the viewport. * test(ui-e2e): verify scoped key works against mock /chat/completions Extend the "Create a key with a specific proxy model" test to extract the new key from the success modal and POST to /chat/completions for the scoped model, asserting 200 and the mock response body. Without this the test could pass even if the model selection failed to register.
Throwaway UI mockup so we can show the customer the end-to-end flow for
per-user MCP credentials before committing to a real implementation.
Flow demoed:
1. Admin adds env vars (3 columns: name, value, scope=Global|Per-user) in
"Add MCP Server"; values can be interpolated in Static Headers as
${VAR_NAME}.
2. User's dashboard cell turns red with "N user fields missing" when their
per-user fields are unset.
3. "Try in Claude Code" preview shows a friendly error naming the missing
fields plus a deep link back to the fill modal.
4. Fill modal saves, badge flips to green Ready, preview now shows
Connected.
Mock state lives entirely in localStorage keyed by (server alias, user id);
no backend wiring. All new code is under components/mcp_tools/mock/ so it
can be torn out once we know what the real shape should be.
|
|
Contributor
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| fillFieldsServer.alias || fillFieldsServer.server_name || "" | ||
| } | ||
| serverName={fillFieldsServer.server_name} | ||
| userId={userID || ""} |
| mockDemoServer.alias || mockDemoServer.server_name || "" | ||
| } | ||
| serverName={mockDemoServer.server_name} | ||
| userId={userID || ""} |
| import { Button } from "@tremor/react"; | ||
| import { getMissingUserFields } from "./mockMcpEnvVars"; | ||
|
|
||
| const { Text, Title, Paragraph } = Typography; |
| onOpenFill, | ||
| onOpenDemo, | ||
| }) => { | ||
| const [tick, setTick] = useState(0); |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| call_type, | ||
| ) | ||
|
|
||
| return data |
| route_all_chat_openai_to_responses: bool = ( | ||
| os.getenv("LITELLM_ROUTE_ALL_CHAT_OPENAI_TO_RESPONSES", "false").lower() == "true" | ||
| ) # When True, routes all OpenAI /chat/completions requests through the Responses API bridge | ||
| use_legacy_interactions_schema: bool = ( |
Comment on lines
+2903
to
+2905
| from litellm.proxy._experimental.mcp_server.server import ( # noqa: PLC0415 | ||
| _get_user_oauth_extra_headers_from_db, | ||
| ) |
- Replace antd Space with flex div in EnvVarsSection for more reliable column alignment between header and rows - Use explicit flex styles on name/value Form.Items so columns line up with the Scope and remove-button columns - Shorten "Per-user field" option label to "Per-user" and update disabled value placeholder to "Defined per user" for clarity - Tweak FillUserFieldsModal description copy to better introduce the list of required per-user credentials
- Replace MCP servers DataTable with a responsive card grid (MCPServerCard) showing logo, name, alias, transport-aware subtitle, health status, auth type, visibility, and access groups - Add search input (name/alias/URL/ID) and sort controls (created, updated, name, health) above the grid, with result count and empty/loading states - Introduce host-based logo guessing via a curated WELL_KNOWN_LOGOS registry in utils, shared by MCPLogoSelector and the create/edit forms to auto-suggest logos from server URLs without overwriting manual picks - Prefill logo from discovery icon_url on curated server creation - Add Delete Server button to MCPServerView header and auto-open the Settings tab when entering edit mode - Simplify MCPLogoSelector by removing the separate preview banner; selection is now shown via the highlighted grid tile, with custom URL input populated for non-registry values - Add unit tests for guessLogoFromUrl and update MCPLogoSelector tests for the new selection UX - Ignore generated tsconfig.tsbuildinfo
- Reset selectedServerId and editServer state when deleting the currently viewed MCP server to return user to the server list - Avoids the detail view remaining mounted with an empty stub server after deletion
Adds the same EnvVarsSection to the existing Edit Settings flow (MCPServerEdit). On mount, pre-fills the form from localStorage keyed by the server's current alias; on save, persists the updated definitions back (re-keyed under the new alias if it was renamed). No backend wiring — same mock pattern as the create flow. Verified via temporary scaffold (now removed): - Mounts MCPServerEdit against a stub server with 3 seeded defs - Form renders all 3 rows pre-filled - Add/remove/change-scope work - Save persists the new defs to localStorage even though the API call fails in the verify harness (persistence runs before updateMCPServer).
The previous line concatenated `.vscode` and the dashboard's `tsconfig.tsbuildinfo` path on a single line, so neither was actually being ignored. Split them back into two entries.
mateo-berri
marked this pull request as ready for review
May 21, 2026 15:43
Contributor
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
mateo-berri
marked this pull request as draft
May 21, 2026 15:44
Contributor
Greptile SummaryThis PR bundles two distinct tracks: (1) a throwaway UI mockup under
Confidence Score: 3/5The backend refactors and bug fixes appear solid, but the new Interactions API streaming schema drops the first two text tokens on every stream, which would break any consumer assembling responses from individual delta events. The streaming iterator rewrite consumes the first and second OutputTextDeltaEvent to emit bookkeeping events without forwarding their text content as step.delta events. Short responses may lose their entire text in the stream. The prototype UI changes are low-risk, but the demo flow's Try in Claude Code modal is not reachable from the new card grid. litellm/interactions/litellm_responses_transformation/streaming_iterator.py needs the closest look. mcp_servers.tsx and MCPServerCard.tsx need a wiring fix to connect the mock demo modal.
|
| Filename | Overview |
|---|---|
| litellm/interactions/litellm_responses_transformation/streaming_iterator.py | Rewrites the Interactions API streaming transformer to support a new event schema (interaction.created/step.start/step.delta) alongside a legacy one; the first two text deltas are consumed for bookkeeping events without emitting their text content to the stream. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_servers.tsx | Replaces DataTable with MCPServerCard grid, adds search/sort, and wires prototype fill-fields and mock Claude Code modals; the MockClaudeCodeModal state is set up but never triggered because MCPServerCard has no onOpenDemo prop. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx | Adds prototype parameters and a My Credentials column to mcpServerColumns, but the function is no longer called from mcp_servers.tsx, making all new code here dead. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Refactors call_tool into helper methods, adds JWT signer injection on the tools/list path, and improves tool/server resolution for REST callers that pass names instead of UUIDs. |
| litellm/proxy/_experimental/mcp_server/rest_endpoints.py | Adds server-name-to-UUID resolution for REST tool calls, enriches 403/404 error responses with IP-filtering context, and passes canonical_server_id into call_tool for cross-server validation. |
| litellm/proxy/_experimental/mcp_server/server.py | Passes user_api_key_auth into list-tools path and adds a tool-server mismatch guard that rejects cross-server tool calls when a requested_server_id is supplied. |
| litellm/proxy/guardrails/guardrail_hooks/mcp_jwt_signer/mcp_jwt_signer.py | Extends JWT signing to cover tools/list calls with a list-only scope (mcp:tools/list), adds inject_mcp_jwt_headers_for_upstream helper for the list path that bypasses pre_call_hook. |
| litellm/llms/custom_httpx/container_handler.py | Converts empty-dict params to None before passing to httpx to prevent stripping existing query-string parameters (e.g. ?api-version=...). |
| litellm/router.py | Falls back to a forwarded model_id from kwargs when the container_id decodes to a native upstream ID that carries no LiteLLM routing payload, ensuring deployment credentials are applied. |
| ui/litellm-dashboard/src/components/mcp_tools/mock/mockMcpEnvVars.ts | New localStorage helper module for prototype MCP per-user env-var storage with cross-tab change events. |
| ui/litellm-dashboard/src/components/mcp_tools/MCPServerCard.tsx | New card component replacing the DataTable row, includes health badges, kebab menu, and prototype missing-fields highlight; missing an onOpenDemo prop to trigger the MockClaudeCodeModal. |
| ui/litellm-dashboard/src/components/mcp_tools/mock/FillUserFieldsModal.tsx | New prototype modal for end-users to fill per-user credential fields, reads/writes from mockMcpEnvVars localStorage helpers. |
| ui/litellm-dashboard/src/components/mcp_tools/mock/MockClaudeCodeModal.tsx | Terminal-styled modal simulating Claude Code error/success state; correctly reads missing fields on open, but cannot be opened from the new card grid UI due to missing wiring. |
Comments Outside Diff (1)
-
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx, line 115-140 (link)mcpServerColumnsandUserFieldsStatusCellare dead codemcp_servers.tsxreplaced theDataTablewith anMCPServerCardgrid in this PR, somcpServerColumnsis no longer called anywhere. The three new parameters (userIdForMockFields,onOpenFillFields,onOpenMockDemo) and themock_user_fieldscolumn containingUserFieldsStatusCellare unreachable.UserFieldsStatusCell.tsxitself is only imported here.
Reviews (1): Last reviewed commit: "chore: fix malformed .gitignore entry" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+83
to
+130
| # Handle OutputTextDeltaEvent | ||
| if isinstance(responses_chunk, OutputTextDeltaEvent): | ||
| delta_text = ( | ||
| responses_chunk.delta if isinstance(responses_chunk.delta, str) else "" | ||
| ) | ||
| self.collected_text += delta_text | ||
| item_id = ( | ||
| getattr(responses_chunk, "item_id", None) or f"interaction_{id(self)}" | ||
| ) | ||
|
|
||
| # Fallback: emit interaction.start, and queue content.start carrying this | ||
| # delta so the first token is preserved in the stream. | ||
| # Send the "interaction started" event on the first delta | ||
| if not self.sent_interaction_start: | ||
| self.sent_interaction_start = True | ||
| if use_legacy: | ||
| return InteractionsAPIStreamingResponse( | ||
| event_type="interaction.start", | ||
| id=item_id, | ||
| object="interaction", | ||
| status="in_progress", | ||
| model=self.model, | ||
| ) | ||
| else: | ||
| return InteractionsAPIStreamingResponse( | ||
| event_type="interaction.created", | ||
| id=item_id, | ||
| object="interaction", | ||
| status="in_progress", | ||
| model=self.model, | ||
| ) | ||
|
|
||
| # Send the "content/step started" event on the second delta | ||
| if not self.sent_content_start: | ||
| self.sent_content_start = True | ||
| self._pending_events.append( | ||
| InteractionsAPIStreamingResponse( | ||
| if use_legacy: | ||
| return InteractionsAPIStreamingResponse( | ||
| event_type="content.start", | ||
| id=getattr(responses_chunk, "item_id", None), | ||
| id=item_id, | ||
| object="content", | ||
| delta={"type": "text", "text": delta_text}, | ||
| delta={"type": "text", "text": ""}, | ||
| ) | ||
| else: | ||
| return InteractionsAPIStreamingResponse( | ||
| event_type="step.start", | ||
| index=0, | ||
| step={"type": "model_output", "content": []}, | ||
| ) | ||
| ) | ||
| return InteractionsAPIStreamingResponse( | ||
| event_type="interaction.start", | ||
| id=getattr(responses_chunk, "item_id", None) | ||
| or f"interaction_{id(self)}", | ||
| object="interaction", | ||
| status="in_progress", | ||
| model=self.model, | ||
| ) | ||
|
|
||
| # Fallback: emit content.start if ContentPartAddedEvent never arrived | ||
| if not self.sent_content_start: | ||
| self.sent_content_start = True | ||
| # Emit the delta itself |
Contributor
There was a problem hiding this comment.
In the new (non-legacy) schema, the first OutputTextDeltaEvent returns an interaction.created event and the second returns a step.start event — both without carrying the delta text. collected_text accumulates the text, but no step.delta is emitted for these two chunks. Any streaming consumer that reconstructs the response from individual step.delta events will be missing the first two tokens. For short responses (1–2 tokens) the entire text content disappears from the stream. The old code used _pending_events to queue setup events while preserving the first delta's text; the new code has no equivalent mechanism.
Comment on lines
235
to
236
| }, [serversWithHealth, selectedTeam, selectedMcpAccessGroup, filterServers]); | ||
|
|
Contributor
There was a problem hiding this comment.
mockDemoServer state and MockClaudeCodeModal are unreachable
mockDemoServer is declared and MockClaudeCodeModal is conditionally rendered on it, but setMockDemoServer is never called anywhere in this file. The new card grid renders MCPServerCard, whose props interface has onOpenFillFields but no onOpenDemo callback, so the "Try in Claude Code" preview can never be triggered. The test plan step "Click ▶ button → preview modal shows error" will always fail in this code state.
Contributor
PR overviewMedium: MCP tool routing and mock credential storageThis PR changes MCP tool resolution and adds a UI-only per-user credential demo. I found one routing check that can associate a tool name with the wrong server, plus the mock flow persists entered credentials in browser storage.
Security review
Risk: 5/10 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| ): | ||
| mcp_server = fallback | ||
| if mcp_server is None: | ||
| raise ValueError(f"Tool {name} not found") |
Contributor
There was a problem hiding this comment.
Medium: MCP tool name can be accepted based on another server's mapping
When resolution falls back to server_name only, this only checks that the requested tool name exists somewhere in tool_name_to_mcp_server_name_mapping; it does not verify that the mapping points to the matched server. An authenticated user allowed to call server A can use a tool name registered on server B and have LiteLLM route that raw name to server A, bypassing the intended tool-to-server membership check for server A.
Require the mapping entry for name or prefixed_tool_name to match the selected candidate's server id/name/alias before returning it.
| if (!serverAlias || !userId || typeof window === "undefined") return; | ||
| try { | ||
| window.localStorage.setItem( | ||
| userKey(serverAlias, userId), |
Contributor
There was a problem hiding this comment.
Low: Per-user MCP credentials are stored in localStorage
The fill modal collects values described as user credentials and writes them directly to localStorage. A later script running on the dashboard origin, or another user of the same browser profile, can read those MCP credentials from storage; keep real values in a backend secret store or make the mock avoid collecting actual secrets.
Adds end-to-end support for per-server env-var definitions and per-user
values that interpolate `${VAR}` placeholders in `static_headers` at
tool-call time.
Schema:
- New `env_vars` JSONB column on `LiteLLM_MCPServerTable` (list of
`{name, scope: "instance"|"per_user", value}`); per-user values reuse
`LiteLLM_MCPUserCredentials` with payload `type="vars"`.
Backend:
- `litellm/proxy/_experimental/mcp_server/env_vars.py` — parse / resolve
/ interpolate helpers + `MissingEnvVarsError`.
- `MCPServerManager._resolve_env_vars_for_server` substitutes
placeholders before merging extra headers. Missing required per-user
vars surface as `CallToolResult(isError=True)` with a dashboard
deep-link, so MCP clients (Claude Code, etc.) print the message to
the user.
- `GET`/`POST /v1/mcp/server/{server_id}/my-env-vars` for the fill
modal and status pill. Only `per_user` definitions are returned;
instance values stay admin-only. Stale names (after a rename or scope
flip) are trimmed on both read and write paths.
UI:
- Fill modal + status pill now hit the real endpoints (mocks remain as
fallback for demo mode).
- `MCPServer` type gains `env_vars`; networking helpers
`getMyMcpEnvVars` / `storeMyMcpEnvVars` added.
Tests:
- New `tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py`
covers parse / collect / resolve / interpolate / missing_required and
user-var storage round-trip.
- Manager tests extended to exercise placeholder interpolation and the
missing-vars deep-link error path.
Co-authored-by: Cursor <cursoragent@cursor.com>
mateo-berri
pushed a commit
that referenced
this pull request
May 24, 2026
Adopts the desired UI from the prototype PR while keeping the working backend (bulk /user-env-vars/status, scope global/user): - Replace the MCP servers table with a card grid (MCPServerCard) plus search and sort. Per-user status renders as a red "N user fields missing / Set" footer on each card, driven by the bulk status endpoint (no per-card N+1 fetch). - Restyle EnvVarsSection as a purple 3-column editor (name / value / scope) with scope labeled Instance / Per-user; value disabled for per-user rows. Surface it as a top-level section in the create and edit forms instead of inside the collapsed Permission panel. - Restyle UserEnvVarsModal to match the prototype fill modal (Per-user tag, masked inputs, "Save Credentials"). - Revert the now-unused env-var chip in mcp_server_columns to baseline. https://claude.ai/code/session_01X5YQzqswkwcVLtsBbk7Qyh
Introduces the From Template instantiation path for MCP servers: InstanceFromTemplateModal plus the Templates/Variables tabs and their mock data, wired into the Instances view alongside From Blank. The create button is guarded against double-submit: setLoading(true) and a re-entrancy check now run before await form.validateFields(), so a second click during validation can no longer fire a second createMCPServer POST and create duplicate server instances.
Contributor
|
🚅 Hi, thanks for the PR! I'm Agent Shin, the automated triage bot for this repository. What's this and why am I getting it? I read the description against our contribution rubric. Here's how it lined up: What you got right:
What's still missing:
If the description isn't updated in the next 24 hours, I'll auto-close this PR. That's not us saying we don't care about the change; we want the open-PR list to mirror what a maintainer can act on right now, so contributors don't get lost in a backlog. A closed PR is a soft "park this for later," not a rejection. Take your time; everything below still works after the close. During the grace period: just update the PR description with the missing pieces. No need to ping me; I'll re-check on the next sweep and skip the auto-close if it now passes. See what counts as QA proof for the full rubric (a linked issue alone isn't enough; it covers context, not proof). If the PR does get auto-closed in 24 hours, you still have easy recovery paths:
Internal BerriAI contributors: this rubric doesn't apply to you; ping a maintainer. (I'm an LLM, so I'm not infallible. If you think I got this wrong, ping a maintainer; they'll override me.) |
Contributor
|
🚅 Hi, thanks for the PR! I'm Agent Shin, the automated triage bot for this repository. What's this and why am I getting it? I read the description against our contribution rubric. Here's how it lined up: What you got right:
What's still missing:
Closing this PR isn't a rejection of the change. We want the open-PR list to mirror what a maintainer can act on right now, so contributors don't get lost in a backlog. A closed PR is a soft "park this for later"; your work is still here, the diff is still here, and getting it reopened is one comment away. Take your time. To bring this PR back:
What "end-to-end QA proof" means, since it's the most common gap: at least one of a short before/after screen recording / video (the bug reproducing, then the fix working; for a brand-new feature, a recording of it working end-to-end), a screenshot (or before/after screenshots) of it working, or the exact commands you ran paired with their real output against the real system. Running Internal BerriAI contributors: this rubric doesn't apply to you; ping a maintainer. (I'm an LLM, so I'm not infallible. If you think I got this wrong, comment |
4 tasks
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
7 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
This is a throwaway UI mockup, intended only to show a customer the end-to-end flow for per-user MCP env-var fields and see whether the shape solves their pain point. No backend wiring — all mock state lives in
localStorage. Expect the whole thing to be rebuilt once we get feedback.All new code lives under
ui/litellm-dashboard/src/components/mcp_tools/mock/for easy removal.Demo flow
DB_PROTOCOL(global),CORP_USERNAME(per-user). They can interpolate these in Static Headers as${DB_PROTOCOL}://${CORP_USERNAME}:${CORP_PASSWORD}@${DB_HOSTNAME}.⚠ N user fields missing · Setpill. Servers without per-user fields just show▶ Try in Claude Code.?fill_fields=<alias>✓ Readyand the Claude Code preview now showsConnected.What's mocked
localStoragekeyed by server alias (mock-mcp-env-defs::<alias>).localStoragekeyed by(alias, userId)(mock-mcp-env-user::<alias>::<userId>).mock-mcp-env-vars-changed) plus the nativestorageevent drive cross-component refresh without page reload.Known shortcuts
DataTabledoesn't expose arowClassNamehook. Loud enough for the demo; revisit if the customer expects the full row to turn red.Input.Passwordfor all per-user fields (even non-secret likeCORP_USERNAME) to keep things simple.localStorageis readable from devtools — fine for a demo, not for shipping.Files
mock/mockMcpEnvVars.ts— localStorage helpers + change pub/submock/EnvVarsSection.tsx— 3-column Form.List, mounted increate_mcp_server.tsxbefore Permission Managementmock/UserFieldsStatusCell.tsx— newMy Credentialscell in the MCP servers tablemock/FillUserFieldsModal.tsx— modal where the end user fills in their per-user valuesmock/MockClaudeCodeModal.tsx— terminal-styled error/success preview of "what Claude Code would show"create_mcp_server.tsxto render the section and persist defs to localStorage on savemcp_server_columns.tsx+mcp_servers.tsxto wire the new column + two modals +?fill_fields=deep-link handlerTest plan
/tools/mcp-servers, click Add New MCP Server → scroll to Environment Variables → add a mix of global + per-user vars → saveN user fields missing/tools/mcp-servers?fill_fields=<alias>) directly → fill modal auto-openshttps://claude.ai/code/session_01MYrd6SvmxtPxxwUMvMLyTT
Generated by Claude Code