fix(ui): scope key models dropdown options to the key's team - #32382
Conversation
Greptile SummaryThis PR fixes a UI bug where the key create and edit forms offered the wrong "all models" sentinel option based on team context — teamless keys were offered
Confidence Score: 5/5Safe to merge — the changes are UI-only, narrowly scoped to the key create/edit model dropdown, and every sentinel-routing path is covered by component tests and e2e tests. The two previously flagged blockers are resolved: No files require special attention. The e2e test file (
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/key_team_helpers/fetch_available_models_team_key.tsx | Adds excludeProxyWideSentinel (strips all-proxy-models from a model list) and hasAllModelsSentinel (detects either sentinel) as shared helpers used by both form components. |
| ui/litellm-dashboard/src/components/organisms/create_key_button.tsx | Applies excludeProxyWideSentinel to merged team models, adds hardcoded "All Proxy Models" option for teamless keys (hidden when a team is selected), and disables individual model options once a sentinel is chosen via Form.useWatch. |
| ui/litellm-dashboard/src/components/templates/key_edit_view.tsx | Applies excludeProxyWideSentinel in both teamless and team model-fetch paths, guards "All Team Models" option behind team != null (fixing the loading-state regression), and collapses sentinel selections to a single value in the onChange handler. |
| ui/litellm-dashboard/src/components/organisms/create_key_button.test.tsx | Adds "models dropdown team gating" describe block with two new tests; updates team-dropdown mock to pass full team objects via onTeamSelect; switches fetch_available_models_team_key mock to importActual so real helper functions are exercised. |
| ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx | Adds six new tests covering all four team-context sentinel rules (teamless/team, show/hide), a deduplication guard, sentinel collapsing, and a regression test for the loading-state guard (teams=[] yields no sentinel). |
| tests/e2e/management/test_key_models_dropdown_e2e.py | New Playwright-based e2e suite covering all four dropdown scenarios end-to-end; sentinel-specific assertions and model creation/verification flow are well-structured, but correctness relies on the test proxy serving the gpt-5.5 model. |
| tests/e2e/management/conftest.py | Adds optional browser (session-scoped) and ui_page (function-scoped) fixtures using importorskip so API tests continue to run without Playwright installed. |
| tests/e2e/docker-compose.yml | Adds UI_USERNAME / UI_PASSWORD env vars to the proxy service so browser tests can log in; supports an overridable LITELLM_E2E_IMAGE for testing locally-built UI images. |
| tests/e2e/e2e_config.py | Exports UI_USERNAME / UI_PASSWORD driven by E2E_UI_USERNAME / E2E_UI_PASSWORD env vars, defaulting to the same credentials as the compose stack. |
Reviews (4): Last reviewed commit: "fix(e2e): inline PROXY_BASE_URL/ui after..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
A teamless key no longer offers the all-team-models option in the create and edit forms; the backend expands that sentinel to the full proxy model list when no team is attached, which is rarely what the user intended. A team key no longer surfaces the all-proxy-models sentinel that leaks in verbatim when the team's own model list carries it; the dropdown keeps All Team Models plus the team's individual models. Adds browser coverage to the management e2e suite: playwright (an optional dependency behind importorskip) drives the proxy-served dashboard at /ui, asserts the dropdown options a real user sees for teamless and team keys on both create and edit, and walks the create modal end to end, reading the persisted key back through /key/info.
bba46a0 to
cebf39d
Compare
Merging this PR will improve performance by 13.27%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
A teamless key has no team allowlist to inherit, so the dropdown now offers All Proxy Models in place of All Team Models on both the create and edit forms, with the same exclusive-selection handling. Component and browser e2e tests updated to pin the swapped option pair; the teamless create case now also walks the modal end to end and reads the persisted key back through /key/info.
The create modal no longer offers All Team Models without a team; the teamless path now offers All Proxy Models, which is what this spec exercises
When a key has a team_id but the teams prop does not yet include the matching team, availableModels stays empty and the models dropdown rendered All Team Models on its own with nothing to compare against. Gate the option on the team object being present so it only appears once team models are known, and add a regression test for the loading state Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Mubashir Osmani seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
On the confidence-score note about Pushed 57ba070 to address the P2 on key_edit_view.tsx (gating |
|
Thanks for the clarification on the mock strategy — the antd shim rendering 57ba070 also looks correct: gating on the resolved Tip: You can customize Greptile's behavior for this repo with |
…t form The teamless fetch path stored modelAvailableCall results without excludeProxyWideSentinel, so an all-proxy-models entry in the response rendered a second option colliding with the hardcoded All Proxy Models sentinel. Apply the same filter used on the team path and add a regression test asserting the sentinel option is not duplicated Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Addressed the remaining item from the 4/5 summary in 46e2f2c. The teamless fetch path in key_edit_view.tsx now runs |
|
The fix is solid. Looking at the
All three points from the original feedback are addressed. Looks good to me. |
mateo-berri
left a comment
There was a problem hiding this comment.
the edit form's onChange is a plain pass-through, so a user can submit a mixed list like ["all-proxy-models", "gpt-4"]. The PR's own design intent (visible in the create-form handler it adds) is exclusive selection, making this an oversight rather than a deliberate trade-off
Is this a valid concern?
|
bugbot run |
|
@coderabbitai review (just using coderabbit to compare with bugbot. Don't treat its comments as blocking) |
…dit form The edit form models Select had a pass-through onChange, so a user could submit a mixed list like ["all-proxy-models", "gpt-4"] even though the sentinels are meant to be exclusive. Mirror the create form and collapse the selection to the sentinel when all-proxy-models or all-team-models is picked; add a regression test that selects a model plus the proxy sentinel and asserts the submitted models list is just ["all-proxy-models"] Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 32382 --repo BerriAI/litellm --json files,additions,deletions,changedFiles -q '.files[].path'
echo "---DIFF---"
gh pr diff 32382 --repo BerriAI/litellm > /tmp/pr.diff
wc -l /tmp/pr.diffLength of output: 1041 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,800p' /tmp/pr.diffLength of output: 36067 The PR description image (3:18:24 PM) shows a "Key Ownership" modal with no team selected (Team field is blank, placeholder "Search or select a team"). The Models dropdown is open showing: "All Proxy Models", "gpt-5.5", "claude-haiku-4-5", "gemini-2.5-flash", "openai-text-embedding-3-small", "e2e-vs-chat-590f68c6195b", "e2e-vs-chat-8ec751388f40" — notably no "All Team Models" option. The PR description image (3:12:19 PM) shows the same modal with Team set to "toes (d68bf164-e893-4bb8-ac6b-1b27472c0cac)". The Models dropdown now shows "All Team Models" (highlighted), "gemini-2.5-flash", "gpt-5.5", "openai-text-embedding-3-small" — notably no "All Proxy Models" option, and a "Create Key" button is visible.
Core logic (
Tests
Minor observations (non-blocking, per your note)
No blocking issues found from my side. ✅ Action performedReview finished.
|
|
@mateo-berri re "Is this a valid concern?" on the exclusive-selection point: yes, it's valid. The edit form's models |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 46e2f2c. Configure here.
📝 WalkthroughWalkthroughThis PR updates key create/edit model dropdown handling to distinguish proxy-wide and team-wide sentinel options, adds management Playwright fixtures and docs, and adds browser coverage for team-scoped key model selection. ChangesModels dropdown team gating
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/management/conftest.py`:
- Around line 45-55: The ui_page fixture can leak a browser context if any setup
step before yield fails, because context.close() is only reached on the happy
path. Update ui_page to ensure the context is always closed even when page.goto,
page.fill, page.click, or page.wait_for_url raises by wrapping the setup/yield
flow in a try/finally or equivalent cleanup pattern. Keep the fix localized to
ui_page and preserve the existing browser.new_context, new_page, and post-yield
cleanup behavior.
In `@tests/e2e/management/test_key_models_dropdown_e2e.py`:
- Around line 22-24: The E402 suppression on the import in the Playwright test
needs an explicit reason comment. Update the `from playwright.sync_api import
Locator, Page, expect` line in `test_key_models_dropdown_e2e.py` so the `noqa`
includes a short justification for the deferred import, and keep the
`pytest.importorskip` guard in place to preserve the current test setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 16a9cf62-0ffd-4edc-ab02-110d855b6efb
📒 Files selected for processing (13)
tests/e2e/CLAUDE.mdtests/e2e/CONTRIBUTING.mdtests/e2e/coverage_registry/mgmt.yamltests/e2e/docker-compose.ymltests/e2e/e2e_config.pytests/e2e/management/conftest.pytests/e2e/management/test_key_models_dropdown_e2e.pyui/litellm-dashboard/e2e_tests/tests/proxy-admin/keys.spec.tsui/litellm-dashboard/src/components/key_team_helpers/fetch_available_models_team_key.tsxui/litellm-dashboard/src/components/organisms/create_key_button.test.tsxui/litellm-dashboard/src/components/organisms/create_key_button.tsxui/litellm-dashboard/src/components/templates/key_edit_view.test.tsxui/litellm-dashboard/src/components/templates/key_edit_view.tsx
…t noqa Wrap the ui_page fixture setup in try/finally so the Playwright context is closed even if login (goto/fill/click/wait_for_url) raises before yield, and add a reason to the E402 suppression on the deferred playwright import per repo lint conventions Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
shouldnt the subsets be disabled if either "all-proxy-models" or "all-team-models" are selected? |
…el is selected Both key create and edit forms let a user pick individual models even after choosing All Proxy Models / All Team Models; the onChange collapse then silently dropped those picks. Add a shared hasAllModelsSentinel helper and disable the individual model options in both forms whenever a sentinel is selected, so the mutual exclusivity is visible rather than corrected after the fact. Adds an edit-form regression test asserting the individual options become disabled once the proxy sentinel is picked Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@mubashir1osmani re "shouldnt the subsets be disabled if either all-proxy-models or all-team-models are selected?" — good call, done in fcc37ee. Both the create and edit forms now disable the individual model options whenever |
Commit 3827afa removed the UI_BASE_URL definition from e2e_config.py but left it imported and used in the management e2e conftest and key-models dropdown test, so those modules raised ImportError on collection. Inline f"{PROXY_BASE_URL}/ui..." at the call sites, keeping the UI_BASE_URL symbol removed as intended. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
|
@coderabbitai review |
✅ Action performedReview finished.
|
a05a1ee
into
litellm_internal_staging
Relevant issues
Linear ticket
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
no team attached

team attached

Screenshots from a live proxy follow in the comments. To reproduce on a dashboard built from this branch, against a proxy on
http://localhost:4000with master keysk-1234and a team whose models includeall-proxy-models:/ui/api-keys/?create=true)All Proxy Modelsis offered,All Team Modelsis notAll Team Modelsand the team's individual models are offered,All Proxy Modelsis not, even though the team's own model list carries itAll Proxy Modelsis offered,All Team Modelsis notAll Team Modelsis offered,all-proxy-modelsis notThe new browser e2e tests walk the same steps headlessly against the proxy-served UI, submit the create modal with the offered sentinel for both the teamless and team paths, and read the created keys back through
/key/info:They read
LITELLM_PROXY_URL(defaulthttp://localhost:4000) and need the proxy to serve a dashboard containing this change, so either a proxy whose bundled UI was rebuilt from this branch or the compose stack pointed at a locally built image (docker build -t litellm-local .thenLITELLM_E2E_IMAGE=litellm-local docker compose up -dfromtests/e2e/)Type
🐛 Bug Fix
Changes
The key create and edit forms offered Models dropdown options that do not match the key's team context. A teamless key was offered
all-team-models, which the backend expands to the full proxy model list when no team is attached, so picking it silently created an unrestricted key. A team key was offeredall-proxy-modelswhenever the team's own model list carried that sentinel, because both forms mergeteam.modelsinto the pickable list verbatimThe create modal (
create_key_button.tsx, also mounted by the newer api-keys page) and the edit form (key_edit_view.tsx) now renderAll Team Modelsonly when the key has a team, and offerAll Proxy Modelsin its place when it does not, so a teamless key can still be granted every proxy model explicitly. A sharedexcludeProxyWideSentinelhelper filtersall-proxy-modelsout of the merged team model list in both forms; the team's individual models stay selectable so a key can still be scoped to a subset.no-default-modelsis deliberately not filtered because the create form uses its presence to force team selectionComponent tests in
create_key_button.test.tsxandkey_edit_view.test.tsxpin the option pair for both team contexts on both forms, with assertions scoped to the models select; each rule was verified to fail with the gate or the filter revertedThe
tests/e2e/management/suite gains browser coverage on the shared harness: playwright logs into the dashboard the proxy serves at/ui, asserts the dropdown options a user actually sees for teamless and team keys on both create and edit, and walks the create modal end to end for both paths, submitting the offered sentinel and asserting through/key/infothat the key persisted with the rightmodelsandteam_id. The suite's conftest gains browser and logged-in page fixtures; playwright stays out of the project dependencies as an optional install behindimportorskip(same pattern as pipecat in the realtime suite), so the suite's API tests collect and run without it.tests/e2e/docker-compose.ymlgains an overridable proxy image (LITELLM_E2E_IMAGE) plus pinned UI credentials, the coverage registry gains amgmt.key.update.happy_pathUI row, and the install steps are documented intests/e2e/CONTRIBUTING.mdNote
Medium Risk
Changes how API key model scope is chosen in the dashboard, which affects authorization breadth; backend rules are unchanged but mis-selection previously could grant wider access than intended.
Overview
Fixes the virtual-key Models dropdown on create and edit so the “all models” sentinel matches whether the key has a team.
Teamless keys now offer All Proxy Models (
all-proxy-models) and no longer show All Team Models, which could expand to the full proxy list without a team. Team keys offer All Team Models plus the team’s models, whileall-proxy-modelsis stripped from the merged list viaexcludeProxyWideSentineleven when the team’s model list includes that sentinel. Picking a sentinel collapses the selection to that value alone and disables individual model options while it is selected.Coverage adds component tests, a Playwright management e2e suite (optional dependency, logged-in
/uifixtures), compose UI credentials, and an updated dashboard e2e spec for teamless create.Reviewed by Cursor Bugbot for commit ed0e718. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
/ui.Tests