fix(team): serialize member_add, member_delete, and delete under the team's advisory lock

[yassin-berriai]

TLDR

Problem this solves:

• A /team/member_add mid-flight during /team/delete could still write a reference to the deleted team
• The write path re-read the team under a row lock, but that lock can deadlock with the access-group endpoints
• /team/delete itself took no lock, so it could race a concurrent add either way
• Fixing the deadlock by dropping that row lock also dropped its accidental protection against /team/member_delete, which could then silently undo a concurrent add
• Lock waiters can hold the whole connection pool, starving the holder of a second connection

How it solves it:

• member_add takes the team's advisory lock, re-reads the team, and only writes if it is still there
• delete_team takes the same lock around its own row delete and reference sweep
• member_delete takes the same lock too, and re-reads the roster under it instead of computing from the snapshot it validated against
• Replaces the deadlock-prone SELECT ... FOR UPDATE with pg_advisory_xact_lock, which the access-group endpoints never take
• Every read and write held under the lock runs on the lock holder's own transaction, never a second pooled connection

User Flow

Before: an admin who deletes a team, or removes one of its members, while another admin is mid-flight adding a different member to it can end up with a member reference surviving the delete, or with a removed member's slot silently coming back

1. Admin A calls POST /team/member_add for team T with a new member, and admin B calls POST /team/delete (or POST /team/member_delete for a different member) for team T microseconds apart
2. Depending on timing, both requests can report success even though team T no longer exists, or the member B just removed
3. GET /user/info?user_id=<the new member> afterward can still list team T alongside whatever team the user actually belongs to, and a membership row for T can still exist in the database; or GET /team/info?team_id=T can still list the member B just removed
4. Separately, two admins removing members from the same busy team at the same time both get 500 Internal server error after five seconds and neither removal is applied

After: the requests are fully serialized by the database

1. Admin A calls POST /team/member_add for team T with a new member, and admin B calls POST /team/delete (or POST /team/member_delete) for team T microseconds apart
2. Whichever request the database lets through first runs to completion before the other's read of the team can proceed
3. GET /user/info?user_id=<the new member> afterward shows either the member on a team that is genuinely still there, or no trace of team T at all; GET /team/info?team_id=T never shows a member a delete already removed
4. The two simultaneous removals both return 200, in whichever order the lock granted them, and the roster ends up with both members gone

Relevant issues

Linear ticket

Resolves LIT-5544

Pre-Submission checklist

• I have added meaningful tests
• The handful of test files covering my change pass locally
• My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

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

This branches off #37959 (LIT-5541) for its transaction-threading prerequisite: the advisory lock is transaction-scoped, and that PR is what gives these endpoints' write paths a prisma_client.tx() to hold it in. If #37959 has not merged to litellm_internal_staging yet, this PR's diff and commit list will show its commit too; merge that one first, or review this diff against its branch instead of against staging

Shared setup, used by every case below. A real Postgres, a proxy on port 14544 with database_connection_pool_limit: 2 and database_connection_pool_timeout: 10, master key sk-1234, and a fresh team per run:

$ export P=http://localhost:14544 K="Authorization: Bearer sk-1234" H="Content-Type: application/json" T=lit5544-$(date +%s)
$ curl -s -X POST $P/team/new -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"team_alias\":\"$T\"}" -o /dev/null -w "team/new HTTP %{http_code}\n"
$ for u in a b; do curl -s -X POST $P/team/member_add -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"member\":{\"user_id\":\"$T-$u\",\"role\":\"user\"}}" -o /dev/null -w "member_add $u HTTP %{http_code}\n"; done

The small pool is what makes case 1 observable at two concurrent requests instead of dozens: a waiter that needs a second connection starves the holder, and Postgres' interactive-transaction timeout (5s) then kills both. Case 2 needs a temporary 3-second delay injected into member_add right after it takes the lock, so delete (fired 1 second later) has to wait on the lock rather than win a timing race against it

Before (11b60b9, the commit before this PR's tip)

Case 1: two admins removing different members of the same team at once

1. Fire both removals concurrently:

$ curl -s -X POST $P/team/member_delete -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"user_id\":\"$T-a\"}" -w "\nmember_delete a HTTP %{http_code} (took %{time_total}s)\n" &
$ curl -s -X POST $P/team/member_delete -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"user_id\":\"$T-b\"}" -w "\nmember_delete b HTTP %{http_code} (took %{time_total}s)\n"
$ wait

{"error":{"message":"Internal server error","type":"internal_server_error"}}
member_delete a HTTP 500 (took 5.028855s)
{"error":{"message":"Internal server error","type":"internal_server_error"}}
member_delete b HTTP 500 (took 5.044846s)

2. Both removals were lost, and the team still carries both members:

$ curl -s "$P/team/info?team_id=$T" -H "$K" | python3 -c "import json,sys;print(json.load(sys.stdin)['team_info']['members_with_roles'])"
$ curl -s "$P/user/info?user_id=$T-a" -H "$K" | python3 -c "import json,sys;print('user a teams:',json.load(sys.stdin)['user_info']['teams'])"

[{'user_id': 'default_user_id', 'role': 'admin'}, {'user_id': 'lit5544-before-b', 'role': 'user'}, {'user_id': 'lit5544-before-a', 'role': 'user'}]
user a teams: ['lit5544-before']

3. The proxy log names the cause, one entry per request: the holder waited on a pooled connection it could not get until its own transaction expired:

prisma.errors.TransactionExpiredError: Transaction already closed: A query cannot be executed on an expired transaction. The timeout for this transaction was 5000 ms, however 5018 ms passed since the start of the transaction.

Case 2: a member_add mid-flight against a concurrent team/delete

1. No curl transcript: 11b60b9 already carries this PR's advisory lock, so this case behaves there exactly as it does in After, and the pool fix is the only delta between the two commits
2. The pre-lock reproduction is not expressible in curl either, because it needs a specific interleaving that a plain concurrent race does not hit (member_add's own re-read catches the ordinary ordering and 404s). It is pinned instead by the deterministic Postgres harness in tests/proxy_admin_ui_tests/test_team_delete_member_add_race.py, whose three legs each fail with the corresponding lock acquisition removed

After (abf4da9, this PR's tip)

Case 1: two admins removing different members of the same team at once

1. Fire both removals concurrently, same commands as before:

$ curl -s -X POST $P/team/member_delete -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"user_id\":\"$T-a\"}" -w "\nmember_delete a HTTP %{http_code} (took %{time_total}s)\n" &
$ curl -s -X POST $P/team/member_delete -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"user_id\":\"$T-b\"}" -w "\nmember_delete b HTTP %{http_code} (took %{time_total}s)\n"
$ wait

member_delete b HTTP 200 (took 0.030653s)
member_delete a HTTP 200 (took 0.047819s)

2. Both removals landed:

$ curl -s "$P/team/info?team_id=$T" -H "$K" | python3 -c "import json,sys;print(json.load(sys.stdin)['team_info']['members_with_roles'])"
$ curl -s "$P/user/info?user_id=$T-a" -H "$K" | python3 -c "import json,sys;print('user a teams:',json.load(sys.stdin)['user_info']['teams'])"

[{'user_id': 'default_user_id', 'user_email': None, 'role': 'admin'}]
user a teams: []

Case 2: a member_add mid-flight against a concurrent team/delete

1. Fire the add, then the delete one second later, with the temporary 3-second delay in member_add:

$ curl -s -X POST $P/team/member_add -H "$K" -H "$H" -d "{\"team_id\":\"$T\",\"member\":{\"user_id\":\"$T-u\",\"role\":\"user\"}}" -o /dev/null -w "member_add HTTP %{http_code} (took %{time_total}s)\n" &
$ sleep 1 && curl -s -X POST $P/team/delete -H "$K" -H "$H" -d "{\"team_ids\":[\"$T\"]}" -w "\ndelete HTTP %{http_code} (took %{time_total}s)\n"
$ wait

member_add HTTP 200 (took 3.031794s)
{"deleted_teams":["lit5544-demo-1787437895"]}
delete HTTP 200 (took 2.069267s)

delete took 2.07s instead of returning immediately: it genuinely waited on the lock member_add held rather than racing it. member_add succeeded legitimately, the team was still live when it read it, and then delete's own locked sweep reaped the fresh reference

2. Nothing from the deleted team survives:

$ curl -s "$P/user/info?user_id=$T-u" -H "$K" | python3 -c "import json,sys;print('user teams:',json.load(sys.stdin)['user_info']['teams'])"
$ psql -tAc "SELECT count(*) FROM \"LiteLLM_TeamMembership\" WHERE team_id='$T'"
$ psql -tAc "SELECT count(*) FROM \"LiteLLM_TeamTable\" WHERE team_id='$T'"

user teams: []
teammembership rows: 0
teamtable rows: 0

Type

🐛 Bug Fix

Caveats (if any)

• Depends on fix(proxy): make /team/member_delete's four cleanups atomic #37959 (LIT-5541) landing first; rebase once it merges so the diff shows only this PR's commits

Final Attestation

• The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/fc97c5d8da914f75a09dd5fa7273c437
Requested by: @yassin-berriai

[yassin-berriai]

@greptileai please review

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[yassin-berriai]

@greptileai re-requesting review after pushing bb66b9d (fixed a PT011 lint failure in the new test file's exception type; no production-code change).

[greptile-apps]

Greptile Summary

This PR serializes team member addition, member deletion, and team deletion with a shared PostgreSQL advisory lock

• Re-reads the current team roster after acquiring the lock
• Makes member cleanup and team-reference deletion transactional
• Adds mocked and real-Postgres race regression coverage

Confidence Score: 5/5

The PR appears safe to merge

No blocking failure remains

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/team_endpoints.py	Coordinates member and team deletion through a shared advisory lock and transactionally updates linked records
litellm/repositories/team_repository.py	Replaces the deadlock-prone row-locking read with a plain read guarded by the caller-held advisory lock
litellm/proxy/management_helpers/access_group_team_sync.py	Exposes the existing team advisory-lock statement for consistent reuse by team lifecycle operations
litellm/proxy/management_endpoints/key_management_endpoints.py	Allows deleted verification-token records to be persisted through an existing transaction
tests/proxy_admin_ui_tests/test_team_delete_member_add_race.py	Exercises deterministic advisory-lock interleavings against PostgreSQL
tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py	Updates transaction mocks and verifies locked reconciliation and rollback behavior

_{Reviews (4): Last reviewed commit: "fix(team): serialize member_add, member_..." | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 96.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tellm/proxy/management_endpoints/team_endpoints.py	95.23%	2 Missing ⚠️
litellm/proxy/management_helpers/utils.py	96.77%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

[greptile-apps]

Regression tests bypass mapped file

These tests cover an existing team endpoint bug but create a standalone module instead of extending tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py, splitting related regression coverage and making the repository's established test mapping less reliable.

Context Used: CLAUDE.md (source)

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!

[yassin-berriai]

@greptileai re-requesting review after pushing aa0f4e5: team_member_delete now also takes the team's advisory lock and re-reads the roster under it before writing, closing the member_delete-vs-member_add gap flagged in the last review. Added a third deterministic Postgres test pinning it.

[yassin-berriai]

@greptileai the real-Postgres location is deliberate: tests/test_litellm runs mocked-only in CI with no DATABASE_URL, matching sibling test_access_group_team_sync.py's same-directory precedent for identical reasons.

[greptile-apps]

That's a solid rationale, and the precedent is real — test_access_group_team_sync.py is confirmed in the same directory using the same pg_advisory_xact_lock pattern. The module-level docstring in your test file even calls this out explicitly, which is the right way to document the constraint for the next reader.

Accepted — placement is correct.

[yassin-berriai]

@greptileai please re-review the current head, aa0f4e5, which adds the team_member_delete advisory-lock fix on top of what you last scored.

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_lit5544_serialize_team_writes (33a9581) with litellm_internal_staging (bb27bfd)¹}

Footnotes

1. No successful run was found on litellm_internal_staging (6d0cc14) during the generation of this report, so bb27bfd was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