fix(doctor): name structural state.db corruption honestly and route it to sessions recover
`hermes doctor` reported every write-health-probe failure as "state.db FTS write corruption" and `--fix` ran the FTS repair ladder — rebuild, REINDEX, sqlite_master surgery + VACUUM — on the damaged file. When the damage is structural (canonical tables/indexes), none of those rungs can fix it, each one writes to the torn file in place, and the operator is then told to "restore from the backup copy beside state.db": a `.malformed-backup` that is a snapshot of the same corrupt image. `hermes sessions recover`, the tool that actually rebuilds canonical rows into a fresh file, was never mentioned (#88587; the 1.7 GB field incident lost days to it). Discriminate before mutating. hermes_state_repair.integrity_damage_is_structural maps `PRAGMA integrity_check` output onto the file: a `Tree N` id resolved through sqlite_master.rootpage, an index named in `row N missing from index X`, or a `Freelist:` line is structural unless the object is a Hermes-owned messages_fts* table/shadow (full-matched, so a user lookalike such as archive_fts_data is never swept into the rebuildable set). state_db_has_structural_damage runs it read-only on a fresh connection; an integrity_check that RAISES under the walk (torn root page) is structural too — no FTS-only fixture does that while sessions/messages read cleanly. doctor's state check consults it first: structural damage becomes a manual issue naming `hermes [-p <profile>] sessions recover --source <this db> --inspect-only` (profile pinned, #105887) and explicitly warning off the .malformed-backup; nothing is mutated and no backup is written. FTS-only damage keeps the existing in-place repair path. Verified against real fixtures: a torn `sessions` root page (before: "FTS write corruption", --fix wrote a 1:1 malformed-backup and failed; after: structural, recover guidance, no writes) and the 16-byte DEADBEEF messages_fts_data stomp (still repaired in place via rebuild_fts). Salvaged from PR #88604 (liuhao1024) onto the split doctor_state.py; the classifier lives beside the repair ladder in hermes_state_repair so the ladder itself can consult it next. Fixes #88587
This commit is contained in:
@@ -162,6 +162,8 @@ def _session_count(state_db_path: Path):
|
||||
|
||||
|
||||
# Corruption class -> (ok label, not-fixed label, failed issue, fix hint). ``{count}`` = recovered sessions.
|
||||
# ``structural`` has no in-place repair: an FTS rebuild cannot fix a canonical b-tree, and the
|
||||
# ``.malformed-backup`` the repair path would leave beside state.db is a copy of the same damage (#88587).
|
||||
_STATE_DB_REPAIRS = {
|
||||
"fts": ("Repaired state.db FTS write health",
|
||||
"state.db FTS write-health repair did not recover automatically",
|
||||
@@ -172,10 +174,21 @@ _STATE_DB_REPAIRS = {
|
||||
"state.db schema malformed and auto-repair failed — restore from the backup copy beside state.db",
|
||||
"state.db schema malformed — run 'hermes doctor --fix' (or 'hermes sessions repair') to recover hidden sessions"),
|
||||
}
|
||||
_STATE_DB_STRUCTURAL_ISSUE = (
|
||||
"state.db structural corruption (canonical tables/indexes damaged, not the FTS index) — an FTS rebuild "
|
||||
"cannot repair it. Stop the gateway, then run 'hermes {profile_arg}sessions recover --source {db_path} "
|
||||
"--inspect-only' and, if it reports recoverable, 'hermes {profile_arg}sessions recover --source {db_path} "
|
||||
"--output recovered-state.db'. Do NOT restore a .malformed-backup copy beside state.db: it is a snapshot "
|
||||
"of the same corrupt file."
|
||||
)
|
||||
|
||||
|
||||
def _repair_state_db(f: Finding, should_fix: bool, state_db_path: Path, kind: str) -> None:
|
||||
"""Shared --fix path for both state.db corruption classes (FTS write health, malformed schema)."""
|
||||
if kind == "structural":
|
||||
from hermes_constants import profile_cli_selector
|
||||
return f.manual_issues.append(_STATE_DB_STRUCTURAL_ISSUE.format(
|
||||
profile_arg=profile_cli_selector(), db_path=state_db_path))
|
||||
ok_label, not_fixed_label, failed_issue, fix_hint = _STATE_DB_REPAIRS[kind]
|
||||
if not should_fix:
|
||||
return f.issues.append(fix_hint)
|
||||
@@ -200,11 +213,15 @@ def _state_db_health(f: Finding, should_fix: bool, state_db_path: Path, _DHH: st
|
||||
check_ok(f"{_DHH}/state.db exists ({_session_count(state_db_path)} sessions)")
|
||||
# COUNT(*) succeeds even when the FTS index is corrupt and every write fails through the triggers;
|
||||
# _db_opens_cleanly drives a rolled-back write to surface that.
|
||||
from hermes_state_repair import _db_opens_cleanly
|
||||
from hermes_state_repair import _db_opens_cleanly, state_db_has_structural_damage
|
||||
# `_db_opens_cleanly` now drives a rolled-back write so this otherwise-silent corruption class is
|
||||
# surfaced (and repaired in place with --fix). See #50502.
|
||||
_write_reason = _db_opens_cleanly(state_db_path)
|
||||
if _write_reason is not None:
|
||||
if state_db_has_structural_damage(state_db_path):
|
||||
check_warn(f"{_DHH}/state.db has structural corruption (canonical tables/indexes damaged, "
|
||||
"not the FTS index)", f"({_write_reason})")
|
||||
return _repair_state_db(f, should_fix, state_db_path, "structural")
|
||||
check_warn(f"{_DHH}/state.db fails a write-health probe (FTS index may be corrupt)", f"({_write_reason})")
|
||||
_repair_state_db(f, should_fix, state_db_path, "fts")
|
||||
except Exception as e:
|
||||
|
||||
@@ -771,6 +771,15 @@ def display_hermes_home() -> str:
|
||||
return str(home)
|
||||
|
||||
|
||||
def profile_cli_selector() -> str:
|
||||
"""``-p <name> `` (trailing space) pinning copy-pasteable ``hermes ...`` guidance to the
|
||||
active NAMED profile, else ``""``: a bare ``hermes`` follows the sticky ``active_profile``
|
||||
file, which can name a different database than the one that failed (#105887). A custom
|
||||
home outside the profile tree has no selector (only HERMES_HOME names it)."""
|
||||
name = profile_name_for_home(get_hermes_home())
|
||||
return f"-p {name} " if name and name != "default" else ""
|
||||
|
||||
|
||||
def secure_parent_dir(path: Path) -> None:
|
||||
"""Chmod ``0o700`` on *path*'s parent, refusing ``/`` and top-level dirs (misresolved HERMES_HOME)."""
|
||||
parent = path.parent.resolve()
|
||||
|
||||
@@ -12,6 +12,7 @@ import itertools
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import sqlite3
|
||||
import stat
|
||||
@@ -684,6 +685,62 @@ def _schema_not_built(exc: BaseException) -> bool:
|
||||
return any(m in str(exc).lower() for m in ("no such table", "no such column"))
|
||||
|
||||
|
||||
# Hermes-owned FTS5 objects: the virtual tables and their shadow b-trees. Full-matched, so a
|
||||
# user-created lookalike (``archive_fts_data``) is not swept into the rebuildable set.
|
||||
_FTS_OBJECT_RE = re.compile(
|
||||
r"messages_fts(_trigram|_cjk)?(_data|_idx|_content|_docsize|_config|_segdir|_segments)?"
|
||||
)
|
||||
_INTEGRITY_TREE_RE = re.compile(r"\bTree (\d+)\b")
|
||||
_INTEGRITY_MISSING_INDEX_RE = re.compile(r"missing from index (\S+)")
|
||||
|
||||
|
||||
def integrity_damage_is_structural(integrity_lines, master_rows) -> bool:
|
||||
"""True when any damaged object named by ``PRAGMA integrity_check`` output lies outside
|
||||
the FTS shadow set: a ``Tree N`` id mapped through ``sqlite_master.rootpage``, an index in
|
||||
``row N missing from index X``, or the file's own freelist. Unparseable lines are not
|
||||
counted (the FTS wording stays, which is incomplete rather than wrong). #88587: an FTS
|
||||
rebuild cannot repair a canonical b-tree, and the ``.malformed-backup`` it leaves behind
|
||||
is a snapshot of the same damage."""
|
||||
name_by_rootpage = {int(rp): name for rp, _type, name in master_rows if rp}
|
||||
for line in integrity_lines:
|
||||
text = str(line)
|
||||
if text.startswith("Freelist"):
|
||||
return True
|
||||
tree = _INTEGRITY_TREE_RE.search(text)
|
||||
if tree:
|
||||
name = name_by_rootpage.get(int(tree.group(1)), "")
|
||||
if name and not _FTS_OBJECT_RE.fullmatch(name):
|
||||
return True
|
||||
missing = _INTEGRITY_MISSING_INDEX_RE.search(text)
|
||||
if missing and not _FTS_OBJECT_RE.fullmatch(missing.group(1)):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def state_db_has_structural_damage(db_path: Path) -> bool:
|
||||
"""Read-only ``integrity_check`` + ``sqlite_master`` rootpage map on a fresh connection;
|
||||
``integrity_damage_is_structural`` over the result. A check that RAISES instead of
|
||||
reporting (a torn page under the walk) is structural too: no FTS-only fixture does that
|
||||
while ``messages``/``sessions`` read cleanly, and the FTS rebuild ladder cannot help.
|
||||
Cannot-open / locked stays False so the caller keeps the FTS path."""
|
||||
try:
|
||||
conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True, timeout=1.0)
|
||||
except sqlite3.Error:
|
||||
return False
|
||||
try:
|
||||
master_rows = [tuple(r) for r in conn.execute(
|
||||
"SELECT rootpage, type, name FROM sqlite_master WHERE rootpage > 0").fetchall()]
|
||||
lines = [str(r[0]) for r in conn.execute("PRAGMA integrity_check").fetchall()]
|
||||
except sqlite3.OperationalError:
|
||||
return False
|
||||
except sqlite3.DatabaseError:
|
||||
return True
|
||||
finally:
|
||||
conn.close()
|
||||
return integrity_damage_is_structural(
|
||||
itertools.chain.from_iterable(line.splitlines() for line in lines), master_rows)
|
||||
|
||||
|
||||
def _db_opens_cleanly(db_path: Path) -> Optional[str]:
|
||||
"""Probe a DB on a fresh connection. Returns None if healthy, else a reason.
|
||||
|
||||
|
||||
93
tests/hermes_cli/test_doctor_structural_corruption.py
Normal file
93
tests/hermes_cli/test_doctor_structural_corruption.py
Normal file
@@ -0,0 +1,93 @@
|
||||
"""#88587 — doctor must name STRUCTURAL state.db corruption honestly.
|
||||
|
||||
The write-health probe's failure used to be reported as "FTS write corruption" unconditionally,
|
||||
routing operators to `--fix` / `sessions repair` (FTS rebuilds that cannot repair canonical-table
|
||||
damage) and to the .malformed-backup beside the DB (a snapshot of the same corrupt file). The
|
||||
discriminator maps integrity_check damage through sqlite_master.rootpage and keeps the FTS path
|
||||
only when every damaged object is a Hermes FTS shadow.
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
import io
|
||||
import sqlite3
|
||||
|
||||
from hermes_cli.doctor_report import Finding
|
||||
from hermes_cli.doctor_state import _state_db_health
|
||||
from hermes_state import SessionDB
|
||||
from hermes_state_repair import integrity_damage_is_structural, state_db_has_structural_damage
|
||||
|
||||
|
||||
def test_integrity_damage_classifier_maps_tree_ids_through_rootpage():
|
||||
"""Field mappings from #88587: tree 5 -> sessions and tree 15 -> gateway_routing are
|
||||
structural; a damaged messages_fts shadow tree is not; a lookalike foreign object,
|
||||
a canonical index named in a "missing from index" line, and the freelist are."""
|
||||
fts_only = [
|
||||
"Tree 12 page 9: btreeInitPage() returns error code 11",
|
||||
"row 3 missing from index messages_fts_trigram_idx",
|
||||
]
|
||||
master = [(12, "table", "messages_fts_data"), (5, "table", "sessions"),
|
||||
(15, "table", "gateway_routing"), (40, "table", "archive_fts_data")]
|
||||
assert integrity_damage_is_structural(fts_only, master) is False
|
||||
assert integrity_damage_is_structural(["Tree 5 page 421385: btreeInitPage() returns error code 11"], master)
|
||||
assert integrity_damage_is_structural(["Tree 15 page 15 cell 0: 2nd reference to page 5453"], master)
|
||||
assert integrity_damage_is_structural(["Tree 40 page 40: btreeInitPage() returns error code 11"], master)
|
||||
assert integrity_damage_is_structural(["row 1 missing from index sqlite_autoindex_delivery_obligations_1"], master)
|
||||
assert integrity_damage_is_structural(["Freelist: invalid page number 167772160"], master)
|
||||
# Unparseable / unknown-tree lines keep the FTS wording (incomplete, never wrong).
|
||||
assert integrity_damage_is_structural(["Tree 999 page 1: garbage", "*** in database main ***"], master) is False
|
||||
|
||||
|
||||
def _seed(tmp_path, rows=120):
|
||||
db_path = tmp_path / "state.db"
|
||||
db = SessionDB(db_path=db_path)
|
||||
db.create_session("s1", source="cli")
|
||||
for i in range(rows):
|
||||
db.append_message("s1", "user", f"hello {i} alpha beta " + "lorem " * 30)
|
||||
db.close()
|
||||
raw = sqlite3.connect(db_path)
|
||||
raw.execute("PRAGMA wal_checkpoint(TRUNCATE)")
|
||||
page_size = raw.execute("PRAGMA page_size").fetchone()[0]
|
||||
root = raw.execute("SELECT rootpage FROM sqlite_master WHERE name='sessions'").fetchone()[0]
|
||||
raw.close()
|
||||
return db_path, page_size, root
|
||||
|
||||
|
||||
def _run_doctor(db_path, should_fix):
|
||||
finding = Finding()
|
||||
with contextlib.redirect_stdout(io.StringIO()):
|
||||
_state_db_health(finding, should_fix, db_path, "~/x")
|
||||
return finding
|
||||
|
||||
|
||||
def test_doctor_routes_structural_damage_to_recover_not_fts_rebuild(tmp_path, monkeypatch):
|
||||
"""Real torn ``sessions`` b-tree: doctor --fix must not run the FTS repair ladder (no
|
||||
.malformed-backup, nothing fixed) and must point at `hermes sessions recover` for THIS
|
||||
database with the profile pinned; a real FTS-only stomp still takes the FTS path."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
db_path, page_size, root = _seed(tmp_path)
|
||||
with open(db_path, "r+b") as f:
|
||||
f.seek((root - 1) * page_size + 8)
|
||||
f.write(b"\xff\xff" * 8)
|
||||
assert state_db_has_structural_damage(db_path) is True
|
||||
|
||||
finding = _run_doctor(db_path, should_fix=True)
|
||||
assert finding.fixed == 0 and finding.issues == []
|
||||
(issue,) = finding.manual_issues
|
||||
assert "structural" in issue and "sessions recover" in issue and str(db_path) in issue
|
||||
assert "FTS write corruption" not in issue and "restore from the backup" not in issue
|
||||
assert not list(tmp_path.glob("state.db.malformed-backup-*"))
|
||||
|
||||
fts_path = tmp_path / "fts" / "state.db"
|
||||
fts_db = SessionDB(db_path=fts_path)
|
||||
fts_db.create_session("s1", source="cli")
|
||||
for i in range(40):
|
||||
fts_db.append_message("s1", "user", f"hello {i} alpha")
|
||||
fts_db.close()
|
||||
raw = sqlite3.connect(fts_path)
|
||||
raw.execute("UPDATE messages_fts_data SET block = X'DEADBEEFDEADBEEFDEADBEEFDEADBEEF'")
|
||||
raw.commit()
|
||||
raw.close()
|
||||
assert state_db_has_structural_damage(fts_path) is False
|
||||
fts_finding = _run_doctor(fts_path, should_fix=False)
|
||||
assert fts_finding.manual_issues == []
|
||||
assert any("FTS" in i for i in fts_finding.issues)
|
||||
Reference in New Issue
Block a user