fix(team callbacks): return callbacks registered via POST from GET /team/{team_id}/callback - #34991
Conversation
…eam/{team_id}/callback
get_team_callbacks only read metadata.callback_settings, while add_team_callbacks writes to metadata.logging, so any callback registered through the API came back as an empty list even though the UI and request-time resolution both showed it. Resolve both slots, redact credential-bearing callback_vars, and have disable_team_logging clear the logging slot it left behind.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| ) | ||
| resolved = reduce( | ||
| lambda acc, callback: convert_key_logging_metadata_to_callback(data=callback, team_callback_settings_obj=acc), | ||
| callbacks, | ||
| base, | ||
| ) |
There was a problem hiding this comment.
GET diverges from runtime precedence
When a team has both callback_settings and logging, _resolve_team_callbacks returns their union while request-time resolution selects logging and skips callback_settings, causing GET to report callbacks and variables that requests never execute.
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!
Greptile SummaryThis PR makes team callback GET responses resolve both stored callback formats, redacts sensitive callback variables, and clears API-created callback entries when logging is disabled.
Confidence Score: 3/5This PR should not merge until GET uses the same callback-source precedence as request-time execution and disabling clears legacy callback variables. Mixed-format teams are reported as a union even though runtime selects one metadata slot, and disabling a legacy configuration leaves callback variables visible through the changed GET path. Files Needing Attention: litellm/proxy/management_endpoints/team_callback_endpoints.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/team_callback_endpoints.py | Adds callback resolution and disable cleanup, but GET no longer matches request-time source precedence and legacy variables survive disabling. |
| tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py | Adds useful regression coverage but codifies union semantics and does not cover legacy callback variables after disabling. |
Comments Outside Diff (1)
-
litellm/proxy/management_endpoints/team_callback_endpoints.py, line 388-393 (link)Disabled callbacks retain stale variables
When a legacy team has values in
callback_settings.callback_vars, this endpoint clears only the callback lists and persists those values, causing the changed GET path to continue returning stale callback configuration after logging is disabled.
Reviews (1): Last reviewed commit: "fix(team callbacks): return callbacks re..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Closing in favor of #35512, which resolves LIT-5093 with live-proxy evidence. The one substantive difference worth recording: this PR's resolver merges Thanks for the original diagnosis; the metadata-slot mismatch you identified was correct and is what #35512 fixes. |
|
Agreed on the precedence point, and thanks for catching it. One thing to make sure doesn't get dropped: this PR also cleared the |
TLDR
Problem this solves:
GET /team/{team_id}/callbackalways returned an empty listdisable_team_loggingleft the slot the proxy actually reads populatedHow it solves it:
metadata.loggingandmetadata.callback_settingscallback_varscome back redacted, not ciphertextloggingentries tooRelevant issues
Linear ticket
Resolves LIT-4886
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy on localhost:4000 (
python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml), Postgres in docker, master keysk-1234Setup, run identically before and after:
Before, at b930e2f (parent of this branch). Both POSTs return 200, the returned team row carries the callbacks under
metadata.logging, andGET /team/infoshows the same, yet the callback endpoint reports nothing:After, at 6f4dabe:
Disabling logging now empties the list; before the fix the
loggingslot survived the disable, so the team kept logging:Type
🐛 Bug Fix
Changes
Team callback config lives in two metadata slots.
add_team_callbacksappendsAddTeamCallbackentries tometadata.logging, which is also what request-time resolution inlitellm_pre_call_utilsand the Admin UI read;metadata.callback_settingsholds the olderTeamCallbackMetadatashape used by config-driven setups and bydisable_team_logging.get_team_callbacksread only the latter, so every callback registered through the API was invisible to the API_resolve_team_callbacksfolds both slots into oneTeamCallbackMetadata, reusingconvert_key_logging_metadata_to_callbackso the GET reports exactly what the proxy will run, and skipping malformedloggingentries the way request-time resolution already does:callback_varsare decrypted only so sensitive keys can be reported as***REDACTED***rather than aslitellm_enc::...ciphertext; non-secret vars such aslangsmith_projectorgcs_bucket_namestill come back usabledisable_team_loggingalso clearsmetadata.logging. It previously reset onlycallback_settings, so a team configured through the API kept logging after a disable, and with this GET fix would also keep reporting those callbacksFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/1838faf8459a4796abd976e2eb680285