fix(desktop): make session delete idempotent and id-resolving (#48641)

DELETE /api/sessions/{id} was the only session endpoint that didn't
resolve the id (detail, messages, rename, export all call
resolve_session_id) and 404'd when the row was already gone. The desktop
optimistically removes the sidebar row, then RESTORES it and shows the
error on any failure — so deleting a session that had just been reaped
(empty-session hygiene) or removed by a concurrent client resurrected a
ghost row and surfaced "session not found". /goal + auto-compression churn
leaves transient empty rows that race the sidebar snapshot, which is the
exact "I deleted the empty one and got 'session not found'" report.

Resolve exact ids / unique prefixes, and treat an already-absent session
as an idempotent success — DELETE's contract is "ensure it's gone". This
mirrors the bulk-delete endpoint, which already treats ghost ids as
success.

Tests: deleting an absent id is idempotent (200, not 404); delete resolves
a unique prefix; a real session still deletes.
This commit is contained in:
brooklyn! 2026-06-18 16:16:06 -05:00 committed by GitHub
parent f8d8f045fa
commit 2944b3c394
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 86 additions and 2 deletions

View file

@ -6886,8 +6886,19 @@ async def delete_session_endpoint(session_id: str, profile: Optional[str] = None
# desktop routes their DELETE to the remote backend. Omit for current/default.
db = _open_session_db_for_profile(profile)
try:
if not db.delete_session(session_id):
raise HTTPException(status_code=404, detail="Session not found")
# Resolve exact ids / unique prefixes like every other session endpoint
# (detail, messages, rename, export all do). A session that no longer
# exists is an idempotent success: DELETE's contract is "ensure it's
# gone", and the desktop optimistically removes the row then RESTORES it
# on any error — so a 404 on an already-absent row resurrected a ghost
# row and surfaced "session not found". /goal + auto-compression churn
# leaves transient empty rows (reaped by empty-session hygiene) that
# race the sidebar snapshot, which is exactly when this fired. Mirrors
# the bulk-delete endpoint, which already treats ghost ids as success.
sid = db.resolve_session_id(session_id)
if not sid:
return {"ok": True, "already_absent": True}
db.delete_session(sid)
return {"ok": True}
finally:
db.close()

View file

@ -4334,6 +4334,79 @@ class TestNormaliseThemeExtensions:
assert r["componentStyles"]["card"] == {"opacity": "0.8", "zIndex": "5"}
class TestDeleteSessionEndpoint:
"""Tests for ``DELETE /api/sessions/{session_id}`` — the single-row delete
behind the desktop sidebar's per-session delete.
The desktop optimistically removes the row, then RESTORES it on any error
and surfaces the message. So a 404 on a row that is already gone (reaped by
empty-session hygiene, or removed by a concurrent client both common amid
/goal + auto-compression churn that leaves transient empty rows) resurrected
a ghost row and showed "session not found". DELETE must be idempotent and
resolve ids like every other session endpoint.
"""
@pytest.fixture(autouse=True)
def _setup_test_client(self, monkeypatch, _isolate_hermes_home):
try:
from starlette.testclient import TestClient
except ImportError:
pytest.skip("fastapi/starlette not installed")
import hermes_state
from hermes_constants import get_hermes_home
from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN
monkeypatch.setattr(
hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db"
)
self.auth_client = TestClient(app)
self.auth_client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN
def _seed(self, ids):
from hermes_state import SessionDB
db = SessionDB()
try:
for sid in ids:
db.create_session(session_id=sid, source="cli")
finally:
db.close()
def _exists(self, sid) -> bool:
from hermes_state import SessionDB
db = SessionDB()
try:
return db.get_session(sid) is not None
finally:
db.close()
def test_delete_existing_session(self):
self._seed(["real_one"])
resp = self.auth_client.delete("/api/sessions/real_one")
assert resp.status_code == 200
assert resp.json().get("ok") is True
assert not self._exists("real_one")
def test_delete_absent_session_is_idempotent(self):
# PREMISE / regression: deleting a row that no longer exists must NOT
# 404 — the desktop would resurrect the ghost row and show
# "session not found". DELETE's contract is "ensure it's gone".
resp = self.auth_client.delete("/api/sessions/never_existed")
assert resp.status_code == 200
assert resp.json().get("ok") is True
def test_delete_resolves_unique_prefix(self):
# Symmetry with the other session endpoints, which all resolve ids.
self._seed(["20260618_abcdef_unique"])
resp = self.auth_client.delete("/api/sessions/20260618_abcdef")
assert resp.status_code == 200
assert resp.json().get("ok") is True
assert not self._exists("20260618_abcdef_unique")
class TestBulkDeleteSessionsEndpoint:
"""Tests for ``POST /api/sessions/bulk-delete`` — backs the
dashboard's "Delete N selected" flow on the sessions page.