fix(team_endpoints): stop partial /team/update from wiping team metadata (#36328)

Co-authored-by: yassin <yassin@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
devin-ai-integration[bot] 2026-09-03 14:44:09 -07:00 committed by GitHub
parent 959e730d55
commit ab44e8d602
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 127 additions and 6 deletions

View file

@ -10,6 +10,7 @@ All /team management endpoints
"""
import asyncio
import copy
import json
import math
import traceback
@ -2189,6 +2190,26 @@ async def update_team(
if field in updated_kv
}
_writes_metadata_backed_field: Final = any(
field in updated_kv
for field in (
*LiteLLM_ManagementEndpoint_MetadataFields,
*LiteLLM_ManagementEndpoint_MetadataFields_Premium,
)
)
if isinstance(existing_team_row.metadata, dict):
if "metadata" not in updated_kv and (_team_member_fields_in_request or _writes_metadata_backed_field):
updated_kv["metadata"] = copy.deepcopy(existing_team_row.metadata)
elif isinstance(updated_kv.get("metadata"), dict):
updated_kv["metadata"] = {
**updated_kv["metadata"],
**{
key: existing_team_row.metadata[key]
for key in TeamMemberBudgetHandler.SYSTEM_MANAGED_METADATA_KEYS
if key in existing_team_row.metadata
},
}
if _team_member_fields_in_request and TeamMemberBudgetHandler.should_create_budget(
team_member_budget=data.team_member_budget,
team_member_rpm_limit=data.team_member_rpm_limit,

View file

@ -2910,6 +2910,7 @@ async def test_update_team_with_team_member_budget_duration(
"metadata": {"team_member_budget_id": "budget_123"},
}
mock_existing_team.metadata = {"team_member_budget_id": "budget_123"}
mock_existing_team.members_with_roles = []
mock_prisma_client.db.litellm_teamtable.find_unique = AsyncMock(
return_value=mock_existing_team
)
@ -11290,6 +11291,78 @@ async def test_patch_preserves_required_metadata_key_that_post_would_wipe():
assert patch_meta == {"cost_center": "FINOPS-1", "team_notes": "edited"} # preserved by PATCH
_STORED_METADATA_WITH_BUDGET: Final = {
"team_member_budget_id": "budget-existing-123",
"team_member_key_duration": "30d",
"logging": [{"callback_name": "langfuse", "callback_type": "success"}],
"cost_center": "cc-1234",
}
async def _written_metadata_with_budget(kind, body):
"""Like ``_written_metadata`` but the team already owns a member budget row."""
from litellm.proxy._types import LiteLLM_BudgetTable
with (
patch("litellm.proxy.proxy_server.premium_user", True), # test-quality-ok: proxy_server module global is the endpoint's only injection point
patch( # test-quality-ok: update_team imports update_budget at call time; the module attribute is its only seam
"litellm.proxy.management_endpoints.budget_management_endpoints.update_budget",
AsyncMock(return_value=LiteLLM_BudgetTable(budget_id="budget-existing-123")),
),
):
return await _written_metadata(kind, dict(_STORED_METADATA_WITH_BUDGET), body)
@pytest.mark.asyncio
@pytest.mark.parametrize("kind", ["post", "patch"])
@pytest.mark.parametrize(
"body",
[
{"team_member_budget": 50.0},
{"team_member_budget_duration": "1d"},
{"team_member_tpm_limit": 500},
{"team_member_rpm_limit": 5},
],
ids=lambda body: next(iter(body)),
)
async def test_team_member_budget_only_update_preserves_stored_metadata(kind, body):
"""LIT-5150: a budget-only update that omits ``metadata`` must not replace the
stored metadata JSON with just ``{"team_member_budget_id": ...}``."""
assert await _written_metadata_with_budget(kind, body) == _STORED_METADATA_WITH_BUDGET
@pytest.mark.asyncio
@pytest.mark.parametrize("kind", ["post", "patch"])
async def test_team_member_key_duration_only_update_preserves_stored_metadata(kind):
"""LIT-5150: a metadata-backed field sent alone is merged into the stored
metadata instead of becoming the whole metadata JSON."""
written = await _written_metadata_with_budget(kind, {"team_member_key_duration": "7d"})
assert written == {**_STORED_METADATA_WITH_BUDGET, "team_member_key_duration": "7d"}
@pytest.mark.asyncio
@pytest.mark.parametrize("kind", ["post", "patch"])
async def test_explicit_null_metadata_with_budget_field_still_clears_metadata(kind):
"""``metadata: null`` is an explicit clear, so only the server-owned budget link survives."""
written = await _written_metadata_with_budget(kind, {"metadata": None, "team_member_budget": 7.0})
assert written == {"team_member_budget_id": "budget-existing-123"}
@pytest.mark.asyncio
async def test_metadata_only_update_keeps_team_member_budget_link():
"""LIT-5150: rewriting metadata without any team member field must not drop the
server-owned ``team_member_budget_id``, or the member budget silently resets."""
body = {"metadata": {"cost_center": "cc-9999"}}
post_meta = await _written_metadata_with_budget("post", body)
patch_meta = await _written_metadata_with_budget("patch", body)
assert post_meta == {"cost_center": "cc-9999", "team_member_budget_id": "budget-existing-123"}
assert patch_meta == {**_STORED_METADATA_WITH_BUDGET, "cost_center": "cc-9999"}
@pytest.mark.asyncio
@pytest.mark.parametrize(
"body, field, expected",
@ -11318,17 +11391,15 @@ async def test_top_level_fields_identical_post_and_patch(body, field, expected):
@pytest.mark.asyncio
async def test_patch_strips_system_managed_metadata_key_like_post():
"""A caller cannot inject/overwrite server-owned keys via PATCH any more than
via POST: team_member_budget_id is stripped from the write in both."""
via POST: the stored team_member_budget_id wins over the caller's value in both."""
existing = {"team_member_budget_id": "budget-123", "cost_center": "1234"}
body = {"metadata": {"team_member_budget_id": "HACKED", "cost_center": "9999"}}
post_meta = await _written_metadata("post", existing, body)
patch_meta = await _written_metadata("patch", existing, body)
assert "team_member_budget_id" not in post_meta
assert "team_member_budget_id" not in patch_meta
assert post_meta == {"cost_center": "9999"}
assert patch_meta == {"cost_center": "9999"}
assert post_meta == {"cost_center": "9999", "team_member_budget_id": "budget-123"}
assert patch_meta == {"cost_center": "9999", "team_member_budget_id": "budget-123"}
@pytest.mark.parametrize(

View file

@ -1538,6 +1538,35 @@ describe("TeamInfoView", () => {
});
});
describe("team member settings", () => {
it("should populate Default Key Duration from the team's stored metadata", async () => {
const user = userEvent.setup({ delay: null });
vi.mocked(networking.teamInfoCall).mockResolvedValue(
createMockTeamData({ metadata: { team_member_key_duration: "30d" } }),
);
renderWithProviders(<TeamInfoView {...defaultProps} />);
await waitFor(() => {
expect(screen.queryAllByText("Test Team").length).toBeGreaterThan(0);
});
await user.click(screen.getByRole("tab", { name: "Settings" }));
await waitFor(() => {
expect(screen.getByRole("button", { name: /edit settings/i })).toBeInTheDocument();
});
await user.click(screen.getByRole("button", { name: /edit settings/i }));
await user.click(await screen.findByRole("button", { name: /team member settings/i }));
await waitFor(() => {
expect(screen.getByLabelText(/^Default Key Duration/)).toHaveValue("30d");
});
});
});
describe("guardrails dropdown grouping", () => {
const guardrail = (name: string, defaultOn: boolean) => ({
guardrail_name: name,

View file

@ -335,7 +335,7 @@ const toTeamFormValues = (info: TeamInfoRecord, effectiveGuardrails: string[]):
default_team_member_models: info.default_team_member_models || [],
team_member_budget: info.team_member_budget_table?.max_budget,
team_member_budget_duration: info.team_member_budget_table?.budget_duration,
team_member_key_duration: info.team_member_key_duration,
team_member_key_duration: info.metadata?.team_member_key_duration,
team_member_tpm_limit: info.team_member_budget_table?.tpm_limit,
team_member_rpm_limit: info.team_member_budget_table?.rpm_limit,
budget_duration: info.budget_duration,