Skip to content

Cherry-pick #29311, #29343, #29358, #27913, and #29447 onto stable/1.85.x#29460

Merged
mateo-berri merged 6 commits into
stable/1.85.xfrom
litellm_cherrypick_1_85_x
Jun 2, 2026
Merged

Cherry-pick #29311, #29343, #29358, #27913, and #29447 onto stable/1.85.x#29460
mateo-berri merged 6 commits into
stable/1.85.xfrom
litellm_cherrypick_1_85_x

Conversation

@yuneng-berri

@yuneng-berri yuneng-berri commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Relevant issues

Backport of three PRs that landed in v1.84.4 via #29352 and #29363, plus two more (#27913 and #29447) that don't apply to the 1.84.x line but do apply to 1.85.x. All five carry into the next 1.85 patch.

Linear ticket

N/A

Pre-Submission checklist

  • I have added meaningful tests (carried over from the source PRs)
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

CI (LiteLLM team)

  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Five cherry-picks from litellm_internal_staging onto stable/1.85.x, plus one follow-on fix to the manual conflict resolution:

Commit on this branch PR Source on staging Also in v1.84.4?
fcdf0231d0 #29089 / #29311 (fix: duplicate claude code traces) 581c30f1e8 yes
87b0e47485 #29343 (refactor(proxy/auth): normalize Bearer prefix in safe-hash helper) 94a043efb2 yes
a06ec43b36 #29358 (fix(reset_budget): write only {spend, budget_reset_at} and stop pre-zeroing counter) 7d1bd9d9f4 yes
0e2510dee2 #27913 (fix(rate-limit): stop v3 limiter from leaking internal stash to provider body) 7a462a4220 no, doesn't apply to 1.84.x
acbbfe9cae #29447 (fix: stop use_chat_completions_api flag from leaking into provider request body) 65b6e04da6 no, doesn't apply to 1.84.x
8824745c4e follow-on fix to the #29311 cherry-pick (extract _build_passthrough_logging_result) n/a no, fixes a bug that v1.84.4 also shipped with

One conflict during the first cherry-pick in litellm/proxy/pass_through_endpoints/streaming_handler.py: stable/1.85.x carried the older await litellm_logging_obj.async_success_handler(...) call that the PR rewrites. Initial resolution kept the surrounding endpoint-dispatch if/elif/elif chain inline and replaced only the trailing call with dispatch_success_handlers, which matched v1.84.4's resolution exactly. That turned out to leave the regression test test_route_streaming_logging_runs_async_handler_for_sdk_passthrough broken because it patches PassThroughStreamingHandler._build_passthrough_logging_result, a helper that #29089's design extracts the if/elif/elif chain into but my initial resolution kept inline. v1.84.4 ships with the same broken test.

The follow-on commit 8824745c4e extracts the helper exactly as #29089 designed it, restoring the testable shape. _route_streaming_logging_to_handler now resolves (standard_logging_response_object, kwargs) through the helper and dispatches via dispatch_success_handlers. Verified locally: tests/pass_through_unit_tests/test_unit_test_streaming.py passes 5/5 and tests/test_litellm/litellm_core_utils/test_litellm_logging.py passes 83/83.

The remaining four cherry-picks applied with auto-merges only, no manual conflict resolution.

Type

🐛 Bug Fix

Changes

Three fixes already shipped in v1.84.4 carried into the 1.85.x line, plus two that only apply to the 1.85 line, plus one follow-on fix to the manual conflict resolution that strictly improves on what v1.84.4 shipped.

#29089 / #29311 removes a duplicate Claude Code trace emission caused by the pass-through streaming handler invoking the success-handler path twice. #29343 normalizes a leading Bearer prefix (case-insensitive, per RFC 7235) before classifying an API key as sk- or JWT in UserAPIKeyAuth._safe_hash_litellm_api_key, so the helper produces consistent hashed output regardless of whether the caller stripped the prefix. #29358 switches ResetBudgetJob's batched key/user/team update to a per-row narrow {spend, budget_reset_at} write and removes the pre-zeroing of the cross-pod spend counter, so a single bad row no longer detonates the batch and a failed reset no longer leaves a budget-bypass window.

