diff --git a/api/models.py b/api/models.py index d121c7a4..cebc99c9 100644 --- a/api/models.py +++ b/api/models.py @@ -239,6 +239,14 @@ def _write_session_index(updates=None): _write_session_index(updates=None) +def prune_session_from_index(session_id: str) -> None: + """Remove one session row from the persisted sidebar index if present.""" + sid = str(session_id or "") + if not sid or not SESSION_INDEX_FILE.exists(): + return + _write_session_index(updates=[]) + + def _active_stream_ids(): with STREAMS_LOCK: active_ids = set(STREAMS.keys()) diff --git a/api/routes.py b/api/routes.py index 420dffe6..61d9de82 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2459,6 +2459,7 @@ from api.models import ( get_state_db_session_messages, merge_session_messages_append_only, _session_message_merge_key, + prune_session_from_index, ensure_cron_project, is_cron_session, ) @@ -5306,7 +5307,7 @@ def handle_post(handler, parsed) -> bool: # here makes the active-session external-refresh poll force-reload the # current chat every few seconds while the user is typing, and that # delayed reload can restore an older draft over newer local input. - s.save(touch_updated_at=False) + s.save(touch_updated_at=False, skip_index=True) return j(handler, {"ok": True, "draft": s.composer_draft}) if parsed.path == "/api/session/update": @@ -5392,10 +5393,6 @@ def handle_post(handler, parsed) -> bool: # Delete from WebUI session store with LOCK: SESSIONS.pop(sid, None) - try: - SESSION_INDEX_FILE.unlink(missing_ok=True) - except Exception: - logger.debug("Failed to unlink session index") # Evict cached agent so turn count doesn't leak into a recycled session from api.config import _evict_session_agent _evict_session_agent(sid) @@ -5409,6 +5406,10 @@ def handle_post(handler, parsed) -> bool: p.with_suffix('.json.bak').unlink(missing_ok=True) except Exception: logger.debug("Failed to unlink session file %s", p) + try: + prune_session_from_index(sid) + except Exception: + logger.debug("Failed to prune deleted session from index: %s", sid, exc_info=True) try: from api.upload import _session_attachment_dir diff --git a/tests/test_regressions.py b/tests/test_regressions.py index d1357eb8..7426013d 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -312,11 +312,8 @@ def test_deleted_session_does_not_appear_in_list(cleanup_test_sessions): assert sid not in ids_after, f"Deleted session {sid} still appears in list -- index not invalidated on delete" -def test_server_delete_invalidates_index(cleanup_test_sessions): - """R8b: session/delete handler must unlink _index.json. - Static check that the fix is in place. - Sprint 11: handler moved from server.py to api/routes.py -- check both. - """ +def test_server_delete_prunes_session_index(cleanup_test_sessions): + """session/delete should prune the deleted row without discarding the index.""" src = (REPO_ROOT / "server.py").read_text() routes_src = (REPO_ROOT / "api" / "routes.py").read_text() if (REPO_ROOT / "api" / "routes.py").exists() else "" # Find the delete handler in either file @@ -327,12 +324,11 @@ def test_server_delete_invalidates_index(cleanup_test_sessions): text.find('if parsed.path == "/api/session/delete":'), ) if delete_idx >= 0: - # Use 1200 chars to accommodate any validation/guard code added - # before the SESSION_INDEX_FILE.unlink() call (e.g. session_id - # character checks, path traversal guards). - delete_block = text[delete_idx:delete_idx+1200] - assert "SESSION_INDEX_FILE" in delete_block, \ - f"{label} session/delete must invalidate SESSION_INDEX_FILE" + delete_block = text[delete_idx:delete_idx+1800] + assert "prune_session_from_index" in delete_block, \ + f"{label} session/delete must prune SESSION_INDEX_FILE" + assert "SESSION_INDEX_FILE.unlink" not in delete_block, \ + f"{label} session/delete must not discard the whole session index" return assert False, "session/delete handler not found in server.py or api/routes.py" diff --git a/tests/test_session_index.py b/tests/test_session_index.py index 62ec9508..7143b479 100644 --- a/tests/test_session_index.py +++ b/tests/test_session_index.py @@ -20,7 +20,7 @@ from unittest.mock import patch import pytest import api.models as models -from api.models import Session, _write_session_index +from api.models import Session, _write_session_index, prune_session_from_index @pytest.fixture(autouse=True) @@ -83,6 +83,22 @@ def test_compact_exposes_last_message_at_from_message_timestamp(): assert compact["last_message_at"] == 200.0 +def test_prune_session_from_index_removes_only_deleted_row(): + index_file = models.SESSION_INDEX_FILE + s_a = _make_session("sess_a", "A", updated_at=100) + s_b = _make_session("sess_b", "B", updated_at=200) + s_a.save() + s_b.save() + + s_a.path.unlink() + prune_session_from_index("sess_a") + + index = _read_index(index_file) + ids = [entry["session_id"] for entry in index] + assert ids == ["sess_b"] + assert index_file.exists() + + def test_all_sessions_backfills_last_message_at_for_legacy_index_rows(): index_file = models.SESSION_INDEX_FILE s = Session( diff --git a/tests/test_stage326_composer_draft_validation.py b/tests/test_stage326_composer_draft_validation.py index 3f5904d6..af9c0cac 100644 --- a/tests/test_stage326_composer_draft_validation.py +++ b/tests/test_stage326_composer_draft_validation.py @@ -100,5 +100,5 @@ def test_draft_save_does_not_touch_session_updated_at(): src = Path(__file__).parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") persist_idx = src.find("s.composer_draft = draft") assert persist_idx != -1, "could not locate composer draft persist site" - save_idx = src.find("s.save(touch_updated_at=False)", persist_idx) - assert save_idx != -1, "composer draft save must preserve session updated_at" + save_idx = src.find("s.save(touch_updated_at=False, skip_index=True)", persist_idx) + assert save_idx != -1, "composer draft save must preserve session updated_at and skip index churn"