diff --git a/openviking/server/routers/sessions.py b/openviking/server/routers/sessions.py index bc173b98a2..9e4bef711c 100644 --- a/openviking/server/routers/sessions.py +++ b/openviking/server/routers/sessions.py @@ -222,6 +222,27 @@ async def list_sessions( return Response(status="ok", result=result) +async def _count_on_disk_archives(session) -> int: + """Count archive_NNN/ directories on disk for a session. + + Returns 0 on any error (missing history dir, transient FS issue) so the + fallback never makes the read path fail; the persisted counter remains + the response in that case. + """ + viking_fs = getattr(session, "_viking_fs", None) + if viking_fs is None: + return 0 + try: + items = await viking_fs.ls(f"{session.uri}/history", ctx=session.ctx) + except Exception: + return 0 + return sum( + 1 + for item in items + if isinstance(item, dict) and str(item.get("name", "")).startswith("archive_") + ) + + @router.get("/{session_id}") async def get_session( session_id: str = Path(..., description="Session ID"), @@ -240,6 +261,19 @@ async def get_session( result["uri"] = session.uri result["user"] = session.user.to_dict() result["pending_tokens"] = int(session.meta.pending_tokens or 0) + + # Observability fallback for sessions written through async / out-of-band + # paths (e.g. the Hermes provider) that materialize archives on disk but + # leave the persisted counter at zero. The persisted value remains + # authoritative for the synchronous write path; we only override when it + # is stale relative to disk. + archive_count = await _count_on_disk_archives(session) + result["archive_count"] = archive_count + if archive_count > 0: + if int(result.get("commit_count") or 0) == 0: + result["commit_count"] = archive_count + if int(result.get("message_count") or 0) == 0: + result["message_count"] = archive_count return Response(status="ok", result=result) diff --git a/tests/server/test_session_counters.py b/tests/server/test_session_counters.py new file mode 100644 index 0000000000..a4afae5847 --- /dev/null +++ b/tests/server/test_session_counters.py @@ -0,0 +1,171 @@ +# Copyright (c) 2026 Beijing Volcano Engine Technology Co., Ltd. +# SPDX-License-Identifier: AGPL-3.0 + +"""Tests for the GET /sessions/{id} archive-derived counter fallback (#1550). + +Sessions written through async / out-of-band paths (e.g. the Hermes +provider) materialize archive_NNN/ directories on disk but leave the +persisted .meta.json counter at zero. The read endpoint must surface the +on-disk truth so the CLI does not report "nothing happened". + +These tests directly exercise the router handler with a fake service so +they do not require the native vectordb backend. +""" + +from types import SimpleNamespace + +import pytest + +from openviking.server.routers import sessions as sessions_router +from openviking.session.session import SessionMeta +from openviking_cli.exceptions import NotFoundError +from openviking_cli.session.user_id import UserIdentifier + + +def _fake_session( + *, + session_id: str = "sess-1", + message_count: int = 0, + commit_count: int = 0, + archive_dir_names=(), + raise_on_ls: bool = False, +): + user = UserIdentifier.the_default_user("u1") + meta = SessionMeta( + session_id=session_id, + message_count=message_count, + commit_count=commit_count, + ) + + async def _ls(uri, ctx=None): + if raise_on_ls: + raise FileNotFoundError(uri) + return [{"name": name, "isDir": True} for name in archive_dir_names] + + viking_fs = SimpleNamespace(ls=_ls) + return SimpleNamespace( + session_id=session_id, + uri=f"viking://session/{session_id}", + meta=meta, + user=user, + ctx=SimpleNamespace(), + _viking_fs=viking_fs, + ) + + +def _patch_service(monkeypatch, *, session=None, exc=None): + async def _get(session_id, ctx, auto_create=False): + if exc is not None: + raise exc + return session + + fake_service = SimpleNamespace(sessions=SimpleNamespace(get=_get)) + monkeypatch.setattr(sessions_router, "get_service", lambda: fake_service) + + +@pytest.mark.asyncio +async def test_count_on_disk_archives_counts_archive_dirs(): + session = _fake_session( + archive_dir_names=("archive_001", "archive_002", "summary.md", "extras") + ) + assert await sessions_router._count_on_disk_archives(session) == 2 + + +@pytest.mark.asyncio +async def test_count_on_disk_archives_returns_zero_on_missing_history(): + session = _fake_session(raise_on_ls=True) + assert await sessions_router._count_on_disk_archives(session) == 0 + + +@pytest.mark.asyncio +async def test_get_session_overrides_zero_counters_when_archives_exist(monkeypatch): + """The Hermes-style stale-counter case.""" + session = _fake_session( + message_count=0, + commit_count=0, + archive_dir_names=("archive_001", "archive_002"), + ) + _patch_service(monkeypatch, session=session) + + response = await sessions_router.get_session( + session_id="sess-1", auto_create=False, _ctx=SimpleNamespace() + ) + result = response.result + assert result["archive_count"] == 2 + assert result["commit_count"] == 2 + assert result["message_count"] == 2 + + +@pytest.mark.asyncio +async def test_get_session_does_not_downgrade_persisted_counters(monkeypatch): + """Persisted counters > derived value must remain authoritative.""" + session = _fake_session( + message_count=42, + commit_count=5, + archive_dir_names=("archive_001", "archive_002"), + ) + _patch_service(monkeypatch, session=session) + + response = await sessions_router.get_session( + session_id="sess-1", auto_create=False, _ctx=SimpleNamespace() + ) + result = response.result + assert result["archive_count"] == 2 + assert result["message_count"] == 42 + assert result["commit_count"] == 5 + + +@pytest.mark.asyncio +async def test_get_session_no_archives_keeps_zero(monkeypatch): + """No archives on disk → archive_count is 0, no override.""" + session = _fake_session(message_count=0, commit_count=0, archive_dir_names=()) + _patch_service(monkeypatch, session=session) + + response = await sessions_router.get_session( + session_id="sess-1", auto_create=False, _ctx=SimpleNamespace() + ) + result = response.result + assert result["archive_count"] == 0 + assert result["message_count"] == 0 + assert result["commit_count"] == 0 + + +@pytest.mark.asyncio +async def test_get_session_response_shape_unchanged(monkeypatch): + """Existing fields must remain present; only archive_count is added.""" + session = _fake_session(message_count=0, commit_count=0, archive_dir_names=()) + _patch_service(monkeypatch, session=session) + + response = await sessions_router.get_session( + session_id="sess-1", auto_create=False, _ctx=SimpleNamespace() + ) + result = response.result + for required_key in ( + "session_id", + "uri", + "user", + "message_count", + "commit_count", + "memories_extracted", + "llm_token_usage", + "embedding_token_usage", + "pending_tokens", + "archive_count", + ): + assert required_key in result, f"missing key {required_key!r}" + + +@pytest.mark.asyncio +async def test_get_session_not_found_passthrough(monkeypatch): + """Missing session still returns NOT_FOUND error response.""" + import json as _json + + _patch_service(monkeypatch, exc=NotFoundError("missing")) + + response = await sessions_router.get_session( + session_id="ghost", auto_create=False, _ctx=SimpleNamespace() + ) + # error_response returns a starlette JSONResponse with the standard payload. + body = _json.loads(response.body) + assert body["status"] == "error" + assert body["error"]["code"] == "NOT_FOUND"