#27913 stops the v3 rate-limiter from leaking its internal TPM/RPM-reservation stash (_litellm_rate_limit_descriptors, _litellm_tpm_reserved_*) onto the provider request body. #29447 stops the use_chat_completions_api control flag from leaking onto the provider request body via the OpenAI bridge. Both surface as upstream HTTP 400s for unknown fields; both were absent from v1.84.x (different code paths) so the v1.84.4 cherry-pick set didn't include them.

The follow-on commit completes the #29089 cherry-pick by extracting the per-endpoint dispatch chain into _build_passthrough_logging_result, matching the source PR's design and unbreaking its regression test.

First cherry-pick PR against the new stable/1.85.x line branch (created from v1.85.2 earlier today).

mateo-berri and others added 5 commits June 1, 2026 16:22
…9343)

* refactor(proxy/auth): normalize Bearer prefix in safe-hash helper

UserAPIKeyAuth._safe_hash_litellm_api_key now strips a leading
"Bearer "/"bearer " prefix before its existing sk-/JWT classification, so
the helper produces the same hashed output regardless of whether the
caller stripped the Authorization header prefix or passed the header
value through unchanged.

* refactor(proxy/auth): make Bearer-prefix strip case-insensitive

Per RFC 7235 the HTTP authorization scheme token is case-insensitive.
Replace the two-prefix loop with a single case-insensitive check so the
helper normalizes "Bearer ", "bearer ", "BEARER ", and any mixed-case
variant before classifying the remainder as sk- or JWT. The contract
test gains coverage of "BEARER " and "BeArEr ".

* test(mcp): align auth-handler test expectations with safe-hash helper

