test(e2e): user budget across keys and team member budget isolation - #33712
test(e2e): user budget across keys and team member budget isolation#33712yassin-berriai wants to merge 2 commits into
Conversation
Fill the two live-coverage gaps in the budgets e2e suite. The internal-user enforcement test drove a single key, so it never proved a user budget is shared across that user's keys; the team-member enforcement test used one member, so it never proved isolation between members. Adds test_user_budget_across_keys_e2e.py: one user with a tiny max_budget owns two keys, neither with a budget of its own; spending the first to a budget_exceeded block makes the untouched second key reject the same way, and the recorded user spend crosses the cap. Adds test_team_member_budget_isolation_e2e.py: two members on one team, the tight member driven past its per-team cap while the roomy member keeps serving and the tight member stays blocked. Extends budget_client with a /user/info spend read, adds the two registry cells, and widens the quota grammar with the isolates_per_member and enforced_across_keys assertions. Resolves LIT-4548
|
|
Greptile SummaryThis PR closes two gaps in the budget e2e suite by adding tests for user budget sharing across keys and per-team-member budget isolation, along with the supporting registry and grammar updates.
Confidence Score: 5/5Safe to merge — purely additive test and registry changes with no production code touched. All five changed files are under tests/e2e/ and the coverage registry. No production logic is modified. The new test files follow established harness conventions (typed Pydantic bodies, Result-tagged-union handling, ResourceManager cleanup, unique_marker isolation), the spend-poll deadline is generous, and the registry cells match the naming grammar. The PR description includes live proof-of-pass against a real proxy stack with raw curl output confirming the gateway behavior being tested. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/quota_management/budgets/test_user_budget_across_keys_e2e.py | New e2e test proving per-user max_budget is enforced across all keys; test logic, fixture setup, and spend-poll deadline are sound |
| tests/e2e/quota_management/budgets/test_team_member_budget_isolation_e2e.py | New e2e test for per-team-member budget isolation; fixture, log-polling assertion, and tight/roomy member split are correctly structured |
| tests/e2e/quota_management/budgets/budget_client.py | Adds UserInfoParams/UserInfoRow/UserInfoResponse models and user_info() method; follows existing patterns for typed GET helpers |
| tests/e2e/coverage_registry/quota_management.yaml | Two new registry cells added (enforced_across_keys, isolates_per_member); IDs, tiers, and rationale are consistent with the existing entries |
| tests/e2e/CLAUDE.md | Naming grammar extended with isolates_per_member and enforced_across_keys assertion tokens; consistent with how existing tokens are listed |
Reviews (5): Last reviewed commit: "test(e2e): annotate _call return type in..." | Re-trigger Greptile
| """Live e2e: a per-user max_budget is enforced across ALL of that user's keys. | ||
|
|
||
| An internal user's budget governs every personal key it owns, not only the one | ||
| that happened to spend it down. One user with a tiny max_budget owns two keys: | ||
| driving the first key to a budget_exceeded block then makes a fresh, untouched | ||
| second key of the same user (which carries no budget of its own, so nothing but the | ||
| shared user budget can block it) reject the same way, and the user's recorded spend | ||
| has crossed the cap. A key-scoped-only budget would leave the second key serving. |
There was a problem hiding this comment.
PR description does not match the diff
The PR description (and attached screenshots/runbook) describes a new tests/e2e/mcp/ suite testing MCP authorization via test_mcp_key_access_e2e.py, with Docker compose bring-up steps, MCP server registration, and references to KeyGenerateBody.object_permission. None of those files or changes are present in this diff. The actual changes are entirely budget-related (test_user_budget_across_keys_e2e.py and test_team_member_budget_isolation_e2e.py). The mismatch makes it difficult to verify the pre-submission checklist and understand the intent of the PR during review.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds two new live e2e tests under
Confidence Score: 4/5Safe to merge — the change is entirely additive new e2e tests with no production code touched. The test logic for both isolation scenarios is sound: fixture teardown is guarded with try/finally, lambda captures are not in loops so closure semantics are correct, and the spend-log assertions use the right predicate. The only nit is a missing return type annotation on the private tests/e2e/quota_management/budgets/test_user_budget_across_keys_e2e.py — the
|
| Filename | Overview |
|---|---|
| tests/e2e/quota_management/budgets/test_user_budget_across_keys_e2e.py | New e2e test proving user max_budget is enforced across all keys; logic is correct but the private helper _call is missing an explicit return type annotation, inconsistent with strict basedpyright mode and all other helpers in the suite. |
| tests/e2e/quota_management/budgets/test_team_member_budget_isolation_e2e.py | New e2e test for per-team-member budget isolation; fixture teardown, lambda captures, and spend-log assertions are all correct. |
| tests/e2e/quota_management/budgets/budget_client.py | Adds UserInfoParams, UserInfoRow, UserInfoResponse models and the user_info helper method; correctly uses the transport layer and follows existing patterns. |
| tests/e2e/coverage_registry/quota_management.yaml | Adds two new coverage registry entries for the new tests; IDs match the @pytest.mark.covers markers in the test files. |
| tests/e2e/CLAUDE.md | Expands the assertion vocabulary in the quota_management naming grammar with isolates_per_member and enforced_across_keys; change is accurate and self-consistent. |
Reviews (2): Last reviewed commit: "test(e2e): user budget across keys and t..." | Re-trigger Greptile
|
@greptileai addressed both findings: corrected the PR description (it now describes this budgets diff, the earlier body was mistakenly the wrong content) and added the explicit return type annotation on _call. Please re-review. |
|
@greptileai the return-type fix is on HEAD 9c878f2 now (the prior review raced the push). Please re-review the latest commit. |
|
@greptileai please review the current head 9c878f2 |
|
Closing in favor of #33745, which supersedes this with the same two tests plus a tightened second-key assertion (block the untouched key promptly rather than driving it to accumulate its own spend) and corrected PR framing. Continuing the work there |
Pull request was closed
Relevant issues
Linear ticket
Resolves LIT-4548
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
All proof captured at commit
ab4392cd36against a live docker-compose proxy stack (ghcr.io/berriai/litellm:main-latest, real OpenAI calls, real spend). The chat model isgpt-5.5because that is the model with a working provider key on this stack; the existing budget suite uses the same literal per its coverage matrix noteBoth new tests passing:
R4 money assertion, raw curl (a per-user budget blocks a second, untouched key of the same user):
R2 money assertion, raw curl (a blocked member does not block a peer on the same team):
Stack brought up namespaced so it does not collide with concurrent suites:
Registry collector accepts the two new cells:
Type
✅ Test
Changes
The budgets e2e suite had two contracts asserted only halfway. The internal-user enforcement test (
test_budget_enforcement_e2e.py) drove a single key, so it proved a user budget blocks that key but never that the budget is shared across all of the user's keys. The team-member enforcement test (test_team_member_budget_e2e.py) used one member, so it proved a member cap blocks that member but never that members are isolated from each othertest_user_budget_across_keys_e2e.pycloses the first gap. One internal user gets a tinymax_budgetand owns two keys, neither carrying a budget of its own. The test spends the first key to abudget_exceededblock, then the untouched second key is rejected the same way; since the second key has no budget of its own, only the shared user budget can block it. It then polls/user/infoand asserts the recorded user spend has crossed the cap, so both the recorded state and the enforced gateway behavior are checkedtest_team_member_budget_isolation_e2e.pycloses the second. Two members share one team with a large team budget; the tight member is capped at a tiny per-team budget and driven past it. Once the tight member is blocked, the roomy member still serves on the same team, its calls land in the spend logs attributed to its own user id, and the tight member stays blocked, proving one member's exhausted budget does not leak onto another'sSupporting changes:
budget_client.pygains a typed/user/infospend read;coverage_registry/quota_management.yamlgains thequota_management.budget.internal_user.enforced_across_keysandquota_management.budget.team_member.isolates_per_membercells; and the Quota Management naming grammar intests/e2e/CLAUDE.mdis widened with theenforced_across_keysandisolates_per_memberassertions so the collector grammar stays exhaustiveThe other three requirements were already covered and are left alone: virtual-key over-budget rejection by
test_budget_enforcement_e2e.py::test_budget_enforcement[KeyBudgetCase]andtest_budget_reset_advances_e2e.py::test_key_spend_blocks_at_cap; team-member reset bytest_team_member_budget_reset_e2e.pyandtest_budget_reset_advances_e2e.py::test_team_member_budget_reset_at_advances; andbudget_durationreset bytest_budget_reset_e2e.py::test_key_budget_resets_after_durationandtest_budget_reset_advances_e2e.py::test_key_budget_reset_at_advances_after_windowQA runbook
Final Attestation