diff --git a/api/agent_sessions.py b/api/agent_sessions.py index c0a5078f..3bccfd2d 100644 --- a/api/agent_sessions.py +++ b/api/agent_sessions.py @@ -6,6 +6,17 @@ from pathlib import Path logger = logging.getLogger(__name__) +# state.db paths that already produced the "no 'source' column" warning below. +# ``get_cli_sessions(all_profiles=True)`` re-reads every profile DB on every +# sidebar poll (behind a 5 s cache), so a single pre-``source`` profile DB would +# otherwise re-emit the identical WARNING line every ~15 s for the life of the +# process. The condition is a property of the DB file, not of the poll, so it +# is reported once per path. Process-lifetime only: a restart warns again, +# which is the desired behaviour (the log line is the operator's cue that the +# agent still needs upgrading). Plain ``set`` mutation under the GIL is +# sufficient here; a duplicate line from two racing first calls is harmless. +_SOURCE_COLUMN_WARNED_DB_PATHS: set[str] = set() + def open_state_db_readonly(db_path: Path, log: logging.Logger | None = None) -> sqlite3.Connection: """Open the live agent ``state.db`` read-only for a pure-read projection. @@ -562,12 +573,15 @@ def read_importable_agent_session_rows( cur.execute("PRAGMA table_info(messages)") message_cols = {row[1] for row in cur.fetchall()} if 'source' not in session_cols: - log.warning( - "agent session listing skipped: state.db at %s has no 'source' column " - "(older hermes-agent?). Agent sessions unavailable. " - "Upgrade hermes-agent to fix this.", - db_path, - ) + warned_key = str(db_path.resolve()) + if warned_key not in _SOURCE_COLUMN_WARNED_DB_PATHS: + _SOURCE_COLUMN_WARNED_DB_PATHS.add(warned_key) + log.warning( + "agent session listing skipped: state.db at %s has no 'source' column " + "(older hermes-agent?). Agent sessions unavailable. " + "Upgrade hermes-agent to fix this.", + db_path, + ) return [] parent_expr = _optional_col('parent_session_id', session_cols) diff --git a/tests/test_issue634.py b/tests/test_issue634.py index daa8c477..de9ebd32 100644 --- a/tests/test_issue634.py +++ b/tests/test_issue634.py @@ -77,3 +77,74 @@ class TestCliSessionsErrorSurface: assert select_pos != -1, "SELECT query not found" assert pragma_pos < select_pos, \ "Schema introspection must run before the main SQL query" + + +# --------------------------------------------------------------------------- +# Follow-up: the warning must fire once per state.db, not once per poll. +# +# get_cli_sessions(all_profiles=True) calls read_importable_agent_session_rows() +# for every profile on every sidebar poll (behind a 5 s cache). On an install +# with one pre-``source`` profile DB that produced one identical WARNING line +# every ~15 s in errors.log. The message is correct; its repetition is not. +# --------------------------------------------------------------------------- +import logging +import sqlite3 + +import pytest + +from api import agent_sessions + + +def _make_pre_source_state_db(path): + """Build a state.db whose ``sessions`` table predates the ``source`` column.""" + conn = sqlite3.connect(str(path)) + conn.execute( + "CREATE TABLE sessions (id TEXT PRIMARY KEY, title TEXT, model TEXT, " + "message_count INTEGER, started_at REAL)" + ) + conn.execute("CREATE TABLE messages (id INTEGER PRIMARY KEY, session_id TEXT, role TEXT, timestamp REAL)") + conn.commit() + conn.close() + + +def _source_column_warnings(caplog): + return [ + rec for rec in caplog.records + if rec.levelno == logging.WARNING and "has no 'source' column" in rec.getMessage() + ] + + +class TestMissingSourceColumnWarnsOncePerDb: + @pytest.fixture(autouse=True) + def _fresh_dedupe_state(self, monkeypatch): + # The dedupe set is process-lifetime state; give each test its own. + monkeypatch.setattr(agent_sessions, "_SOURCE_COLUMN_WARNED_DB_PATHS", set(), raising=False) + + def test_same_db_path_twice_logs_once(self, tmp_path, caplog): + db = tmp_path / "state.db" + _make_pre_source_state_db(db) + with caplog.at_level(logging.WARNING, logger="api.agent_sessions"): + first = agent_sessions.read_importable_agent_session_rows(db, exclude_sources=None) + second = agent_sessions.read_importable_agent_session_rows(db, exclude_sources=None) + assert first == [] and second == [] + warnings = _source_column_warnings(caplog) + assert len(warnings) == 1, [w.getMessage() for w in warnings] + # Message text is unchanged: still names the path and the upgrade hint. + assert str(db) in warnings[0].getMessage() + assert "Upgrade hermes-agent" in warnings[0].getMessage() + + def test_different_db_path_logs_again(self, tmp_path, caplog): + db_a = tmp_path / "planner" / "state.db" + db_b = tmp_path / "coder" / "state.db" + db_a.parent.mkdir() + db_b.parent.mkdir() + _make_pre_source_state_db(db_a) + _make_pre_source_state_db(db_b) + with caplog.at_level(logging.WARNING, logger="api.agent_sessions"): + agent_sessions.read_importable_agent_session_rows(db_a, exclude_sources=None) + agent_sessions.read_importable_agent_session_rows(db_b, exclude_sources=None) + agent_sessions.read_importable_agent_session_rows(db_a, exclude_sources=None) + warnings = _source_column_warnings(caplog) + assert len(warnings) == 2, [w.getMessage() for w in warnings] + assert str(db_a) in warnings[0].getMessage() + assert str(db_b) in warnings[1].getMessage()