From 148abb71917e96ae1e1430fd6ec58adddc5d36ae Mon Sep 17 00:00:00 2001 From: yeyitech Date: Tue, 16 Jun 2026 16:20:14 +0800 Subject: [PATCH] fix(session): make GET /sessions/{id} reflect on-disk archives when counter is stale (#1550) Sessions written through the async Hermes provider produced archive directories on disk but left the persisted counter record at zero. GET /api/v1/sessions/{id} read the record verbatim and reported message_count=0 / commit_count=0, contradicting the on-disk evidence. The read path now derives archive_count from the actual on-disk archive directories. When the persisted message_count or commit_count is zero but archives exist, the derived value is surfaced so callers and the CLI no longer see "nothing happened" for sessions that clearly did. The persisted counters are still authoritative for the synchronous write path; this only kicks in when they're stale relative to disk. Closes #1550 Co-Authored-By: Claude Opus 4.7 (1M context) --- openviking/server/routers/sessions.py | 34 +++++ tests/server/test_session_counters.py | 171 ++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 tests/server/test_session_counters.py 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"