Reviewer egilewski identified two recovery-loss paths:
Path A — quarantine race (hermes_state.py):
SessionDB checks state.db before quarantine_zeroed_state_db() without a
shared cross-process lock, and Path.rename() may replace an existing
destination. Two writable startups can therefore move the first
instance's newly created database over the .zeroed-*.bak, erase the
original damaged-file evidence, and replace the live database with
another empty one.
Fix: add a cross-process lock (msvcrt on Windows, fcntl on POSIX)
around quarantine_zeroed_state_db() with a 5s bounded timeout. Under
the lock: re-check is_zeroed_state_db (another process may have already
quarantined it and created a fresh DB), use a PID-suffixed unique
destination, and non-clobbering rename with counter fallback.
Path B — size-cap pruning gap (hermes_cli/backup.py):
_too_large() runs before failed-database tracking. With keep=1 and a
size cap, an oversized state.db is omitted while failed_dbs stays empty,
so automatic pruning deletes the older complete snapshot that may
contain the only recoverable database.
Fix: track oversized DB files in a new oversized_skipped list (both in
the directory walk and top-level file loop). The manifest now records
oversized_skipped. Pruning is suppressed when failed_dbs or
oversized_skipped is non-empty, preserving the older complete snapshot
as recovery source.
Tests:
- test_concurrent_quarantine_no_clobber: two threads racing on the
same zeroed state.db — verifies quarantine backup survives with
original bytes and live DB is valid.
- test_oversized_db_suppresses_pruning: keep=1 + oversized state.db
verifies the older complete snapshot is not pruned.
All 33 tests pass (3 zeroed_state_db + 25 TestQuickSnapshot + 5
quarantine_forensic_logging).
Jasmine Naderi
6048696ed0