mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
feat(kanban): add hermes kanban repair CLI verb
Adds kanban_db.repair_db() — a structured, non-raising wrapper around the same narrow repair policy as the connect-time guard: probe with PRAGMA integrity_check under the board's cross-process init flock; quarantine the corrupt bytes FIRST via the content-addressed backup; REINDEX only when every integrity message is index-scoped; re-check; report ok / repaired / corrupt / missing. Locked/busy OperationalError still propagates raw (a locked healthy DB is not corruption and gets no quarantine), and a repair invalidates the per-process healthy-path cache so the next connect() re-probes. The CLI verb reports status human-readably (or --json), exits 0 for ok/repaired/missing and 1 when the DB is still corrupt (non-index corruption stays fail-closed with manual-recovery guidance). It dispatches BEFORE kanban_command's auto-init: init_db() raises KanbanDbCorruptError on a corrupt board, which previously would have made a repair verb unreachable on exactly the boards that need it. CLI tests drive the real argparse surface (build_parser + kanban_command) against real corrupted SQLite fixtures.
This commit is contained in:
Teknium
parent
49828a3fd6
commit
60cfa11136
3 changed files with 332 additions and 0 deletions
|
|
@ -880,6 +880,25 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu
|
|||
p_gc.add_argument("--log-retention-days", type=int, default=30,
|
||||
help="Delete worker log files older than N days (default: 30)")
|
||||
|
||||
# --- repair ---
|
||||
p_repair = sub.add_parser(
|
||||
"repair",
|
||||
help="Check kanban.db integrity and auto-repair index-only corruption",
|
||||
description=(
|
||||
"Runs PRAGMA integrity_check on the board's DB and reports the "
|
||||
"result. When the failure consists only of index-scoped errors "
|
||||
"('wrong # of entries in index <name>' / 'row N missing from "
|
||||
"index <name>'), the corrupt file is quarantined to a "
|
||||
".corrupt.<hash>.bak sibling first and the damaged indexes are "
|
||||
"rebuilt with REINDEX — the same narrow auto-repair the "
|
||||
"connect-time guard applies. Any other corruption class is "
|
||||
"reported and left untouched (fail-closed). Exits 0 when the DB "
|
||||
"is healthy or was repaired, non-zero when it is still corrupt."
|
||||
),
|
||||
)
|
||||
p_repair.add_argument("--json", action="store_true",
|
||||
help="Emit the repair report as JSON")
|
||||
|
||||
kanban_parser.set_defaults(_kanban_parser=kanban_parser)
|
||||
return kanban_parser
|
||||
|
||||
|
|
@ -950,6 +969,12 @@ def kanban_command(args: argparse.Namespace) -> int:
|
|||
# schema creation; `create` / `list` / every other command would
|
||||
# error out on a fresh install.
|
||||
with board_scope:
|
||||
# `repair` must dispatch BEFORE the auto-init below: on a corrupt DB
|
||||
# init_db() itself raises KanbanDbCorruptError, which would turn
|
||||
# every `hermes kanban repair` into "could not initialize database"
|
||||
# without ever reaching the repair path.
|
||||
if action == "repair":
|
||||
return _cmd_repair(args)
|
||||
try:
|
||||
kb.init_db()
|
||||
except Exception as exc:
|
||||
|
|
@ -2886,6 +2911,76 @@ def _cmd_gc(args: argparse.Namespace) -> int:
|
|||
return 0
|
||||
|
||||
|
||||
def _cmd_repair(args: argparse.Namespace) -> int:
|
||||
"""Check DB integrity and apply the narrow index-REINDEX auto-repair.
|
||||
|
||||
Dispatched BEFORE the auto ``kb.init_db()`` in :func:`kanban_command`
|
||||
(init itself refuses corrupt DBs), so this is reachable on exactly the
|
||||
boards that need it. Exit codes: 0 = healthy / repaired / no DB file,
|
||||
1 = still corrupt (non-index corruption, or REINDEX did not produce a
|
||||
clean re-check).
|
||||
"""
|
||||
try:
|
||||
report = kb.repair_db()
|
||||
except Exception as exc: # locked/busy probe, unexpected I/O
|
||||
print(f"kanban repair: {exc}", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
if getattr(args, "json", False):
|
||||
print(json.dumps({
|
||||
"status": report.status,
|
||||
"db_path": str(report.db_path),
|
||||
"messages": report.messages,
|
||||
"post_repair_messages": report.post_repair_messages,
|
||||
"backup_path": (
|
||||
str(report.backup_path) if report.backup_path else None
|
||||
),
|
||||
"reindexed": report.reindexed,
|
||||
}, indent=2))
|
||||
return 0 if report.status in {"ok", "repaired", "missing"} else 1
|
||||
|
||||
if report.status == "missing":
|
||||
print(f"No kanban DB at {report.db_path} — nothing to repair.")
|
||||
return 0
|
||||
if report.status == "ok":
|
||||
print(f"{report.db_path}: integrity_check ok — no repair needed.")
|
||||
return 0
|
||||
if report.status == "repaired":
|
||||
print(f"{report.db_path}: repaired.")
|
||||
print(f" reindexed: {', '.join(report.reindexed)}")
|
||||
if report.backup_path:
|
||||
print(f" pre-repair backup: {report.backup_path}")
|
||||
print(" integrity_check now ok.")
|
||||
return 0
|
||||
# still corrupt
|
||||
print(f"{report.db_path}: CORRUPT.", file=sys.stderr)
|
||||
for line in (report.messages or [])[:10]:
|
||||
print(f" {line}", file=sys.stderr)
|
||||
if report.reindexed:
|
||||
print(
|
||||
f" REINDEX ({', '.join(report.reindexed)}) attempted but "
|
||||
f"integrity_check is still failing:",
|
||||
file=sys.stderr,
|
||||
)
|
||||
for line in (report.post_repair_messages or [])[:10]:
|
||||
print(f" {line}", file=sys.stderr)
|
||||
else:
|
||||
print(
|
||||
" Not an index-only failure — automatic REINDEX repair does "
|
||||
"not apply (fail-closed).",
|
||||
file=sys.stderr,
|
||||
)
|
||||
if report.backup_path:
|
||||
print(f" corrupt copy quarantined at: {report.backup_path}",
|
||||
file=sys.stderr)
|
||||
print(
|
||||
" Recover manually (e.g. `sqlite3 kanban.db \".recover\"` into a "
|
||||
"fresh file) or move the file aside to start a new board.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Slash-command entry point (used by /kanban from CLI and gateway)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1907,6 +1907,112 @@ def _guard_existing_db_is_healthy(path: Path) -> None:
|
|||
raise KanbanDbCorruptError(resolved, backup, reason)
|
||||
|
||||
|
||||
@dataclass
|
||||
class RepairResult:
|
||||
"""Outcome of :func:`repair_db` for CLI/status reporting.
|
||||
|
||||
``status`` is one of:
|
||||
|
||||
* ``"ok"`` — integrity_check was already clean; nothing done.
|
||||
* ``"repaired"`` — index-only errors found, REINDEX applied, re-check
|
||||
clean. ``backup_path`` holds the pre-repair quarantine copy.
|
||||
* ``"corrupt"`` — still corrupt: either a non-index error class
|
||||
(fail-closed, no repair attempted) or a REINDEX whose re-check did
|
||||
not come back clean.
|
||||
* ``"missing"`` — no DB file (or zero-byte placeholder); nothing to do.
|
||||
"""
|
||||
|
||||
status: str
|
||||
db_path: Path
|
||||
messages: list[str] = field(default_factory=list)
|
||||
post_repair_messages: list[str] = field(default_factory=list)
|
||||
backup_path: Optional[Path] = None
|
||||
reindexed: list[str] = field(default_factory=list)
|
||||
|
||||
|
||||
def repair_db(
|
||||
db_path: Optional[Path] = None,
|
||||
*,
|
||||
board: Optional[str] = None,
|
||||
) -> RepairResult:
|
||||
"""Probe a kanban DB and apply the narrow index-REINDEX repair if needed.
|
||||
|
||||
Shares the exact policy of :func:`_guard_existing_db_is_healthy`: only
|
||||
integrity failures composed *entirely* of index-scoped errors are
|
||||
repairable; the corrupt bytes are quarantined via
|
||||
:func:`_backup_corrupt_db` BEFORE any mutation; the REINDEX runs under
|
||||
the board's cross-process init flock; and anything else stays corrupt
|
||||
(fail-closed) for the caller to surface. Unlike the guard this never
|
||||
raises :class:`KanbanDbCorruptError` — it returns a structured
|
||||
:class:`RepairResult` so ``hermes kanban repair`` can report and choose
|
||||
its own exit code.
|
||||
|
||||
Transient ``sqlite3.OperationalError`` (locked/busy) still propagates
|
||||
raw, exactly like the guard: a locked healthy DB is not corruption and
|
||||
must not be quarantined.
|
||||
"""
|
||||
if db_path is not None:
|
||||
path = db_path
|
||||
else:
|
||||
path = kanban_db_path(board=board)
|
||||
try:
|
||||
resolved = path.resolve()
|
||||
except OSError:
|
||||
resolved = path
|
||||
try:
|
||||
if not resolved.exists() or resolved.stat().st_size == 0:
|
||||
return RepairResult(status="missing", db_path=resolved)
|
||||
except OSError:
|
||||
return RepairResult(status="missing", db_path=resolved)
|
||||
|
||||
with _cross_process_init_lock(resolved):
|
||||
messages: list[str] = []
|
||||
try:
|
||||
probe = _sqlite_connect(resolved)
|
||||
try:
|
||||
messages = _run_integrity_check(probe)
|
||||
finally:
|
||||
probe.close()
|
||||
except sqlite3.OperationalError:
|
||||
# Locked/busy — not corruption; let the caller report it raw.
|
||||
raise
|
||||
except sqlite3.DatabaseError as exc:
|
||||
# Same quarantine the connect-time guard takes for a file
|
||||
# sqlite refuses to open at all (e.g. malformed page 1).
|
||||
return RepairResult(
|
||||
status="corrupt",
|
||||
db_path=resolved,
|
||||
messages=[f"sqlite refused to open file: {exc}"],
|
||||
backup_path=_backup_corrupt_db(resolved),
|
||||
)
|
||||
if _integrity_messages_ok(messages):
|
||||
return RepairResult(status="ok", db_path=resolved, messages=messages)
|
||||
|
||||
# Quarantine FIRST — identical policy to the connect-time guard.
|
||||
backup = _backup_corrupt_db(resolved)
|
||||
index_names = _repairable_index_names(messages)
|
||||
if not index_names:
|
||||
return RepairResult(
|
||||
status="corrupt",
|
||||
db_path=resolved,
|
||||
messages=messages,
|
||||
backup_path=backup,
|
||||
)
|
||||
repaired, post = _attempt_index_reindex_repair(resolved, index_names)
|
||||
# The file changed on disk; force the next connect() in this process
|
||||
# to re-probe instead of trusting the stale healthy-path cache.
|
||||
with _INIT_LOCK:
|
||||
_INITIALIZED_PATHS.discard(str(resolved))
|
||||
return RepairResult(
|
||||
status="repaired" if repaired else "corrupt",
|
||||
db_path=resolved,
|
||||
messages=messages,
|
||||
post_repair_messages=post,
|
||||
backup_path=backup,
|
||||
reindexed=index_names,
|
||||
)
|
||||
|
||||
|
||||
def connect(
|
||||
db_path: Optional[Path] = None,
|
||||
*,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ and the ``hermes kanban repair`` CLI verb."""
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
|
|
@ -376,3 +377,133 @@ def test_wal_checkpoint_truncates_wal_file(tmp_path, monkeypatch):
|
|||
)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# repair_db() API + `hermes kanban repair` CLI verb
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _run_kanban_cli(argv: list[str]) -> int:
|
||||
"""Drive the real argparse surface exactly like `hermes kanban …`."""
|
||||
import argparse
|
||||
|
||||
from hermes_cli import kanban as kc
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
sub = parser.add_subparsers(dest="command")
|
||||
kc.build_parser(sub)
|
||||
args = parser.parse_args(["kanban", *argv])
|
||||
return kc.kanban_command(args)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def cli_home(tmp_path, monkeypatch):
|
||||
"""Isolated HERMES_HOME so kanban_db_path() resolves inside tmp_path."""
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path)
|
||||
return home
|
||||
|
||||
|
||||
def test_repair_db_reports_ok_on_healthy_board(tmp_path):
|
||||
db_path = tmp_path / "kanban.db"
|
||||
_build_board_db(db_path)
|
||||
report = kb.repair_db(db_path=db_path)
|
||||
assert report.status == "ok"
|
||||
assert report.messages == ["ok"]
|
||||
assert report.backup_path is None
|
||||
|
||||
|
||||
def test_repair_db_missing_file(tmp_path):
|
||||
report = kb.repair_db(db_path=tmp_path / "nope.db")
|
||||
assert report.status == "missing"
|
||||
|
||||
|
||||
def test_repair_db_repairs_index_corruption_with_backup_first(tmp_path):
|
||||
db_path = tmp_path / "kanban.db"
|
||||
_build_board_db(db_path)
|
||||
_corrupt_index(db_path, "idx_tasks_status")
|
||||
|
||||
report = kb.repair_db(db_path=db_path)
|
||||
assert report.status == "repaired"
|
||||
assert report.reindexed == ["idx_tasks_status"]
|
||||
assert report.backup_path is not None and report.backup_path.exists()
|
||||
# Backup captured the PRE-repair bytes (still corrupt in the copy).
|
||||
assert any(
|
||||
m.startswith("wrong # of entries in index")
|
||||
for m in _integrity_messages(report.backup_path)
|
||||
)
|
||||
# Live DB is clean and data intact.
|
||||
assert _integrity_messages(db_path) == ["ok"]
|
||||
conn = kb.connect(db_path=db_path)
|
||||
try:
|
||||
assert "task-0" in {t.title for t in kb.list_tasks(conn)}
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_repair_db_fail_closed_on_page_corruption(tmp_path):
|
||||
db_path = tmp_path / "kanban.db"
|
||||
original = _write_page_corrupt_db(db_path)
|
||||
report = kb.repair_db(db_path=db_path)
|
||||
assert report.status == "corrupt"
|
||||
assert report.reindexed == []
|
||||
assert report.backup_path is not None and report.backup_path.exists()
|
||||
# No REINDEX mutation happened on the live file.
|
||||
assert db_path.read_bytes() == original
|
||||
|
||||
|
||||
def test_cli_repair_ok_exit_zero(cli_home, capsys):
|
||||
kb.init_db()
|
||||
rc = _run_kanban_cli(["repair"])
|
||||
out = capsys.readouterr().out
|
||||
assert rc == 0
|
||||
assert "integrity_check ok" in out
|
||||
|
||||
|
||||
def test_cli_repair_repairs_and_exits_zero(cli_home, capsys):
|
||||
db_path = kb.kanban_db_path()
|
||||
_build_board_db(db_path)
|
||||
_corrupt_index(db_path, "idx_tasks_status")
|
||||
|
||||
rc = _run_kanban_cli(["repair"])
|
||||
out = capsys.readouterr().out
|
||||
assert rc == 0
|
||||
assert "repaired" in out
|
||||
assert "idx_tasks_status" in out
|
||||
assert "pre-repair backup" in out
|
||||
assert _integrity_messages(db_path) == ["ok"]
|
||||
|
||||
|
||||
def test_cli_repair_still_corrupt_exits_nonzero(cli_home, capsys):
|
||||
db_path = kb.kanban_db_path()
|
||||
db_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
_write_page_corrupt_db(db_path)
|
||||
|
||||
rc = _run_kanban_cli(["repair"])
|
||||
err = capsys.readouterr().err
|
||||
assert rc != 0
|
||||
assert "CORRUPT" in err
|
||||
assert "fail-closed" in err
|
||||
|
||||
|
||||
def test_cli_repair_json_shape(cli_home, capsys):
|
||||
db_path = kb.kanban_db_path()
|
||||
_build_board_db(db_path)
|
||||
_corrupt_index(db_path, "idx_tasks_status")
|
||||
|
||||
rc = _run_kanban_cli(["repair", "--json"])
|
||||
payload = json.loads(capsys.readouterr().out)
|
||||
assert rc == 0
|
||||
assert payload["status"] == "repaired"
|
||||
assert payload["reindexed"] == ["idx_tasks_status"]
|
||||
assert payload["backup_path"]
|
||||
assert Path(payload["backup_path"]).exists()
|
||||
|
||||
|
||||
def test_cli_repair_missing_db_exits_zero(cli_home, capsys):
|
||||
rc = _run_kanban_cli(["repair"])
|
||||
out = capsys.readouterr().out
|
||||
assert rc == 0
|
||||
assert "nothing to repair" in out
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue