From 12df57498af97442db3201ccbfa3bfdbb701e29b Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes <2362425+rodrigogs@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:38:07 -0300 Subject: [PATCH] fix(sessions): warn once per state.db when 'source' column is missing Observed: on a multi-profile install where one profile's state.db predates the agent's `source` column, errors.log gained one identical line every ~15 s (276 lines in ~70 min): api.models WARNING agent session listing skipped: state.db at /data/hermes/profiles/planner/state.db has no 'source' column (older hermes-agent?). Agent sessions unavailable. Upgrade hermes-agent to fix this. Mechanism: api/agent_sessions.py read_importable_agent_session_rows() runs `PRAGMA table_info(sessions)` and warns unconditionally when the column is absent (api/agent_sessions.py:564 before this change). api/models.py get_cli_sessions(all_profiles=True) calls it for every profile on every sidebar poll, behind only the 5 s _CLI_SESSIONS_CACHE_TTL_SECONDS cache, so the same DB re-emits the same warning on every cache miss. The condition is a property of the DB file, not of the poll. Fix: a module-level `_SOURCE_COLUMN_WARNED_DB_PATHS: set[str]` keyed by the resolved db_path. The warning fires the first time a given path is seen in the process and is suppressed after that; a different path warns on its own. Message text is unchanged. Process-lifetime only: a restart warns again, which is intended (the line remains the operator's cue that the agent still needs upgrading). Plain set mutation under the GIL is sufficient; a duplicate line from two racing first calls is harmless. Tests (tests/test_issue634.py, real pre-`source` sqlite DBs under tmp_path, asserting on captured log records): - same db_path called twice -> exactly one WARNING (failed before: `assert 2 == 1`) - two db_paths, then the first again -> exactly two WARNINGs, one per path (failed before: `assert 3 == 2`) The seven existing #634 source-shape tests still pass; neighbours test_issue3762/3930/5455/1494 pass. Deliberately not changed: the exception-path warning in get_cli_sessions() (api/models.py), the read-only-open fallback warning, and the 5 s cache TTL. The operator-side root cause (the agent reconciler creating a pre-`source` state.db) is a separate upstream fix; this only stops the repetition. --- api/agent_sessions.py | 26 ++++++++++++---- tests/test_issue634.py | 71 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) 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()