ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(state): fail closed on unscoped SQLite corruption

Browse Source
This commit is contained in:
Bruce Xu
2026-08-21 14:46:55 +00:00
committed by kshitij
parent c80a0a551c
commit 50bbcbf2b4
7 changed files with 167 additions and 242 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/web_server.py
+2 -2
View File
@@ -12089,7 +12089,7 @@ def _open_session_db_at_path(db_path: Path, *, read_only: bool):
"""
import sqlite3
from hermes_state import SessionDB, is_malformed_db_error
from hermes_state import SessionDB, is_malformed_schema_error
if not read_only:
return SessionDB(db_path=db_path, read_only=False)
@@ -12126,7 +12126,7 @@ def _open_session_db_at_path(db_path: Path, *, read_only: bool):
except sqlite3.DatabaseError as exc:
message = str(exc).lower()
stale_schema = "no such table" in message or "no such column" in message
if not stale_schema and not is_malformed_db_error(exc):
if not stale_schema and not is_malformed_schema_error(exc):
raise
SessionDB(db_path=db_path, read_only=False).close()
try:
hermes_state.py
+25 -109
View File
@@ -1529,8 +1529,9 @@ def apply_database_pragmas(
# The canonical ``sessions`` / ``messages`` data is intact in these cases —
# only the derived schema is broken — so recovery preserves all transcripts
# and merely rebuilds the FTS layer.
_MALFORMED_SCHEMA_MARKERS = (
"malformed database schema",
_MALFORMED_SCHEMA_MARKERS = ("malformed database schema",)
_MALFORMED_DB_MARKERS = (
*_MALFORMED_SCHEMA_MARKERS,
"database disk image is malformed",
)
@@ -1542,32 +1543,29 @@ _repair_attempt_lock = threading.Lock()
def is_malformed_db_error(exc: BaseException) -> bool:
"""True if *exc* is a SQLite 'malformed schema / disk image' error.
"""True for explicit malformed-schema or generic corrupt-image errors.
These are the corruption classes where the schema fails to parse, so
targeted ``sqlite_master`` surgery (not an ordinary FTS rebuild) is the
only recovery path.
This broad classifier is for diagnostics and explicit offline recovery
dispatch. Runtime repair must use :func:`is_malformed_schema_error`, since
a generic corrupt-image error does not identify the damaged object.
"""
if not isinstance(exc, sqlite3.DatabaseError):
return False
return any(marker in str(exc).lower() for marker in _MALFORMED_DB_MARKERS)
def is_malformed_schema_error(exc: BaseException) -> bool:
"""True only when SQLite explicitly reports malformed schema text.
A generic ``database disk image is malformed`` error is SQLITE_CORRUPT
and may come from any B-tree or freelist page. It does not prove that
canonical rows are intact, so runtime schema/FTS repair must fail closed.
"""
if not isinstance(exc, sqlite3.DatabaseError):
return False
return any(marker in str(exc).lower() for marker in _MALFORMED_SCHEMA_MARKERS)
def _is_not_a_database_error(exc: BaseException) -> bool:
"""True if *exc* is SQLite's 'file is not a database' error.
Raised when a connection's backing file is not a SQLite database — the
runtime connection-corruption class: a sibling process (forked curator
agent, external repair pass) replaced/truncated the file out from under
the live connection. The file on disk may be perfectly healthy; the
CONNECTION is broken. Distinct from the malformed-schema class: the fix
is a reconnect, not schema surgery.
"""
if not isinstance(exc, sqlite3.DatabaseError):
return False
return "file is not a database" in str(exc).lower()
# Markers that mean the host filesystem cannot accept another write. Kept as
# plain substrings so OSError, sqlite3.OperationalError, and wrapped RPC
# error strings all match the same helper.
@@ -3781,13 +3779,6 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
# in place at most once per SessionDB instance so a genuinely
# unrecoverable database can't put writers into a rebuild loop.
self._fts_runtime_rebuild_attempted = False
# One-shot guard for the runtime connection-reopen recovery on the
# write path. A connection whose backing file was replaced/truncated
# by a sibling process surfaces as "file is not a database" on every
# write; we close and reopen the connection at most once per
# SessionDB instance so a genuinely unrecoverable database can't put
# writers into a reconnect loop.
self._notadb_reconnect_attempted = False
# One-shot guard for the usermerge-floor config write on the
# incremental FTS merge cadence (see _merge_fts_incrementally).
self._fts_usermerge_floor_applied = False
@@ -3973,7 +3964,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
# place (backup first; canonical sessions/messages preserved),
# then reopen once. This is what lets Desktop/Dashboard
# self-heal instead of silently showing "no sessions".
if not is_malformed_db_error(exc) or not _claim_repair_attempt(self.db_path):
if not is_malformed_schema_error(exc) or not _claim_repair_attempt(self.db_path):
raise
logger.error(
"state.db schema is malformed (%s) — attempting automatic "
@@ -4566,20 +4557,6 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
except sqlite3.DatabaseError as exc:
if _is_no_more_rows(exc) and self._sleep_before_write_retry(deadline, patience_s):
continue
# Runtime connection-corruption self-heal: a connection whose
# backing file was replaced/truncated by a sibling process
# (e.g. a forked curator agent inheriting and closing the
# write fd, or an external repair pass) surfaces as "file is
# not a database" on EVERY subsequent write. Without a
# reconnect branch the gateway wedges permanently: every
# transcript/routing write raises, messages stay in memory,
# and swap grows without bound until the process is killed.
# Close the broken connection, reopen the DB file, and retry
# the write once.
if _is_not_a_database_error(exc):
if not self._reconnect_after_notadb():
raise
continue
# Corrupt FTS shadow tables make every write raise the
# malformed/corrupt error class through the FTS sync triggers
# while the canonical messages table is intact. Recover here,
@@ -4630,77 +4607,16 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
time.sleep(min(jitter, max(deadline - now, 0.001)))
return True
def _reconnect_after_notadb(self) -> bool:
"""Close the corrupted write connection and reopen state.db.
Returns True when the connection was successfully replaced and the
failed write should be retried. Mirrors the constructor's
``_connect_and_init`` so WAL/schema reconciliation runs on the fresh
connection. Never raises — logs and returns False on failure so the
original error propagates.
One-shot per instance: a genuinely unrecoverable database must not
put writers into a reconnect loop that pins CPU on every write.
"""
if self._notadb_reconnect_attempted:
return False
self._notadb_reconnect_attempted = True
logger.warning(
"state.db connection reported 'file is not a database' — closing "
"and reopening the connection to self-heal (one-shot)."
)
try:
with self._lock:
if self._conn is not None:
try:
self._conn.close()
except Exception:
pass
self._conn = None
new_conn = _connect_tracked_db(
str(self.db_path),
tracking_path=self.db_path,
check_same_thread=False,
timeout=1.0,
isolation_level=None,
)
new_conn.row_factory = sqlite3.Row
# Publish BEFORE schema init: _init_schema/_reconcile_columns
# operate on self._conn, not on the local variable.
self._conn = new_conn
self._wal_active = (
apply_wal_with_fallback(new_conn, db_label="state.db")
== "wal"
)
apply_database_pragmas(new_conn, db_label="state.db")
new_conn.execute("PRAGMA foreign_keys=ON")
self._fts_cjk_loaded = load_fts5_cjk_extension(new_conn)
self._init_schema()
except Exception as exc:
logger.error(
"state.db reconnect after 'file is not a database' failed (%s); "
"the database may need the full offline repair path.",
exc,
)
return False
logger.warning(
"state.db connection reopened successfully; retrying the failed write."
)
return True
@staticmethod
def _is_fts_write_corruption_error(exc: sqlite3.DatabaseError) -> bool:
"""True for the error class a corrupt FTS index raises on writes.
The message varies by SQLite version: older builds raise the generic
``database disk image is malformed`` (covered by
``is_malformed_db_error``); newer builds (e.g. ubuntu-latest CI)
raise the FTS5-specific ``fts5: corrupt structure record for table
"messages_fts"``. Both mean the same thing for the write path: the
canonical rows are fine, the FTS shadow tables are not.
Only an FTS5-specific error is safe to recover online. SQLite's
generic ``database disk image is malformed`` does not identify the
damaged object and may represent canonical B-tree or freelist damage;
treating it as FTS-only corruption would continue writing to a
structurally damaged database.
"""
if is_malformed_db_error(exc):
return True
msg = str(exc).lower()
return "fts5" in msg and "corrupt" in msg
tests/hermes_cli/test_web_server.py
+24
View File
@@ -604,6 +604,30 @@ class TestWebServerEndpoints:
db.close()
assert len(writable_opens) == 1
def test_generic_corruption_does_not_trigger_writable_heal(
self, tmp_path, monkeypatch
):
"""Unscoped SQLITE_CORRUPT must not escalate a dashboard read to writes."""
import sqlite3
import hermes_state
from hermes_cli import web_server
db_path = tmp_path / "state.db"
db_path.write_bytes(b"not-empty")
opens = []
def corrupt_open(*_args, **kwargs):
opens.append(kwargs.get("read_only", False))
raise sqlite3.DatabaseError("database disk image is malformed")
monkeypatch.setattr(hermes_state, "SessionDB", corrupt_open)
with pytest.raises(sqlite3.DatabaseError, match="disk image is malformed"):
web_server._open_session_db_at_path(db_path, read_only=True)
assert opens == [True]
def test_get_sessions_zero_byte_store_returns_empty_list(self):
from hermes_constants import get_hermes_home
tests/state/test_fts_runtime_rebuild.py
+18 -6
View File
@@ -222,11 +222,9 @@ class TestRuntimeFtsRebuild:
# Cleanup
os.chmod(proc_root / "222" / "fd", 0o755)
def test_corruption_error_classification_covers_both_sqlite_messages(self):
"""SQLite's message for a corrupt FTS index varies by version: older
builds raise the generic malformed-image error, newer builds raise an
FTS5-specific one. Both must trigger the self-heal."""
assert SessionDB._is_fts_write_corruption_error(
def test_corruption_error_classification_requires_fts_provenance(self):
"""Generic SQLITE_CORRUPT cannot prove only derived data is damaged."""
assert not SessionDB._is_fts_write_corruption_error(
sqlite3.DatabaseError("database disk image is malformed")
)
assert SessionDB._is_fts_write_corruption_error(
@@ -238,6 +236,21 @@ class TestRuntimeFtsRebuild:
sqlite3.DatabaseError("no such table: nothing_fts_related")
)
def test_generic_malformed_write_fails_closed(self, db, monkeypatch):
db.create_session("s1", source="test")
monkeypatch.setattr(
db, "rebuild_fts", lambda: pytest.fail("must not rebuild FTS")
)
def _structural_corruption(_conn):
raise sqlite3.DatabaseError("database disk image is malformed")
with pytest.raises(sqlite3.DatabaseError, match="disk image is malformed"):
db._execute_write(_structural_corruption)
assert db._fts_runtime_rebuild_attempted is False
assert db._fts_enabled is True
def test_append_self_heals_after_fts_corruption(self, db, tmp_path):
if not db._fts_enabled:
pytest.skip("FTS5 unavailable in this build")
@@ -557,4 +570,3 @@ class TestRuntimeFtsRebuild:
assert recovered.search_messages("canonical survives")
finally:
recovered.close()
tests/test_state_db_malformed_repair.py
+21
View File
@@ -70,6 +70,27 @@ def test_duplicate_fts_makes_every_statement_fail(tmp_path):
assert is_malformed_db_error(exc_info.value)
def test_generic_malformed_open_does_not_attempt_schema_surgery(
tmp_path, monkeypatch
):
"""A generic SQLITE_CORRUPT error has no schema/FTS provenance."""
db_path = tmp_path / "state.db"
repair_calls = []
def _generic_corruption(*_args, **_kwargs):
raise sqlite3.DatabaseError("database disk image is malformed")
monkeypatch.setattr(hermes_state, "apply_wal_with_fallback", _generic_corruption)
monkeypatch.setattr(
hermes_state,
"repair_state_db_schema",
lambda *args, **kwargs: repair_calls.append((args, kwargs)),
)
with pytest.raises(sqlite3.DatabaseError, match="disk image is malformed"):
SessionDB(db_path=db_path)
assert repair_calls == []
def test_repaired_db_search_works(tmp_path):
tests/test_state_db_notadb_fail_closed.py
+77
View File
@@ -0,0 +1,77 @@
"""Tests for fail-closed state.db NOTADB handling and journal-mode EIO retries.
Covers the two independently-valuable pieces salvaged from the state.db
hardening rollup:
* fail closed when a live write connection reports ``file is not a database``;
* transient ``disk i/o error`` retry in ``_on_disk_journal_mode`` so a
one-shot EIO doesn't push callers onto the fail-closed unknown-mode branch.
"""
import sqlite3
from unittest.mock import MagicMock
import pytest
from hermes_state import SessionDB, _on_disk_journal_mode
class _NotADbOnce:
"""Connection proxy that raises 'file is not a database' on execute."""
def __init__(self, real_conn):
self._real = real_conn
def execute(self, *args, **kwargs):
raise sqlite3.DatabaseError("file is not a database")
def __getattr__(self, name):
return getattr(self._real, name)
class TestFailClosedAfterNotADb:
def test_write_does_not_reopen_after_connection_identity_breaks(
self, tmp_path, monkeypatch
):
"""One connection cannot safely heal a shared DB identity change."""
db = SessionDB(db_path=tmp_path / "state.db")
real_conn = db._conn
try:
db.create_session(session_id="s1", source="cli", model="test")
reopen = MagicMock()
monkeypatch.setattr("hermes_state._connect_tracked_db", reopen)
db._conn = _NotADbOnce(real_conn)
with pytest.raises(sqlite3.DatabaseError, match="not a database"):
db.create_session(session_id="s2", source="cli", model="test")
reopen.assert_not_called()
finally:
db._conn = real_conn
db.close()
class TestOnDiskJournalModeEioRetry:
def _conn_raising_then(self, failures, result_rows):
conn = MagicMock()
cursor = MagicMock()
cursor.fetchone.return_value = result_rows
conn.execute.side_effect = list(failures) + [cursor]
return conn
def test_transient_eio_clears_on_retry(self):
conn = self._conn_raising_then(
[sqlite3.OperationalError("disk i/o error")] * 2, ("wal",)
)
assert _on_disk_journal_mode(conn) == "wal"
def test_persistent_eio_returns_none(self):
conn = MagicMock()
conn.execute.side_effect = sqlite3.OperationalError("disk i/o error")
assert _on_disk_journal_mode(conn) is None
# Bounded: retried a handful of times, not forever.
assert conn.execute.call_count == 4
def test_non_eio_operational_error_fails_fast(self):
conn = MagicMock()
conn.execute.side_effect = sqlite3.OperationalError("database is locked")
assert _on_disk_journal_mode(conn) is None
assert conn.execute.call_count == 1
tests/test_state_db_notadb_selfheal.py
-125
View File
@@ -1,125 +0,0 @@
"""Tests for the state.db runtime connection self-heal (PR #82280 remainder).
Covers the two independently-valuable pieces salvaged from the state.db
hardening rollup:
* one-shot reconnect when a live write connection reports
``file is not a database`` (backing file replaced/truncated by a sibling
process — the connection is broken, the on-disk file may be healthy);
* transient ``disk i/o error`` retry in ``_on_disk_journal_mode`` so a
one-shot EIO doesn't push callers onto the fail-closed unknown-mode branch.
"""
import sqlite3
from unittest.mock import MagicMock
import pytest
from hermes_state import SessionDB, _is_not_a_database_error, _on_disk_journal_mode
class _NotADbOnce:
"""Connection proxy that raises 'file is not a database' on execute."""
def __init__(self, real_conn):
self._real = real_conn
def execute(self, *args, **kwargs):
raise sqlite3.DatabaseError("file is not a database")
def __getattr__(self, name):
return getattr(self._real, name)
class TestIsNotADatabaseError:
def test_matches_sqlite_message(self):
assert _is_not_a_database_error(
sqlite3.DatabaseError("file is not a database")
)
def test_rejects_other_database_errors(self):
assert not _is_not_a_database_error(
sqlite3.DatabaseError("database disk image is malformed")
)
def test_rejects_non_sqlite_exceptions(self):
assert not _is_not_a_database_error(ValueError("file is not a database"))
class TestReconnectAfterNotADb:
def test_write_self_heals_when_connection_breaks(self, tmp_path):
"""A broken connection over a healthy file reconnects and retries."""
db = SessionDB(db_path=tmp_path / "state.db")
try:
db.create_session(session_id="s1", source="cli", model="test")
# Simulate the runtime corruption class: the connection starts
# raising 'file is not a database' while the on-disk file is
# perfectly healthy (sibling replaced/truncated the old inode).
db._conn = _NotADbOnce(db._conn)
db.create_session(session_id="s2", source="cli", model="test")
assert db._notadb_reconnect_attempted is True
assert db.get_session("s2") is not None
# The pre-existing row survived (same on-disk file).
assert db.get_session("s1") is not None
finally:
db.close()
def test_reconnect_is_one_shot(self, tmp_path):
"""A second 'file is not a database' propagates instead of looping."""
db = SessionDB(db_path=tmp_path / "state.db")
try:
db._notadb_reconnect_attempted = True
db._conn = _NotADbOnce(db._conn)
with pytest.raises(sqlite3.DatabaseError, match="not a database"):
db.create_session(session_id="s3", source="cli", model="test")
finally:
db._conn = None
db.close()
def test_failed_reconnect_returns_false_and_original_error_propagates(
self, tmp_path, monkeypatch
):
"""If the reopen itself fails, the original write error surfaces."""
db = SessionDB(db_path=tmp_path / "state.db")
try:
monkeypatch.setattr(
"hermes_state._connect_tracked_db",
MagicMock(side_effect=sqlite3.DatabaseError("file is not a database")),
)
db._conn = _NotADbOnce(db._conn)
with pytest.raises(sqlite3.DatabaseError, match="not a database"):
db.create_session(session_id="s4", source="cli", model="test")
assert db._notadb_reconnect_attempted is True
finally:
db._conn = None
db.close()
class TestOnDiskJournalModeEioRetry:
def _conn_raising_then(self, failures, result_rows):
conn = MagicMock()
cursor = MagicMock()
cursor.fetchone.return_value = result_rows
conn.execute.side_effect = list(failures) + [cursor]
return conn
def test_transient_eio_clears_on_retry(self):
conn = self._conn_raising_then(
[sqlite3.OperationalError("disk i/o error")] * 2, ("wal",)
)
assert _on_disk_journal_mode(conn) == "wal"
def test_persistent_eio_returns_none(self):
conn = MagicMock()
conn.execute.side_effect = sqlite3.OperationalError("disk i/o error")
assert _on_disk_journal_mode(conn) is None
# Bounded: retried a handful of times, not forever.
assert conn.execute.call_count == 4
def test_non_eio_operational_error_fails_fast(self):
conn = MagicMock()
conn.execute.side_effect = sqlite3.OperationalError("database is locked")
assert _on_disk_journal_mode(conn) is None
assert conn.execute.call_count == 1