diff --git a/pyproject.toml b/pyproject.toml index a2a1d6c0ee24..d68ce4cfa56f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -332,6 +332,7 @@ markers = [ "integration: marks tests requiring external services (API keys, Modal, etc.)", "real_concurrent_gate: opt out of the autouse stub that disables _detect_concurrent_hermes_instances", "real_agent_prewarm: opt out of the autouse stub that disables the tui_gateway deferred agent pre-warm timer", + "requires_wal: needs the runtime to actually enable SQLite WAL mode (skipped where Hermes falls back to journal_mode=DELETE)", ] # integration tests take way too long to run in the normal CI environments addopts = "-m 'not integration'" diff --git a/tests/conftest.py b/tests/conftest.py index 1fcd1bd4abbd..159afd4c2b48 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,6 +21,7 @@ test runner at ``scripts/run_tests.sh``. import asyncio import os +import sqlite3 import sys from pathlib import Path @@ -541,6 +542,44 @@ def _ensure_current_event_loop(request): # delivery is harmless. _LIVE_SYSTEM_GUARD_BYPASS_MARK = "live_system_guard_bypass" +_REQUIRES_WAL_MARK = "requires_wal" + + +def _wal_is_usable() -> bool: + """True when Hermes will actually put a database into WAL mode here. + + Hermes refuses journal_mode=WAL on SQLite builds carrying the upstream + WAL-reset corruption bug (3.7.0–3.51.2, excluding backports 3.50.7 / + 3.44.6) and falls back to DELETE. On such a build NO ``-wal`` sidecar is + ever created, so a test asserting on WAL frames, ``-wal`` file size, or + checkpoint behaviour cannot pass — it is testing a mode the runtime + declined to enable, not a regression. + + This matters because the interpreter running the tests and the interpreter + running Hermes can link DIFFERENT SQLite versions: a repo ``.venv`` on + 3.50.4 (vulnerable → DELETE) alongside a Hermes managed runtime on 3.53.1 + (fixed → WAL). The same test then passes in one and fails in the other. + + IMPORTANT: this must NOT import ``hermes_state``. That module computes + ``DEFAULT_DB_PATH`` from ``get_hermes_home()`` at import time, so importing + it during collection — before the per-test ``_isolate_hermes_home`` fixture + redirects ``HERMES_HOME`` — permanently caches the DEVELOPER'S REAL + ``~/.hermes/state.db`` for the whole session. Tests then read live + production sessions instead of a tempdir. The version predicate is + duplicated from ``hermes_state._is_sqlite_wal_reset_vulnerable`` (upstream + fixed ranges, stable) rather than imported, and + ``test_conftest_wal_gate.py`` pins the two implementations in agreement. + """ + info = sqlite3.sqlite_version_info + if info < (3, 7, 0): + return True # pre-WAL library: cannot hit the race + if info >= (3, 51, 3): + return True # fixed upstream + if (3, 50, 7) <= info < (3, 51, 0): + return True # 3.50.x backport + if (3, 44, 6) <= info < (3, 45, 0): + return True # 3.44.x backport + return False def pytest_configure(config): # noqa: D401 — pytest hook @@ -551,6 +590,12 @@ def pytest_configure(config): # noqa: D401 — pytest hook "(only for tests that genuinely need real os.kill / subprocess " "behaviour — e.g. PTY tests that signal their own child).", ) + config.addinivalue_line( + "markers", + f"{_REQUIRES_WAL_MARK}: test needs the runtime to actually enable " + "SQLite WAL mode; skipped on builds where Hermes falls back to " + "journal_mode=DELETE for the WAL-reset bug.", + ) # The pyproject addopts pin ``--timeout-method=signal`` relies on # ``signal.SIGALRM``, which does not exist on Windows — pytest-timeout @@ -561,6 +606,26 @@ def pytest_configure(config): # noqa: D401 — pytest hook config.option.timeout_method = "thread" +def pytest_collection_modifyitems(config, items): # noqa: D401 — pytest hook + """Skip ``requires_wal`` tests when the linked SQLite can't use WAL. + + Cheaper and more honest than each test hand-rolling a version check: the + reason string names the actual linked version so the skip is diagnosable + rather than mysterious. + """ + if _wal_is_usable(): + return + + reason = ( + f"SQLite {sqlite3.sqlite_version} has the WAL-reset bug — Hermes uses " + "journal_mode=DELETE here, so no -wal sidecar exists to assert on" + ) + skip_marker = pytest.mark.skip(reason=reason) + for item in items: + if item.get_closest_marker(_REQUIRES_WAL_MARK) is not None: + item.add_marker(skip_marker) + + @pytest.fixture(autouse=True) def _live_system_guard(request, monkeypatch): """Block real os.kill / systemctl / gateway-pid scans during tests. diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 25ed7223129f..6f96b203bc5c 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -3248,6 +3248,17 @@ def test_connect_falls_back_to_delete_on_locking_protocol(tmp_path, monkeypatch, import sqlite3 as _sqlite3 from unittest.mock import patch as _patch + import hermes_state as _hs + + # The fallback warning is deduped process-globally ("once per process per + # database" — _log_wal_fallback_once / _log_wal_reset_bug_once). Any earlier + # test in this file that opened a kanban.db already consumed the one-shot + # for that label, so without clearing it this test sees zero warnings and + # fails only when run as part of the file (it passes in isolation). Clear + # both dedup sets so the warning is emitted for this connect(). + _hs._wal_fallback_warned_paths.clear() + _hs._wal_reset_bug_warned_paths.clear() + home = tmp_path / ".hermes" home.mkdir() monkeypatch.setenv("HERMES_HOME", str(home)) diff --git a/tests/hermes_cli/test_kanban_db_repair.py b/tests/hermes_cli/test_kanban_db_repair.py index b6ee83197c1a..03f4eefe7978 100644 --- a/tests/hermes_cli/test_kanban_db_repair.py +++ b/tests/hermes_cli/test_kanban_db_repair.py @@ -356,6 +356,7 @@ def test_wal_checkpoint_failure_never_fails_the_tick(tmp_path, monkeypatch): conn.close() +@pytest.mark.requires_wal def test_wal_checkpoint_truncates_wal_file(tmp_path, monkeypatch): """End-to-end: the checkpoint actually truncates the -wal sidecar.""" db_path = tmp_path / "kanban.db" diff --git a/tests/test_conftest_wal_gate.py b/tests/test_conftest_wal_gate.py new file mode 100644 index 000000000000..1e93ca53a8f3 --- /dev/null +++ b/tests/test_conftest_wal_gate.py @@ -0,0 +1,77 @@ +"""The conftest WAL gate must agree with hermes_state, and must not import it. + +``tests/conftest.py::_wal_is_usable`` duplicates the SQLite WAL-reset version +predicate instead of importing ``hermes_state``. That is deliberate: importing +``hermes_state`` during collection caches ``DEFAULT_DB_PATH`` from the real +``~/.hermes`` before the per-test ``HERMES_HOME`` redirect, which makes tests +read the developer's live production database. + +Duplication needs a guard, so these tests pin the two implementations in +agreement across the documented upstream boundaries. +""" + +import sqlite3 + +import pytest + +from hermes_state import is_sqlite_wal_reset_vulnerable +from tests.conftest import _wal_is_usable + + +@pytest.mark.parametrize( + "version_info", + [ + (3, 6, 23), # pre-WAL + (3, 7, 0), # first WAL release — vulnerable + (3, 44, 5), # below the 3.44 backport + (3, 44, 6), # 3.44 backport — fixed + (3, 45, 0), # next minor, back to vulnerable + (3, 50, 4), # the version that exposed this (repo .venv) + (3, 50, 6), # below the 3.50 backport + (3, 50, 7), # 3.50 backport — fixed + (3, 51, 2), # last vulnerable + (3, 51, 3), # fixed upstream + (3, 53, 1), # the managed runtime + ], +) +def test_conftest_gate_agrees_with_hermes_state(version_info, monkeypatch): + """``_wai_is_usable`` must be the exact inverse of the canonical predicate.""" + monkeypatch.setattr(sqlite3, "sqlite_version_info", version_info) + assert _wal_is_usable() is not is_sqlite_wal_reset_vulnerable(version_info), ( + f"conftest gate and hermes_state disagree for SQLite {version_info}" + ) + + +def test_conftest_does_not_import_hermes_state_at_collection(): + """The gate must stay import-free of hermes_state. + + Importing it during collection caches DEFAULT_DB_PATH from the real + ~/.hermes, so tests read live production sessions instead of a tempdir. + Reading the source is not an option here (banned), so assert on behavior: + the gate must work with ``hermes_state`` absent from ``sys.modules`` and + blocked from being imported. + """ + import builtins + import sys + + real_import = builtins.__import__ + blocked: list[str] = [] + + def guard(name, *args, **kwargs): + if name == "hermes_state" or name.startswith("hermes_state."): + blocked.append(name) + raise AssertionError( + "conftest._wal_is_usable imported hermes_state — this caches " + "DEFAULT_DB_PATH from the real ~/.hermes during collection" + ) + return real_import(name, *args, **kwargs) + + saved = sys.modules.pop("hermes_state", None) + builtins.__import__ = guard + try: + _wal_is_usable() # must not raise + finally: + builtins.__import__ = real_import + if saved is not None: + sys.modules["hermes_state"] = saved + assert not blocked