The auto-router savings number reads a cold cache the same way whichever reason
it is cold for, and the two want opposite arithmetic. A switch leaves the new
model cold, so the write is a cost the switch caused and the baseline, already
warm on the model it never left, would have paid only a read. A first turn has
nothing cached anywhere, so the baseline would have written the same prompt and
the write belongs on both arms.
Charging every cold cache as a switch, which is all the rollup row could
support, understated a genuine first turn badly: on a 20k prompt with a 1k
completion it reported +$0.005 against a true +$0.12, and a whole conversation
only converged as it lengthened, reaching half its real value at five turns.
Worse, the write premium is fixed by prompt size while the saving grows with
completion length, so under roughly 750 completion tokens on a 20k prompt the
understated number crossed zero and a profitable route rendered as a loss.
The discriminator is what the session was served last. The complexity router
records the model it picked against the proxy-derived session id, reads it back
on the next turn and carries it on the routing decision, where the existing
per-attempt writer records it with the baseline so a fallback clears both
together. Three states, not two: no session at all keeps the conservative rule
and under-claims, a tracked session with nothing recorded is a real first turn,
and a tracked session served something else before is the switch.
Session identity moves to router_utils/session_identity.py, since the router
now needs the same reader the complexity router already had. That reader is
wider than its docstring claimed: the proxy writes the id it derives from an
`x-*-session-id` header or from Anthropic's `metadata.user_id` into the same
`metadata.session_id` the complexity router was reading, so header-carrying
clients were always covered.
Observation only. Nothing is pinned, no candidate pool narrows, and
`session_affinity` is untouched; a request naming no session never touches the
cache at all.
The popover explained the counterfactual and stopped there, so nothing
prepared a reader for a total below zero or for a first turn reading at a
fraction of what it saved. The copy that did mention the cold-cache write was
dropped in f66687b; this puts it back and adds the two consequences that
follow from it.
`_baseline_usage` rebuilt `PromptTokensDetailsWrapper` from three fields, so
everything else the request was priced on came back `None`. Audio, image and
video counts, the character and image counts Vertex prices multimodal
embeddings by, and the audio and video durations all vanished from the
baseline arm, which then priced a text-only request that never ran. On a 20k
prompt carrying 4k image tokens the baseline came out 36% light, and since it
is the arm being subtracted from, every one of those requests reported less
saving than it earned.
Dump the details and override only the cache buckets, so a field added to the
wrapper later is carried without anyone remembering to add it here. The 5m/1h
creation breakdown is dropped along with the creation count: `generic_cost_per_token`
charges a cache-write whenever that breakdown is present, even against a zeroed
count, which would have put the phantom write back on the baseline for every
long-cache request.
The exclude set is a frozenset fed straight to `model_dump`, so the override
is a call signature rather than a dict literal the reader has to trust.
`litellm/proxy/_experimental/out` is release build output, not source. Rebuilding
it locally to look at the dashboard left the files dirty in the worktree, and a
`git add -A` on the previous commit swept 145 of them in.
The path is restored to exactly what litellm_internal_staging carries, so this
branch no longer touches it.
`litellm_params.model` is not always a model. On Azure it is the deployment
name, which is absent from the cost map, so pricing it directly returned nothing
and the candidate dropped out of the pool with only a debug line. If that
candidate was the priciest, the baseline quietly became the second priciest and
every saving was understated; if the pool was all Azure, nothing priced, the
driver was disabled and the card read $0.00 with nothing at default log level
saying why. Wildcard and aliased deployments drop the same way.
`model_info.base_model` is what names the real model, and router.py already
resolves pricing through the same base_model, base_model, model chain in
`_get_model_from_deployment` and its cooldown counterpart. Candidate resolution
now follows it.
The savings driver only ever worked for the semantic auto-router. It was the one
strategy router that declared a baseline model, so every complexity, quality and
adaptive router fell through to no baseline, compute_autorouter_savings
short-circuited, and autorouter_savings_spend was structurally zero. A deployment
routing exclusively through complexity routers saw $0.00 against real spend, which
reads as "routing saved nothing" rather than "nothing was measured".
A complexity router's tier ladder already names the model an operator would have
had to run to serve the hardest request, so the counterfactual is the priciest
model in the REASONING tier, falling back to the highest-severity tier configured
when REASONING is absent. Deliberately not the priciest model the router can
reach: a pricey model sitting in a low tier is a choice the router made, not a
ceiling it was bounded by, and crediting savings against it would overstate them.
The pricing and resolution both routers need is now one module rather than two
copies. Deployments resolve through model_info.base_model before litellm_params
.model, because on Azure the latter is a deployment name that is absent from the
cost map; without that hop an Azure candidate never prices, and if it was the
priciest the baseline silently drops to the second priciest and understates every
saving.
resolve_baseline can no longer raise. It is read on the routing path while
decorating a request that is about to be served, and a dashboard counterfactual
must not be able to take a live request down; an unresolvable baseline zeroes the
driver instead.
The warm baseline was gated on the request having read from cache, so it never
applied to the case it exists for. A switch to a cold model reads nothing
precisely because that model's cache is empty, so the gate skipped it and priced
the baseline as if it too had written the whole prompt. Staying on one model
would have had that prompt cached already and paid only the read rate, so the
counterfactual side was inflated by a write it would never repeat.
On a 20k-token prompt switching opus-5 to haiku that reported +$0.1200 saved
against +$0.1000 for the same traffic with caching off entirely, so paying to
re-warm a cold model looked better than not caching at all. It now reports
+$0.0050, which is worse, as it should be.
The gate also made the write bucket a proxy for "this was a switch", which put a
cliff between a request that read nothing and one that read a single token: the
same prompt moved between +$0.1200 and +$0.0050 depending on one token. Pricing
the baseline warm whenever there is anything to write removes the cliff; the two
now differ by a rounding error.
A first turn of a genuinely new conversation is charged the same way, which
understates its saving slightly, since nothing was cached anywhere and the
baseline would have paid to write too. A single rollup row cannot tell that apart
from a switch, and understating is the safe direction for this number.
The test that asserted both arms write on a cold start was asserting the bug.
The header was hand-rolled rows, so the title, the subtitle, the legend and the
tab control all competed for one line. The subtitle is longer on Cumulative than
on Per day, so it wrapped on one tab and not the other and the chart moved with
it; pinning the controls against shrinking then pushed them past the card edge
once the viewport narrowed.
CardHeader already solves this. It is a grid that switches to
`grid-cols-[1fr_auto]` when a card-action slot is present, sizing the controls to
their content and giving the rest to the title, with the description on its own
row. Using CardTitle, CardDescription and CardAction removes the bespoke layout
rather than tuning it, and the controls wrap inside their own column instead of
overflowing.
The guard test now anchors on those slots. Its previous selector matched a
summary card's hint rather than this subtitle, so it passed with the subtitle
moved back into the controls.
Title, legend, toggle and subtitle all shared a row. The subtitle is longer on
Cumulative than on Per day, so it wrapped on one tab and not the other, growing
the header by a line and shifting the legend, the toggle and the chart with it.
The title and the controls now hold a fixed row and the subtitle sits on its own
line beneath, so nothing above the chart depends on how long that text is.
The test that guards this selected the subtitle by class and matched a summary
card's hint instead, which made it pass with the subtitle moved back into the
row. It now finds the element by its text.
The card header was a wrapping flex row, and the subtitle is longer on
Cumulative ("Running total saved") than on Per day ("Saved per day"). The extra
width pushed the legend and the accumulation toggle onto a second row, so both
jumped whenever the tab changed.
`CardHeader` is now the row itself, the same structure `SummaryCard` in this file
already uses, with the controls held in a shrink-0 box so they keep their place
whatever the subtitle says. The other two headers in the file are title-only and
correctly stay plain.
The parent router adds and removes deployments while it runs (`model_list` is
appended to and popped from during deployment updates and health checks), so a
baseline pinned on first access keeps naming a model the router no longer has,
and a pricier deployment added later can never become the baseline. Nothing
recomputes it, so the value stays wrong for the life of the instance.
The cache was never worth having: resolving the baseline over a four-candidate
router takes about 60 microseconds against a network call three orders of
magnitude larger. Removing it also removes the question the previous round was
spent answering, since there is no longer a cached value whose "not computed
yet" state has to be told apart from "computed to nothing".
A malformed `usage_object` now logs at warning rather than debug. It zeroes the
auto-router driver for every affected row, and a shape change in `Usage` would
otherwise surface only as a dashboard that quietly reads $0.00.
`auto_router_savings_baseline_model` was missing from `all_litellm_params`, so
unlike the four auto-router fields beside it, it was classified as a
provider-specific parameter and rode along in extra_body where a downstream
provider could read it.
The guard that exists for exactly this was a hand-written tuple of eleven
params, which can only catch a field being removed from the strip list, never a
new one that was never registered. That is the way this actually goes wrong, and
it is how this field slipped through. The test now derives the list from the
params model itself, so any future router-strategy field is covered the moment
it is declared.
The baseline is also qualified when it comes from configuration, not only when
derived. It travels to the spend writer as a bare string with no provider beside
it, so an operator writing `deepseek-r1` meaning Azure would otherwise be priced
against whoever owns that name. With every baseline qualified at the source,
`compute_savings_spend` no longer takes a `baseline_provider` it could never be
given: the parameter existing at all was the implicit contract, and removing it
is what makes the invariant explicit.
Three review points, one theme: state that was true in the tests but not
expressed in the code.
`autorouter_savings_spend` was a required key on `BaseDailySpendTransaction`
while every reader coalesces a missing value to zero and the aggregation is
explicitly tested against rows that omit it. Rows queued by a pod on the previous
release, or replayed from the Redis buffer across an upgrade, carry no such key,
so the honest declaration is NotRequired. Requiring it also made every
construction site outside this PR a type error for no gain.
The derived baseline was cached behind a value plus a separate "have we derived
yet" flag, so a baseline of None was indistinguishable from an uncomputed one
until the flag was consulted. `cached_property` already stores None as a real
cached value, so both attributes and the flag collapse into one declaration and
the ambiguity is gone by construction rather than by discipline.
`_baseline_usage` moves the cache-creation tokens into the cached count and drops
the creation charge, which is the whole point of the counterfactual and was
readable only to someone who had absorbed the PR description. It now says so
where it happens.
The pairing test that enumerates additive metrics now unwraps NotRequired, so a
metric declared that way is still covered rather than silently skipped.
A deployment can name its vendor in `custom_llm_provider` rather than in the
model prefix, which is the normal shape for Azure, Bedrock and Vertex. Pricing
the bare name alone resolves it to whichever vendor owns that name, or to
nothing: `claude-sonnet-4@20250514` prices at $0 without vertex_ai, and
`deepseek-r1` raises without azure_ai. Either way the candidate lost the
priciest-candidate contest, so the derived baseline silently became a cheaper
model and the driver under-reported.
Candidates now resolve through `get_llm_provider` and are carried as
`provider/model`, which is also what reaches the spend writer, so the baseline
resolves back to the vendor that served it rather than to whoever owns the bare
name.
A candidate with no per-token price is no longer eligible. Nothing that costs
nothing can stand in for what the traffic would otherwise have cost, and as a
baseline it would report the whole real spend as a loss. That outcome was
already unreachable, but only because the served model is drawn from the same
candidate set and fallbacks clear the baseline; the driver should not depend on
that chain holding.
"vs. the router's baseline model" defined the number by a config field most
operators never set, and now that the baseline is derived it named nothing at
all. The comparison is the priciest model the router can reach, so the card says
that.
Without an auto-router a deployment has to pick one model, and it has to be one
that can carry the hardest request, so the counterfactual is the priciest model
that router could have chosen.
A fixed flagship default measured savings against a model the operator may never
have run. A router choosing only between sonnet and haiku saved nobody the price
of opus, so every such deployment would have opened the dashboard to savings it
was never going to make, and the figure drifted the moment the routes changed.
The baseline is now the priciest candidate the router itself can reach, by output
rate with input breaking the tie. Routes name model groups rather than models, so
each is resolved through the parent router's deployments before being priced, and
the default model counts as a candidate. Resolution is lazy and cached, because
the parent router's deployments are still being assembled while the auto-router
is constructed.
`auto_router_savings_baseline_model` still overrides it for operators who would
genuinely have run something else. When nothing can be priced the baseline is
None and the driver reports zero, since a missing number beats a fabricated one.
Total saved needs no change: it sums the drivers, so the derived baseline flows
into it. Compression and prompt caching stay priced at the served model's rates,
which answers what each optimization saved on the request that actually ran.
The two sides of the comparison arrived spelled differently. The spend log
records a normalized model name alongside its provider, while the baseline
arrives as the operator wrote it in config, with the provider prefixed, implied
or absent, so the raw strings were never comparable.
Read as a switch, `anthropic/claude-opus-5` against a served `claude-opus-5`
priced one deployment against itself, and because the baseline arm is priced
warm while the served arm pays its cold-cache write, a request that never
changed model reported a $0.0707 loss. That mis-comparison was harmless until
the cache fix made the two arms asymmetric, so the identity check has to land
with it.
Pricing had the same root cause: a baseline resolved without a provider takes
whichever vendor owns the bare name. `azure_ai/deepseek-r1` and
`deepseek/deepseek-r1` are the same bare model at different rates, and the
difference decides the sign, +$0.039 against -$0.0731 on the same request.
Both now resolve through `get_llm_provider` to a canonical (model, provider)
before being compared or priced, so one deployment spelled two ways is not a
switch and every arm is priced under the vendor that serves it.
The per-day chart no longer stacks its drivers. Stacking sums the series into
one bar, and a driver that goes negative would be drawn below the axis while
the rest of the bar still read as the day's total.
Staying on one model writes the prompt cache once and reads it thereafter.
Switching leaves the new model cold, so it pays to write the whole prompt again,
and that charge exists only because the router switched.
Both arms were priced as if each model wrote the cache, which credited the
baseline a cache-creation charge it would never have paid again. On a
sonnet to haiku switch mid-conversation that phantom write was larger than the
entire real cost of the request: the route lost $0.0104 and was reported as
having saved $0.0179, with the sign inverted.
The baseline is now priced as the warm cache a single-model deployment would
have had, so the cold-cache write counts against the saving. A request that read
nothing from cache is a genuine first turn the baseline would have paid to write
too, so both arms still write there and cold-start savings stay honest.
The result is signed rather than floored at zero. A cache-thrashing route is a
real cost and the dashboard has to be able to report it; flooring per request
would leave a number that can only ever go up and would hide exactly the routing
behaviour an operator needs to see. The donut plots only drivers that saved,
since a negative slice has no meaning, while the card and the range total keep
the signed truth. usd() now sizes and signs off the magnitude so a small loss
reads as -$0.0004 rather than $-0.00.
A malformed usage_object and a model with no discounted cache-read rate both
degrade to zero savings rather than failing the daily spend write; pin both so
the fail-open contract cannot regress into a raise.
Three defects, one cause: the driver was wired by hand at each stage of the
savings pipeline instead of going through the owner of each stage.
Pricing re-derived per-token arithmetic instead of calling the cost engine.
`prompt_tokens` already includes cache-read and cache-creation tokens, so
charging the whole total at the flat input rate and then subtracting a separate
cache-write penalty priced those tokens twice. Both arms now price the identical
usage through `generic_cost_per_token`, so each token is charged once in its own
dimension and tiered rates, ephemeral cache-write tiers and regional uplifts stay
consistent with what the request was actually billed. On a cache-heavy
opus-to-haiku switch the old formula reported $0.0458 against a true $0.0717.
The savings baseline was recorded by a second per-attempt writer sitting beside
the routing decision, and the exit path that runs when no pre-routing strategy
applies only cleared the decision. A fallback to a plain model group therefore
kept the previous attempt's baseline, letting a caller who forces a router
failure inflate the recorded savings. Both facts now travel from one response
through one recorder, so no exit can clear one and leave the other.
Aggregation summed every daily metric except this one, so two requests sharing a
rollup key kept only the first value, and since the cross-pod Redis drain runs
the same merge the field was dropped on every flush.
The driver was also absent from the entire read path: no column in the rollup
query, no accumulation, no field on the response model. The dashboard read a key
the API never sent, so the card, donut segment and graph series would have
rendered $0.00 forever however much routing saved. Wiring the write path without
the read path is the failure this had already shipped, so the drivers are now
enumerated from the response model itself and each is asserted to be summed,
accumulated, carried and totalled.
Also collapses the twelve hand-written per-field blocks in the daily upsert into
one enumeration feeding both the create and the increment.
Adds auto-router as a third savings driver alongside compression and prompt
caching: a summary card, a donut segment, and a series in the savings graph.
Savings are the net dollars from routing a request to the selected model
instead of a counterfactual baseline. The delta prices prompt tokens at each
model's input rate and completion tokens at each model's output rate, then
subtracts the cache-write penalty the selected deployment incurs on a cold
cache. Cache-read discounts stay attributed to the prompt-caching driver to
avoid double-counting. The result is floored at zero so an escalation to a
pricier model never reads as negative savings.
The baseline model defaults to claude-opus-5 and is operator-configurable per
deployment via the auto_router_savings_baseline_model litellm_param. It flows
AutoRouter to PreRoutingHookResponse to the metadata bucket to SpendLogsMetadata
to the daily spend writer, mirroring the routing_decision path, and is stripped
from untrusted caller metadata so it cannot be spoofed.
Savings accrue into a new autorouter_savings_spend column on the six
LiteLLM_Daily*Spend rollup tables; no LiteLLM_SpendLogs queries are added.
* feat(spend-logs): record when a spend log row is the auto-router's own classifier call
The complexity router's classifier sub-call copies the parent request's metadata
verbatim, so its spend log row carries the caller's key, team and user and is
indistinguishable from traffic the caller actually sent. Nothing on the row says
otherwise: call_type is "acompletion" either way, model_group is overwritten to the
classifier's own model group so the row never looks auto-routed, and routing_decision
is absent exactly as it is on an ordinary request.
Record the fact the system already knows at call time. internal_call_origin is
declared on SpendLogsMetadata, which is the allowlist _get_spend_logs_metadata
projects onto, and stamped in _classifier_call_metadata; both classifier paths
already route through that one function and it feeds the metadata and
litellm_metadata buckets alike, so every request surface is covered at one site.
The key is reserved rather than caller-supplied, so it joins routing_decision in the
untrusted-metadata strip and a caller cannot label their own traffic as router
overhead.
The classifier call also inherited no session identity, so the router minted a fresh
trace id and the row landed in a session of its own. Forwarding the parent's session
puts it in the trace of the request that triggered it, which is where an operator
looks for what the routing cost.
* feat(ui): show which log rows are the auto-router's own classifier calls
A classifier row now carries internal_call_origin and shares its parent's session,
so the session trace lists it beside the request that triggered it. Without a marker
in the sidebar it reads as another call the caller made, which is the confusion this
resolves.
The tag renders only for a recognized origin, so ordinary traffic and any future
origin this build does not know about stay unlabelled rather than being asserted as
classifier calls.
* feat(proxy): add a generic list contract for management/v1 entity lists
Paging, sorting, filtering and search for an entity collection, declared once
as a ListSpec and served by handle_list. The route injects a ListExecutor that
owns its table, so this module never imports Prisma.
The caller's scope is derived from the caller alone and ANDed with whatever
they filtered on, so a query parameter can only narrow what they may read.
This is the shared half of the budgets list; it lands here so the endpoint has
something to register against, and drops out when the framework arrives on its
own branch.
* feat(proxy): add GET /management/v1/budgets
The Budgets page reads /budget/list, which returns the whole table as a bare
array with no way to page, sort or filter it. A customer with enough budgets to
fill the page has no way to find one.
Registers LiteLLM_BudgetTable against the management/v1 list contract: sortable
on budget_id, max_budget, tpm_limit, rpm_limit and created_at, default order
newest-first with budget_id breaking ties, search on budget_id, and filters for
budget_duration, max_budget and created_at. budget_duration is deliberately not
sortable; the column holds "7d"/"30d" strings, so a lexicographic ORDER BY puts
"30d" ahead of "7d".
tpm_limit and rpm_limit are BigInt? in Prisma, so rows validate through a
pydantic model on the way out and serialize as JSON numbers.
A caller without admin view is refused 403 as a problem document rather than
served an empty page. /budget/list is untouched.
* fix(proxy): rework the budgets list onto the merged list contract
PR #35308 landed a different shape than this branch was written against: `where`
is a tuple of frozen predicates rather than a Prisma-shaped mapping, `ListSpec`
carries both the row and the wire type, and `where_sql` / `order_by_sql` render
for a raw-SQL executor. The budgets executor now queries through `query_raw` the
way the spend logs facet does, selecting only the columns it serves.
Also casts datetime binds in `where_sql`. They cross into the query engine as
JSON, so an uncast placeholder arrives as text and Postgres refuses
`timestamp >= text` outright; every `filter[created_at][gte|lte]` was answering
500. The cast reads the bind as an instant and drops it to naive UTC to match
Prisma's TIMESTAMP(3) column, the same one /spend/logs/ui applies.
* refactor(proxy): fold the predicate renderer instead of recursing
recursive_detector flags `_render_all`, and the flag is fair: it recursed once
per predicate, so the stack grew with the number of filters on the request for
no reason. Walking a predicate list is a running bind index, which is a fold.
`_render` still re-enters for `AnyOf`, but its clauses are plain comparisons
built by `?q=`, so that nesting is one level deep and no caller can drive it
deeper.
* fix(tool-management): drop unsupported prisma select kwarg from team lookup
POST /v1/tool/policy returned HTTP 500 for every request carrying a team_id,
with "LiteLLM_TeamTableActions.find_unique() got an unexpected keyword argument
'select'". _resolve_team_id_to_object_permission_id looked the team up with
select={"object_permission_id": True}; prisma-client-py 0.11.0 has no select
kwarg on find_unique, so the call raised TypeError and the handler's except
block turned it into a 500. Both the initial read and the fallback read taken
when a concurrent writer wins the update_many race were failing the same way,
so per-team tool blocking was unreachable
The kwarg is dropped rather than replaced; the generated client has no
projection API, and this is a single-row lookup where selecting all columns
costs nothing worth working around
The existing tests missed this because the team table was an AsyncMock, which
accepts any keyword. The new double binds each call against the real
find_unique and update_many signatures, so an unsupported kwarg raises the same
TypeError production does
* refactor(test): tighten typing on the tool policy team table double
Replaces the double's Any annotations and bare list types with concrete ones:
kwargs are object, rows are Sequence[MagicMock] held as a tuple, and the call
logs are list[dict[str, object]]. Behaviour is unchanged; the double still
binds every call against the real generated prisma action signature, verified
by reintroducing the select kwarg and watching both regression tests fail
A bedrock guardrail configured mode: during_mcp_call never ran. ProxyLogging
remapped the event to during_mcp_call and dispatched, but bedrock's own
async_moderation_hook then hard-coded during_call and re-checked, so the second
check rejected the very requests the guardrail was configured for and the tool
call proceeded unscanned with no error.
Remap call_mcp_tool the way model_armor already does, which matches the remap
ProxyLogging.during_call_hook itself performs, and teach the shared
get_guardrails_messages_for_call_type helper that an MCP tool call carries its
payload in the same messages key, without which the hook passes the gate and
then bails on an empty message list.
The SLO measures how many gateway replicas happen to be warm rather than the
request path. Clearing the floor needs roughly 5-7 replicas at ~10-14 RPS each;
stage idles at one and reactive HPA scale-up lands minutes into a ~3 minute
test.
It failed both of its assertions on consecutive days: 93.3% errors at an
inflated 264 RPS, where the failing requests never reached a pod and closed-loop
RPS rose because they failed fast, then 16.7 RPS with zero failures.
The covers marker and registry row stay put, so the cell returns to the gap
list rather than disappearing from the denominator.
The ID-JAG egress arm could only assert a caller that presented its own IdP
identity token on the request, so an agent holding a brokered LiteLLM
credential got a 412 and never reached the upstream. The assertion captured at
SSO login was already persisted per user for exactly this purpose, but nothing
read it back.
The arm now falls back to that stored assertion, keyed on the authenticated
principal's user_id. The identity is always taken from the credential the
gateway authenticated, never from a caller-supplied field, so no caller can
select whose identity is asserted upstream. A missing, expired, or
unidentified subject stays a 412; ID-JAG exists to assert a specific user and a
missing subject has no safe substitute. A store outage is the one exception: it
is surfaced as a typed AssertionStoreUnavailable and mapped to 503, so a
database blip cannot 500 the egress or the upstream-401 retry, and does not
tell the user to sign in again over something they cannot fix.
Sourcing a subject from the store rather than the request changed what
invalidation can rely on, so the exchanged-token cache changed with it. The
entry is now addressed by a slot key derived from the principal, plus the
caller's own token when it presented one, with a fingerprint of the subject
token and config stored beside the bearer and compared on every read. A
mismatch reads as a miss and re-mints, so a rotated assertion or an edited
server config cannot be served a bearer authorized under the old inputs, and
two callers cannot receive each other's. Invalidation is a single delete of a
key it can always compute, needing no store lookup on the recovery path.
The upstream-401 invalidate-and-retry path was also gated on a truthy inbound
subject token, which skipped recovery entirely for store-sourced calls. The
gate is now mode-aware: token_exchange still requires an inbound token because
it has nothing else to mint from, id_jag does not.
oauth2_id_jag is also now selectable in the admin dashboard with its own field
set, instead of being reachable only from config.yaml or the REST API. The
auth-type selects drop antd list virtualization: at eleven options the last one
no longer mounts, which is a scroll in a browser but makes the option
unreachable to anything reading the rendered list.
Co-authored-by: Yassin Kortam <yassin.kortam@gmail.com>
All three send a `telemetry` object in the arguments to Datadog's
search_datadog_logs tool. Datadog tightened that tool's input schema to reject
unknown properties, so every call now fails validation with 'unexpected
additional properties ["telemetry"]' before the behavior each test exists to
prove is reached.
`telemetry` was never a documented Datadog parameter; the tests relied on the
server ignoring extra properties. The proxy transmitted exactly what the tests
supplied and surfaced the upstream error faithfully, so this is test-side.
The covers markers and registry rows stay put: the collector counts a cell as
covered only when a test pytest would actually run declares it, so skipping
hands all four cells back to the gap list where they belong.
* fix(aiohttp): dispose recycled client sessions deterministically
LiteLLMAiohttpTransport replaced its cached aiohttp.ClientSession on
loop-mismatch, loop-inspection failure, and "Session is closed" retry
without reliably closing the previous session:
- the close task from asyncio.create_task() was never referenced, so
it could be garbage-collected before running;
- the (RuntimeError, AttributeError) fallback branch replaced the
session without closing it at all;
- sessions bound to a closed event loop were abandoned to the GC
("rely on GC"), and sessions bound to a loop running in another
thread were closed from the wrong loop.
Replaced sessions surfaced as intermittent "Unclosed client session" /
"Unclosed connector" errors from the event-loop exception handler at
GC time.
_close_recycled_session() now covers the three lifecycles a recycled
session can be in: same-loop closes keep a strong task reference until
completion; sessions owned by a loop running elsewhere are closed on
their own loop via run_coroutine_threadsafe; sessions whose loop is
gone are disposed synchronously through the connector teardown that
aiohttp's own finalizer uses, which releases pooled connections and
silences the finalizer warnings.
Fixes#24230
* fix(aiohttp): guard threadsafe close callback against cancelled futures
---------
Co-authored-by: Anmol Jaiswal <68013660+anmolg1997@users.noreply.github.com>
* feat(proxy): add generic list handler for /management/v1
Adds the ListSpec/QueryPlan machinery the control-plane list endpoints are
meant to share, so a resource declares what it exposes instead of hand-rolling
its own paging, sorting and filter parsing.
build_query_plan is pure: it turns query parameters into a QueryPlan or an
RFC 9457 problem without any I/O, which is what lets the plan be asserted as a
value. The database half is a ListExecutor protocol injected by the caller, so
this module has no Prisma dependency at all.
Four things the framework guarantees rather than leaving to each resource: the
spec's unique tiebreaker is always the final sort key, so pages cannot repeat
rows when the leading column is all nulls; ordering is NULLS LAST in both
directions, since Postgres otherwise floats empty values to the top the moment
the sort direction flips; the scope predicate is a separate conjunct ahead of
every caller filter, so a filter on a scoped column cannot widen it; and a
denied scope is a 403 problem rather than a 200 with an empty list.
No route and no consumer yet; budgets registers against it next. The facet
endpoint's has_more shapes are untouched, and a test pins them so page mode
cannot quietly absorb them.
* fix(proxy): accept the bare filter[field] form in the list framework
Section 5 of the design doc spells equality without an operator bracket
(`?filter[status]=active`, and `/management/v1/keys?filter[team_id]=` in the
sub-resource paragraph); only the other operators carry a second bracket. The
parser only understood `filter[field][op]`, so the canonical spelling came back
as an unknown query parameter.
`filter[field]` now resolves to the field's `eq` operator, which means it still
goes through the declared operator set rather than around it: a field that does
not offer `eq` rejects the shorthand. The allowed-parameter list advertises the
bare spelling for `eq` and the bracketed one for everything else.
Drops two guards from the key parser that could not fire. Operator validation
already rejects every malformed operator, and `field in spec.filters` already
rejects every field nobody declared, so a well-formedness check on top of them
was unreachable; the tests cover the malformed keys directly instead.
* fix(proxy): validate list specs at construction and reject repeated params
Two gaps a review flagged on the list framework.
The page-size cap was only enforced against a supplied page_size, so a spec
whose default_page_size exceeded its max_page_size served more rows than the
resource allows on exactly the request that omits the parameter. A default of
zero was worse: it reached the total_pages division and made the resource 500 on
every request. ListSpec now validates 1 <= default_page_size <= max_page_size
when it is built, so a misconfigured resource fails as it is registered rather
than per request. default_sort is checked against sortable for the same reason;
caller-supplied sort was already validated, but the default never passed through
that path and a typo there reached the ORDER BY clause untouched. Raising is
right here despite the usual model-failures-as-values rule: there is no request
in flight and no caller to answer.
Repeated query parameters silently collapsed to their last value, so
?page=1&page=999 paged from 999 and a repeated sort key quietly won, which is
the same silently-altered-semantics failure the surface already rejects unknown
parameters to avoid. They are now a 400. The check lives in handle_list rather
than build_query_plan because a Mapping[str, str] cannot represent a repeat at
all; the boundary that can see one is the boundary that rejects it. A denied
scope still outranks it, matching every other rejection here.
Also corrects the order_by_sql docstring, which claimed every field reaching it
had been validated against sortable. That held for caller-supplied sort only.
* refactor(proxy): model list predicates as frozen values instead of dicts
The LIT002 budget rejected the framework: building a where-fragment meant a dict
literal per operator, and a dict keyed by a column name chosen at runtime cannot
be frozen into a TypedDict or a dataclass field, so there was no spelling of the
old shape the rule would accept.
Replacing the fragments with a tagged union removes the construction entirely. A
plan's where is now a tuple of frozen Compare / Within / IsNull / AnyOf, matched
exhaustively, and the field name is a value rather than a key. That also retires
the Mapping[str, object] the plan used to carry, which said nothing about what
was inside it and left the fragment shape as a convention two sides had to keep
agreeing on. Scope predicates take the same type, so a resource declares its row
filter in the same vocabulary rather than hand-rolling a backend dict.
where_sql renders a plan for a raw-SQL executor, binding every caller-supplied
value to a numbered placeholder and writing only spec-declared column names into
the statement. It is the counterpart to order_by_sql, which already existed for
the same reason: nulls ordering forces the executor onto raw SQL, so the escaping
and placeholder arithmetic belong in one reviewed place rather than in each
consumer.
Also folds the two remaining mutable builds out of the module (set comprehensions
and Counter to frozenset/tuple, the serialized page to a tuple pydantic coerces),
and lifts the LIKE escaper into common.py so the facet endpoint and the framework
share one copy instead of two that can drift.
No behavioural change to the facet endpoint; its tests, including the one pinning
the escaping, pass untouched.
An MCP permission level answers which servers and tools it permits, and a
level that answers nothing places no restriction. Key auth was reading a
lookup FAULT as that same answer, so the end user, agent and org ceilings
quietly disappeared for as long as one lasted, while the keyless
gateway-admitted path failed closed on the very same fault.
Those levels now separate the two fault classes the user level already
did. A principal row that NAMES an object_permission_id whose contents
cannot be read is a known entitlement with unknown contents, so it denies.
A lookup that fails before we can tell whether the principal is entitled
at all still places no ceiling, that being the state which existed before
the level did; denying there would refuse MCP to the majority of callers,
who have no such entitlement configured. The keyless path is unchanged.
Resolves LIT-4960
PR #35185 added classifier_context_window_size and classifier_context_per_turn_chars
to ComplexityRouterConfig; they worked via config.yaml and the API but had no UI
control on the Add Model or Edit Auto-Router screens. Wires the two fields into
both, shown only when the LLM classifier is selected.
* fix(mcp): annotate connected-app reachability on the gateway connect page
The MCP connect page resolved its server grid through the dashboard identity
(admin shortcut or view_all returns the whole registry) while the gateway DCR
session it sets up resolves servers as an admitted subject through grant
sources only, so the page showed servers and tool counts the session is never
served. GET /v1/mcp/server now accepts connected_app_view=true and stamps each
returned server with connected_app_reachable, computed by the same
_reload_admitted_user + get_allowed_mcp_servers pair the live session uses.
The connect page requests the flag in connect mode and renders unreachable
servers dimmed with a label, excluded from the Connected count and tool-count
fetches. Failure to build the admitted set marks everything unreachable, which
matches what such a session would actually be served. Default behavior without
the param is unchanged for every existing consumer.
* fix(mcp): block connecting unavailable servers from the connect-mode detail view
A server the connect page marks unavailable could still be added through its
detail view Connect action, so the selection could contain servers the
connected-app session is never served. The unavailability decision now lives in
one predicate, connectUnavailabilityLabel, consumed by the card indicator, the
detail view action area, the toggle-on path, the oauth auto-select effect, and
the Connected count, so no interaction path can disagree with the label. This
also closes the same pre-existing hole for servers marked not supported on this
connection, whose detail view likewise offered Connect, and removes a
grandfathered nested ternary, ratcheting the eslint suppressions baseline down
* fix(mcp): hide unreachable servers on the connect page instead of dimming them
Product decision: the connect page should only show what a connected-app
session will actually be served, so annotated-unreachable servers are now
filtered out of the connect-mode list at fetch time rather than rendered
dimmed. Unsupported auth types keep their existing dimmed label since they are
a property of the server, not the caller. A user with zero reachable servers
gets an explanatory empty state pointing at grants. The list filter is the
single source: counts, tabs, auto-select, detail view, and tool-count fetches
all derive from the already-filtered state
* fix(mcp): guarantee the connect view lists every session-reachable server
The connect view's membership came from the dashboard resolver with the
admitted-subject answer only annotated on top, so a server reachable by the
session but missing from the dashboard list would be invisible on the page; an
under-report, the mirror of the bug this PR fixes. The connect view now unions
in any session-reachable server the dashboard resolver did not list, built
from the registry and redacted through the same ladder, so page membership
equals the admitted set by construction in both directions
* fix(mcp): honor connected_app_view only for the dashboard UI session credential
The reachability view resolves through the owning user's admitted identity, so
a caller-passed virtual key could use the param to enumerate servers beyond
its own scope (ids, names, descriptions of the owner's wider grants). The view
is now gated on is_ui_session_credential, a predicate factored out of
resolve_ui_session_team_ids so the two user-identity widening sites share one
trust boundary: the SSO-minted dashboard session token acting as its user. Any
other credential gets the param as a no-op and the admitted resolver is never
consulted for it
* fix(mcp): resolve UI sessions with the admitted-user context everywhere, not per endpoint
The list endpoint unioned in session-reachable servers itself while tool
counts, Connect actions, and credential endpoints still authorized through
build_effective_auth_contexts, whose contexts carry team grants but never the
user row's own object permission; a user-granted server could render on the
connect page while every interaction on it failed. The admitted-user context
(the same auth a gateway session resolves with) is now appended inside
build_effective_auth_contexts for UI session credentials, so the page list and
every per-server action endpoint answer identically, and the list endpoint's
one-off union is deleted. Caller-passed keys are still never widened
(is_ui_session_credential gate inside the context builder) and a reload
failure falls back to team contexts only
* fix(mcp): resolve non-admin dashboard sessions as the admitted subject on tool routes
Server reachability on the REST tool routes came from the widened context
union while tool permission checks ran on the bare session key, which carries
no object permission, so a dashboard user could invoke tools their user-level
grant excludes. Rather than bookkeeping which context granted which server,
the routes now choose one principal at the boundary: acting_user_auth swaps a
non-admin UI session for the admitted-subject auth, the same identity a
gateway session resolves with, so reachability, per-source fail-closed tool
ceilings, rate limits, and billing attribution all bind through the admitted
arms that already exist downstream. Admin sessions keep their operator view
and caller-passed credentials are never widened. One swap point per route,
no per-server principal picking, no parallel permission logic
* fix(mcp): derive the connect page's detail view from the reachable server list
The detail view held its own copy of the server object, so it outlived the list it came
from. When a refetch dropped that server as unreachable, the open detail view kept
rendering it and its Connect action still ran: the guard looked the server back up by id
or name in the current list, found nothing, and fell through, because a missing target
read as "nothing to block" rather than "no longer connectable"
Store the selected server's id and derive the row from the list instead. A server the
list no longer carries cannot be the detail view's subject, so the stale render, the
stale tools query and the guard bypass stop being reachable states rather than being
blocked one at a time. handleToggle now takes the server it is toggling, which deletes
the lookup that could miss at all
* refactor(mcp): one owner for the identity a dashboard session acts as
Three call sites reloaded the admitted subject independently, and the management
endpoint carried its own copy of the reload, the HTTPException swallow and the logging.
admitted_user_context is now the only place that answers "what user identity does this
dashboard session act as", and the connected-app reachability helper reads it, which
also drops its dead empty-user_id branch
That owner now carries the request's tracing span onto the admitted principal.
_reload_admitted_user builds a fresh auth from the user row and has no span of its own,
so swapping it in on the REST tool routes silently detached every downstream lookup and
the tool-call logging from the request's trace
Toolset scoping and the acting-as-user swap are mutually exclusive, so they now share
one owner on the tools list route. The admitted subject resolves per grant source and a
team source deliberately carries none of the caller's object_permission, so a toolset
narrowing layered on top would evaporate on every team-granted server: the request would
be admitted through the toolset grant and then served tools from servers the toolset
never named. A request carrying a toolset name stays on the caller's own credential,
exactly as it did before the swap
* fix(mcp): commit every async connect-page write against the list as it stands
Three continuations in the panel decided against state captured before their await and
committed after it, so a reachability refetch landing in between could not be seen
handleToggle validated the server at click time and then, once listMCPTools resolved,
wrote its name into the selection whatever the list had since become; a server the
refresh had dropped was selected anyway. It now re-asks connectableNow at the commit,
and that predicate resolves the id against the current list, so absence fails closed
instead of reading as nothing to block
The load pipeline was worse, because its cancel flag was shared across runs: the
successor's effect body reset it to false before the predecessor's fetch resolved, so a
superseded load could still run setServers and put the dropped server back on the page
outright. The flag is now a per-effect local that only that run's cleanup can clear,
which is also what makes unmount stop the chunked tool-count loop again. The load
passes its own liveness check down to the tool-count and oauth-status writes rather
than having them consult a flag they share with every other run
* fix(mcp): write the connect-page server list to its ref as it is committed
connectableNow resolves a server id against serversRef, but that ref was a mirror kept
in step by a passive effect, so it lagged the state it mirrored by however long React
took to render and flush. A continuation resolving inside that window read the previous
list: the commit-time reachability check would find a server the refetch had already
dropped, call it connectable, and select it, which is the mismatch the check exists to
prevent
The lag was the whole defect, so the mirror is gone. commitServers writes the ref and
the state together, at the one point the list is ever replaced, and the ref is now
never older than the last committed list. Readers that want the newest answer
(connectableNow, the oauth auto-select effect) get it; rendering still derives from
state, so what is on screen is unchanged
Pinned by a test that resolves the refetch and the in-flight Connect in the same tick,
with no render flushed between them, which is the interleaving the earlier regression
could not reach. The two prop mirrors are deliberately untouched: their staleness is
inherent to appending to a parent-owned list from an async callback rather than caused
by the mirror, and no reachability decision reads them
Greptile found that the OpenAPI fallback added a commit ago collapsed operation
IDs that registration keeps apart: foo/bar and foo.bar register as two tools but
sanitize_openapi_tool_name rewrites both to foo_bar, so a policy naming either
also decided the other.
The cause was two owners for one map, and picking the wrong one. Registration
names an operationId inline at _register_openapi_tools with
operation_id.replace(" ", "_").lower(), which keeps / and . ; the separate
sanitize_openapi_tool_name replaces every character outside [a-zA-Z0-9_-] and
belongs to register_tools_from_openapi, which has no production caller. Nothing
made the matcher use the one that actually registers, so it used the lookalike.
That inline expression is now openapi_tool_name in utils, and both registration
and the matcher call it. Replaying the registering function is the whole safety
argument, and it is structural rather than a claim: two operationIds that
register as two tools normalize to two names here by construction, because this
is the map that registered them. A coarser lookalike cannot be substituted
without a test failing.
The matcher also loses its exact-then-fallback split. The transform is identity
on native servers and idempotent on already-registered names, so normalizing
both sides is exact matching where no OpenAPI spec is involved. Executable lines
drop by three this round; the branch is +5 over the merge-base for four shared
owners that removed duplication at six call sites.
The collector read @pytest.mark.covers off every collected item, and
collection does not evaluate skips, so a test carrying both a skip and a
covers marker reported its cell as covered while asserting nothing. 17
files under tests/e2e do exactly that, which inflated the headline from
290/434 to 311/434.
A cell now counts as covered only when at least one test pytest would
actually run declares it; a cell claimed by both a live and a skipped
test stays covered. Skip state comes from pytest's own evaluator, so
skip, skipif (bool and string conditions), and module-level pytestmark
resolve exactly as they do in the e2e run. Cells left uncovered this way
are listed under the headline and exported as skipped_markers (JSON) and
litellm_e2e_coverage_skipped_markers (Prometheus) so the gap surfaces
instead of disappearing; the Loki line contract is unchanged. A marker
on a skipped test that points outside the registry is still an orphan,
so --strict keeps its reach.
Because skipif resolves against the environment the collector runs in,
the number now depends on that environment; run it where the e2e suite
runs. A pytest.skip() call inside a test body remains invisible to a
static pass, which the module docstring and README both state.
Greptile flagged that match_known_tool_name case-folded both the configured
entry and the derived spellings. Routing keeps two tools whose names differ
only in case as two tools, so folding merged identities the dispatcher
separates: on a server exposing getPet and getpet, an allowlist naming getPet
also granted getpet, and a blocklist naming getPet also denied getpet. That is
unauthorized execution on one arm and the wrong tool denied on the other.
Matching is now exact, which is what identity means here. The case leniency it
replaces was never typo tolerance; _register_openapi_tools rewrites every
operationId through sanitize_openapi_tool_name, so an allowed_tools entry
holding the spec's own spelling never equals the registered name. That link is
recovered by replaying the same rewrite, and only on servers that carry a
spec_path, which is how the rest of the manager already recognizes an OpenAPI
server. Every name that rewrite produces is lowercased, so no two tools on such
a server can differ only in case and the fold cannot merge anything.
Native servers get no folding at all. test_case_folding_applies_to_openapi_
servers_and_not_to_native_ones pins both halves, and two tests pin that a
policy naming one tool leaves its case-variant sibling alone. Dropping the
spec_path guard, dropping the fold, and forcing the fold path are all killed.
The two pre-existing case-insensitivity tests describe OpenAPI servers in their
own docstrings but built fixtures without a spec_path, a shape production never
produces for one; they now set it.
Bugbot flagged REST listing advertising key/team grants that tools/call then
refuses. The listing side was fixed by routing through
filter_tools_by_key_team_permissions, but the two paths still answered the
question with separate implementations that only happened to agree: listing
stripped the known prefix and compared bare, dispatch compared whatever name it
was handed, and each carried its own reading of None and of an empty list.
Changing either side silently diverges from the other, which is how this defect
appeared in the first place.
MCPRequestHandler.tool_is_granted owns the whole decision, and both
is_tool_allowed_for_server and filter_tools_by_key_team_permissions read it.
None still means no tool-level restriction and an empty list still grants
nothing, now stated once. Grants are stored bare by every writer, so matching
stays exact against the bare name, deliberately unlike the server-level lists,
which honor every spelling routing accepts.
test_key_team_listing_and_dispatch_agree drives both production paths over one
matrix. It asserts the expected verdict as well as the agreement, because two
paths reading one predicate makes equality alone tautological: a wrong
predicate keeps them consistent and the agreement assertion alone survived two
mutants that the verdict assertion kills.
The allow list, the deny list, allowed_params and the discovery filter all ask
the same question, "which configured entry names this tool on this server", and
each answered it in its own idiom: any() over a spelling tuple, all() over the
same tuple negated, a next() that pulled a value out of a dict, and a
lowercased set membership. Two review findings on this PR were symptoms of that
duplication. Deriving the operands differently at one site produced the
over-strip; needing a value rather than a boolean at another produced a
truthiness test that read an explicitly empty allowed_params list as "nothing
configured" and allowed every parameter.
match_known_tool_name returns the matching entry or None, and all four sites
read it, so no site can test a container's values to decide membership and the
empty-list fail-open is no longer representable. Matching is case-insensitive
everywhere, which closes the last divergence between discovery and dispatch: a
case-variant disallowed_tools entry used to hide a tool from tools/list while
tools/call still executed it.
Executable lines over the merge-base drop from +9 to +4, all of it the new
owner; mcp_server_manager.py loses 12 lines and the discovery filter loses 17.
The REST listing filter matched key/team grants through _tool_name_matches, which after the prefix-boundary change answers for every spelling routing accepts. Key-level entries in mcp_tool_permissions and toolset rows name a tool on one server and dispatch compares them bare, so a wire-form entry advertised a tool that tools/call then refused. REST listing now goes through filter_tools_by_key_team_permissions, the same function the MCP list path uses.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The gateway publishes a tool as `<server prefix><separator><tool name>` and has to
recover that boundary on the way back in, to compare a called name against a toolset
or allow/deny list and to rebuild the native name sent upstream. Several sites
recovered it by cutting at the FIRST separator and others reconstructed it by hand
from `MCPServer.name` with a literal `-`, so both disagreed with the prefix the
server actually publishes
`get_server_prefix` publishes short_prefix, then alias, then server_name, then
server_id; it never reads `name`. A server with no alias therefore publishes its
hyphen-filled UUID `server_id` as the prefix, and cutting at the first separator
leaves most of the UUID glued to the tool name. Every comparison against the stored
`(server_id, tool_name)` toolset row then misses: an allowlist denies a tool the list
endpoint just advertised, and a disallowed entry stops blocking, which fails open
Recover the boundary in one place instead. `match_known_server_prefix` matches a name
against the server's registered prefixes, longest first so a prefix that itself
contains the separator beats a shorter prefix that is merely its leading segment, and
returns None when the name carries none of them. `strip_known_server_prefix` and
`is_tool_name_prefixed` both delegate to it, and the sites that receive a wire name
call the owner rather than re-deriving the boundary. `split_server_prefix_from_name`
stays for the routing pair it was written for, with a docstring saying so
The server-level permission checks are the other half. They run after the boundary is
already resolved, so their input is bare and the correction there is to derive the
wire form rather than strip it back out; stripping a stored entry a second time cuts a
boundary the caller already consumed, which breaks a native name that itself opens
with the server prefix. Deriving from `get_server_prefix` alone is not enough either,
because routing resolves an inbound name against every prefix from
`iter_known_server_prefixes`, so enforcement keyed to the published spelling answers
for fewer names than are reachable. Turning `LITELLM_USE_SHORT_MCP_TOOL_PREFIX` on
republishes every tool under the short ID while an entry stored under the alias stays
routable and silently stops being enforced, which is a fail-open on a config nobody
edited. `iter_known_tool_name_spellings` yields the bare name plus the wire form under
each accepted prefix, and the allow list, the deny list, `allowed_params` and the
routing map that `_create_prefixed_tools` builds now all key off that one function, so
the set of names enforcement honors and the set routing accepts cannot drift apart
`_tool_name_matches` takes the server as a required argument, so a future caller
cannot silently fall back to guessing, and it matches against that same spelling set,
so `tools/list` hides exactly what dispatch refuses. Answering for fewer spellings in
the filter than enforcement honors leaves a blocked tool advertised, which is how the
alias-form entry above stayed listed even once the call was refused. The OpenAPI
registry lookup builds its key the same way registration does, via `add_server_prefix_to_name` and `get_server_prefix`,
because registration used exactly one key; a server whose `name` differs from its
published prefix stops missing its own tools