The two MCP auth tests asserted that UserAPIKeyAuth(api_key="Bearer ...")
retained the raw header bytes on the api_key field. _safe_hash_litellm_api_key
now normalizes that input — stripping the Bearer prefix and hashing the
resulting sk- key — so the expectations move to the normalized form:
the bare token in the parametrize case, and hash_token("sk-...") in the
backward-compat assertion. This matches what the real auth flow produces
(the builder strips Bearer and the DB stores the hashed token), so the
mocks now line up with production rather than with the un-normalized
validator output.
…eroing counter (#29358)

* fix(reset_budget): write only {spend, budget_reset_at} and stop pre-zeroing counter

ResetBudgetJob's batched update_data path shipped the full key/user/team
model on each reset. Prisma rejects object_permission_id and budget_limits
on the update input type, so any row carrying those fields detonated the
entire batch -- spend never reset, budget_reset_at never advanced. After
v1.84.0 started populating object_permission_id on UI-created keys, this
fires routinely.

_reset_budget_common also zeroed the cross-pod spend counter before the
DB write, so failed resets left enforcement reading 0 from the counter
while the DB still held the over-budget spend, admitting requests past
the cap until the counter naturally re-saturated from new reservations.

Switch the write to per-row narrow updates ({spend, budget_reset_at})
via db.batch_, and move the counter invalidation out of
_reset_budget_common so it only fires after the DB write commits. On
DB-write failure the counter is left untouched, enforcement continues
to block, and the next scheduler tick can retry without leaving a
bypass window.

Fixes #27730.

* fix(reset_budget): address Greptile review on #29358

- Strengthen the bypass-half regression test: replace the for-loop over
  call_args_list (vacuously true when empty) with assert_not_called(),
  so the test would actually flag a re-introduction of counter-zeroing
  via any code path.
- Add the same explanatory docstring on _write_user_reset_updates and
  _write_team_reset_updates that _write_key_reset_updates already has,
  so all three helpers point future maintainers at #27730.

* test(reset_budget): update test_proxy_budget_reset for new batch-write path

Same shape as the previous test_reset_budget_job.py update: keys/users/teams
now write through prisma.db.batch_().<table>.update, not update_data, so the
tests need a batcher mock and updated assertions. Adds:

- _wire_batcher_for_test helper that returns a list which accumulates per-row
  batch updates captured from prisma_client.db.batch_().
- _attrify helper that wraps dict fixtures so getattr(item, "token") works
  alongside the dict item-access the fake_reset_* mocks rely on. The new
  narrow-write helpers use getattr to pull out the row's id, and would
  silently skip plain dicts otherwise.
- Updates 3 partial_failure tests to assert against the batch-call list
  (rows by id, payload contains only {spend, budget_reset_at}) instead of
  update_data.assert_awaited_once + data_list inspection.
- Updates test_reset_budget_continues_other_categories_on_failure: only
  budget + enduser still flow through update_data; key/user/team go through
  the batch path now.
- Wires the batcher mock into 3 service_logger_*_success tests so commit()
  is actually awaitable and the success hook fires.

These tests were silently passing locally only because the editable install
in .venv pointed at the main repo, not the worktree — running pytest with
PYTHONPATH overridden to the worktree (matching CI) reproduces the failures.
…der body (#27913)

* fix(rate-limit): stop v3 limiter from leaking internal stash to provider body

PR #27001 (atomic TPM rate limit) introduced a reservation flow that
writes four LiteLLM-internal keys onto the request data dict:

  _litellm_rate_limit_descriptors
  _litellm_tpm_reserved_tokens
  _litellm_tpm_reserved_model
  _litellm_tpm_reserved_scopes
  _litellm_tpm_reservation_released

These keys are forwarded as request body params to the upstream provider,
which rejects them as unknown fields:

  OpenAI    -> 400 'Unknown parameter: _litellm_rate_limit_descriptors'
              (mapped by litellm to RateLimitError / 429, hiding the bug
               behind a misleading 'throttling_error' code)
  Anthropic -> 400 '_litellm_rate_limit_descriptors: Extra inputs are
               not permitted'

Net effect: every chat completion against any real provider fails the
moment a virtual key has any tpm_limit / rpm_limit set — i.e. v3-enforced
key-level TPM/RPM limits are broken end-to-end. The v3 RPM/TPM check
itself still runs (raises 429 on over-limit), but the success path
poisons the upstream body.

Reproduced on litellm_internal_staging HEAD (410ce76) against
gpt-4o-mini and claude-haiku-4-5 with a 1-RPM/1-TPM key — first request
fails with the provider's unknown-field error.

Fix: the stash is metadata only.

  - Add RATE_LIMIT_DESCRIPTORS_KEY constant and a _LITELLM_STASH_KEYS
    registry so we have a single source of truth for stash keys.
  - New helper _stash_value_in_metadata_channels writes to
    data['metadata'] / data['litellm_metadata'] without touching the
    top level.
  - _stash_reservation_in_data and the descriptor stash now route
    through that helper. _mark_reservation_released stops writing
    top-level.
  - _lookup_stashed_value also checks kwargs['metadata'] /
    kwargs['litellm_metadata'] (raw request_data shape) in addition to
    kwargs['litellm_params']['metadata'] (completion kwargs shape).
  - async_post_call_failure_hook now reads descriptors via the unified
    metadata lookup instead of request_data.get(top-level).
  - Defense in depth: async_pre_call_hook strips any stash key that
    somehow surfaced at the top level (stale cache, future refactor,
    test fixture) before returning.

Tests:
  - New regression test asserts no _litellm_* stash key is present at
    the top level of data after async_pre_call_hook, and that the
    metadata channel still carries the reservation + descriptors so
    success / failure reconciliation works.
  - Existing test_tpm_concurrent.py tests that asserted top-level
    presence are updated to read from data['metadata'] — the location
    is an implementation detail; the spec is that post-call callbacks
    can resolve the stash.

Verified end-to-end against OpenAI gpt-4o-mini and Anthropic
claude-haiku-4-5 via /v1/chat/completions on a low-rpm key:

  - With limits not exceeded: HTTP 200, valid completion response,
    no leaked fields in body.
  - With RPM exceeded: HTTP 429 from v3 enforcement
    ('Rate limit exceeded ... Limit type: requests').
  - With TPM exceeded: HTTP 429 from v3 enforcement
    ('Rate limit exceeded ... Limit type: tokens').

Full v3 hook test suite passes (171 tests).

Co-authored-by: Mateo Wang <[email protected]>

* chore(rate-limit): use RATE_LIMIT_DESCRIPTORS_KEY constant in test, trim noisy comments

Address greptile P2: test fixture now uses the imported constant.
Drop comments that re-explain what well-named identifiers already convey.

* fix(rate-limit): reject caller-supplied stash values to prevent TPM-refund abuse

Strip _LITELLM_STASH_KEYS from data top-level and both metadata channels at
the start of async_pre_call_hook. Without this, an authenticated caller can
inject _litellm_rate_limit_descriptors plus _litellm_tpm_reserved_tokens in
body metadata, trigger a proxy-side rejection, and cause
async_post_call_failure_hook to refund TPM counters against attacker-named
scopes (e.g. another tenant's api_key).

---------

Co-authored-by: Cursor Agent <[email protected]>
Co-authored-by: Mateo Wang <[email protected]>
…quest body (#29447)

* fix: stop use_chat_completions_api flag from leaking into provider request body

use_chat_completions_api is a LiteLLM control flag that forces the
/responses -> /chat/completions bridge. It was missing from
all_litellm_params, so get_non_default_completion_params treated it as a
model-specific param and forwarded it to the upstream provider. A
model-level "use_chat_completions_api: true" in the proxy config therefore
reached the chat-completions path and was rejected by strict providers
(OpenAI/Anthropic) with HTTP 400 for an unknown body field.

Register it as a known internal param so it is stripped on every path
(completion, the responses bridge that calls litellm.completion, and
filter_out_litellm_params).

Adds a regression test driving litellm.completion() with a mocked OpenAI
client that asserts the flag never reaches the request body.

* test: clarify extra_body assertion in use_chat_completions_api leak test

Replace the misleading 'not in ... or {}' precedence idiom with an explicit
parenthesized guard that also handles extra_body being None.
@yuneng-berri yuneng-berri requested a review from a team June 2, 2026 00:16
@yuneng-berri

Copy link
Copy Markdown
Collaborator Author

Replaces #29458. Branch rename from worktree-cherrypick-1-85-xlitellm_cherrypick_1_85_x auto-closed the original PR; reopening was rejected by the API, so this is a fresh PR with identical content. Greptile review on #29458 covered the diff; full ship-readiness audit including 5 Opus subagent reviews is captured in the conversation thread.

@codecov

codecov Bot commented Jun 2, 2026

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This cherry-pick PR backports five bug fixes onto stable/1.85.x: a duplicate-trace fix for Claude Code pass-through streaming (#29311), Bearer-prefix normalization in _safe_hash_litellm_api_key (#29343), a narrow per-row Prisma write for budget resets that eliminates a bypass window (#29358), prevention of internal rate-limiter stash keys from leaking into provider request bodies (#27913), and registering use_chat_completions_api in all_litellm_params so it is never forwarded upstream (#29447).

Confidence Score: 4/5

Safe to merge; all five fixes are narrow and well-tested. The deferred stream logging refactor is the most complex change but is covered by new unit tests.

The budget reset fix correctly moves Redis counter zeroing to after the DB commit and adds previously-missing user-counter invalidation. The rate-limiter stash-key fix closes a real injection path and is backed by two new regression tests. The dispatch_success_handlers refactor is internally consistent but relaxes the should_run_logging double-log guard for assembled stream responses, creating a latent risk if async_success_handler is called directly with such a result in future code. The budget_reset_at field is written back unchanged for keys with no budget_duration, causing harmless redundant reset cycles but not data loss.

litellm/litellm_core_utils/litellm_logging.py and litellm/proxy/common_utils/reset_budget_job.py

Important Files Changed

Filename Overview
litellm/litellm_core_utils/litellm_logging.py Introduces dispatch_success_handlers, _is_assembled_stream_success, and _is_sync_litellm_request; adds has_dispatched_final_stream_success dedup guard for streaming; modifies async_success_handler to bypass the should_run_logging guard for assembled stream responses.
litellm/litellm_core_utils/streaming_handler.py Per-chunk sync logging is now conditional on _is_sync_litellm_request; assembled-stream dispatch switches from async_success_handler + executor.submit to dispatch_success_handlers(prefer_async_handlers=True).
litellm/proxy/_types.py Adds case-insensitive Bearer prefix stripping in _safe_hash_litellm_api_key before sk-/JWT classification; the normalized (stripped) key is hashed or returned.
litellm/proxy/common_utils/reset_budget_job.py Replaces bulk update_data(update_many) with narrow per-row prisma.db.batch_().update({spend, budget_reset_at}); removes pre-zeroing of Redis spend counters from _reset_budget_common and adds post-commit _invalidate_spend_counter calls in each caller; also adds previously-missing user spend counter invalidation.
litellm/proxy/hooks/parallel_request_limiter_v3.py Stops stash keys from living at the top-level of data; introduces _stash_value_in_metadata_channels, _strip_stash_keys_from_top_level/all_channels; _lookup_stashed_value now only reads metadata channels (not top-level kwargs); RATE_LIMIT_DESCRIPTORS_KEY is stored and read from metadata.
litellm/types/utils.py Adds use_chat_completions_api to all_litellm_params so it is filtered from provider request bodies.

Comments Outside Diff (1)

  1. litellm/proxy/common_utils/reset_budget_job.py, line 486-492 (link)

    P2 Empty batch committed when all IDs are None

    _write_key_reset_updates (and its user/team siblings) calls batcher.commit() unconditionally after iterating updated_keys. If every item in the list has token=None (all skipped by the continue), the commit fires against an empty batch. Prisma's batch API handles this gracefully in most drivers, but the behaviour is driver-version-dependent and will silently skip all writes with no error. The outer if updated_keys: guard only ensures the list is non-empty — it doesn't guarantee any items have a non-None identifier.

Reviews (1): Last reviewed commit: "fix: stop use_chat_completions_api flag ..." | Re-trigger Greptile

Comment on lines +435 to +438
batcher.litellm_verificationtoken.update(
where={"token": token},
data={"spend": 0, "budget_reset_at": k.budget_reset_at},
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 budget_reset_at written back as-is when budget_duration is None

When a key has no budget_duration, _reset_budget_common does not update budget_reset_at, so k.budget_reset_at still holds the old (expired) timestamp. _write_key_reset_updates then writes {"spend": 0, "budget_reset_at": <old-expired-value>} back to the DB. On the next job run the key will appear in keys_to_reset again (because budget_reset_at <= now), triggering a harmless but redundant reset cycle. Guarding the field with a non-None check prevents writing a stale timestamp.

Suggested change
batcher.litellm_verificationtoken.update(
where={"token": token},
data={"spend": 0, "budget_reset_at": k.budget_reset_at},
)
batcher.litellm_verificationtoken.update(
where={"token": token},
data={
"spend": 0,
**(
{"budget_reset_at": k.budget_reset_at}
if k.budget_reset_at is not None
else {}
),
},
)

Comment on lines +2565 to 2570
if not self._is_assembled_stream_success(
result
) and not self.should_run_logging(
event_type="async_success"
): # prevent double logging
): # prevent double logging (non-streaming)
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 should_run_logging guard bypassed for all assembled stream calls to async_success_handler

async_success_handler previously checked should_run_logging unconditionally to prevent double logging. The new condition skips that guard whenever _is_assembled_stream_success returns True, relying on the has_dispatched_final_stream_success flag set by dispatch_success_handlers. Any call site that invokes async_success_handler directly with an assembled stream result (bypassing dispatch_success_handlers) will now skip the double-log guard with no fallback. The comment should at minimum make this assumption explicit so future callers don't inadvertently create double-logging regressions.

Suggested change
if not self._is_assembled_stream_success(
result
) and not self.should_run_logging(
event_type="async_success"
): # prevent double logging
): # prevent double logging (non-streaming)
return
# For assembled stream results, dedup is enforced by the
# ``has_dispatched_final_stream_success`` guard in
# ``dispatch_success_handlers``. Always route assembled stream
# responses through ``dispatch_success_handlers`` — calling
# ``async_success_handler`` directly with an assembled result
# bypasses that guard and may produce duplicate log events.
if not self._is_assembled_stream_success(
result
) and not self.should_run_logging(
event_type="async_success"
): # prevent double logging (non-streaming)
return

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

The #29311 cherry-pick onto stable/1.85.x carried the test
test_route_streaming_logging_runs_async_handler_for_sdk_passthrough,
which patches PassThroughStreamingHandler._build_passthrough_logging_result
to verify the SDK-passthrough dispatch contract. The manual conflict
resolution kept the per-endpoint if/elif/elif chain inline in
_route_streaming_logging_to_handler (matching v1.84.4's resolution),
so the patched attribute did not exist and the test errored at
collection with AttributeError.

Extract the chain into the static _build_passthrough_logging_result
helper as #29089 originally designed it. _route_streaming_logging_to_handler
now resolves (standard_logging_response_object, kwargs) through the
helper and dispatches via dispatch_success_handlers; the helper itself
is synchronous and CPU-bound, suitable for the unit test's patch target.

Verified locally: tests/pass_through_unit_tests/test_unit_test_streaming.py
passes (5/5) and tests/test_litellm/litellm_core_utils/test_litellm_logging.py
passes (83/83).

v1.84.4 ships with the same broken test; this strictly improves on that
resolution.

@mateo-berri mateo-berri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; thanks!

@mateo-berri mateo-berri merged commit 6a026ee into stable/1.85.x Jun 2, 2026
67 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants