From ecf62c5fa8ca54a135bbc22e71b2b705356bc896 Mon Sep 17 00:00:00 2001 From: Ishaan Jaffer Date: Wed, 6 May 2026 16:10:00 -0700 Subject: [PATCH] test(cascade_delete): regression test for status-based cascade filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile P3 (regression coverage): the cascade filter in delete_agent was changed from terminated_at is None to status not in SESSION_TERMINAL_STATUSES in commit a0015e8564. Add an explicit test that exercises the bug surface — a session flipped to error (terminal) but with terminated_at deliberately left None. The legacy filter would have re-terminated it; the status-based filter must skip it. --- .../test_cascade_delete.py | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/test_litellm/proxy/agent_session_endpoints/test_cascade_delete.py b/tests/test_litellm/proxy/agent_session_endpoints/test_cascade_delete.py index ffeafe543e5..36f1cd5eacb 100644 --- a/tests/test_litellm/proxy/agent_session_endpoints/test_cascade_delete.py +++ b/tests/test_litellm/proxy/agent_session_endpoints/test_cascade_delete.py @@ -6,6 +6,7 @@ provider.terminate. """ from litellm.proxy.agent_session_endpoints.constants import ( + SESSION_STATUS_ERROR, SESSION_STATUS_TERMINATED, ) @@ -54,6 +55,64 @@ def test_delete_agent_terminates_sessions_and_calls_provider( assert sess_b["id"] in terminate_session_ids +def test_delete_agent_skips_session_already_in_terminal_status( + client, noop_provider, fake_prisma_client +): + """Greptile P3 (regression): the cascade filter must use + ``status not in SESSION_TERMINAL_STATUSES`` rather than + ``terminated_at is None``. A session whose status was flipped to + ``error`` (terminal) but whose ``terminated_at`` was never written + must NOT be re-terminated by the cascade — that would double-fire + ``provider.terminate`` and confuse the audit trail. + """ + agent = client.post( + "/v2/agents", + headers={"Authorization": "Bearer k"}, + json={"name": "t", "model": "gpt-4"}, + ).json() + aid = agent["id"] + + # An "active" session that should still get terminated by the cascade. + sess_active = client.post( + "/v2/sessions", + headers={"Authorization": "Bearer k"}, + json={"agent_id": aid, "repos": []}, + ).json() + # A session whose status was already flipped to ``error`` (terminal) + # but with ``terminated_at`` deliberately left None — the legacy + # filter would have re-terminated this row, but the status-based + # filter should skip it. + sess_already_terminal = client.post( + "/v2/sessions", + headers={"Authorization": "Bearer k"}, + json={"agent_id": aid, "repos": []}, + ).json() + terminal_row = next( + r + for r in fake_prisma_client.db.litellm_agentsession.rows + if r.id == sess_already_terminal["id"] + ) + terminal_row.status = SESSION_STATUS_ERROR + terminal_row.terminated_at = None # explicit — this is the bug surface + + # Reset terminate_calls so we can isolate what cascade fires. + noop_provider.terminate_calls.clear() + + res = client.delete(f"/v2/agents/{aid}", headers={"Authorization": "Bearer k"}) + assert res.status_code == 200 + + # Only the active session should have been routed through + # provider.terminate; the already-terminal one must be skipped. + terminate_session_ids = {c["session_id"] for c in noop_provider.terminate_calls} + assert sess_active["id"] in terminate_session_ids + assert ( + sess_already_terminal["id"] not in terminate_session_ids + ), "cascade re-terminated a session that was already in a terminal status" + + # The already-terminal session's status must be unchanged. + assert terminal_row.status == SESSION_STATUS_ERROR + + def test_delete_session_terminates_active_runs( client, noop_provider, fake_prisma_client ):