`scripts/run_tests.sh tests/<dir>/` is how a change gets its regression
coverage run, so a test filed under the wrong directory is a test nobody
runs when that code changes. Two kinds of drift had accumulated.
Parallel directories for one source package, folded into the mirror:
tests/acp -> tests/acp_adapter (its __init__/conftest move with it)
tests/cli -> tests/hermes_cli (prompt_toolkit fixture merged into
hermes_cli/conftest.py)
tests/run_agent -> tests/agent (backoff fixture becomes
agent/conftest.py)
tests/relay -> tests/gateway/relay
tests/state -> tests/hermes_state
246 loose files at tests/ root, routed by the package they import/patch:
hermes_cli, hermes_state, agent, gateway, tools, plugins, tui_gateway, cron.
Installer and desktop-update script tests go to tests/scripts/{install,
desktop_update}/. 43 tests of root-level modules (batch_runner, utils,
hermes_constants, packaging) stay at the root.
Filenames drop their issue numbers (95 files: test_89315_x.py -> test_x.py);
the number stays in the module docstring where it has context.
Collisions: test_cli_skin_integration.py existed in both tests/ and tests/cli
with different subsets — merged into one (10 tests, all kept);
run_agent/test_pre_compress_memory_context.py -> agent/..._handoff.py;
tests/test_account_usage.py -> agent/test_account_usage_fetch.py;
tests/test_web_server.py -> hermes_cli/test_web_server_ws_ping.py.
Deleted: test_minisweagent_path.py (empty since PR #2804),
test_model_picker_scroll.py (tested a private copy of the logic, imported
nothing), test_process_loop_event_loop_warning.py (asserted asyncio behaviour,
imported nothing from Hermes).
Repo-root path arithmetic (Path(__file__).parents[N], dirname chains) is
bumped for the 202 files that changed depth and verified by evaluating every
such expression against the new location. classify_changes' desktop-updater
lane prefix, tests-os.yml's ignore glob and every in-tree path comment follow
the moves. tests/test_tests_tree_layout.py keeps the tree from drifting back.
Startup only quarantined a 0-byte / all-NUL state.db. A file whose first page
was clobbered with record bytes (#102198) went straight to sqlite3.connect,
which raised "file is not a database" and deleted the -wal sidecar — the one
piece of evidence that could have been recovered.
`has_invalid_sqlite_header_preopen` generalises the zeroed probe (zeroed is a
subset of "no SQLite header"; same live-connection contract, never raises).
`quarantine_invalid_state_db` moves the file AND its -wal/-shm aside as
`state.db.<zeroed|notadb>-<ts>-<pid>.bak` before anything opens it; a fresh
DB is created as before.
Re-authored on current main (the quarantine helpers moved to
hermes_state_dbfile.py in d15c61b5dc); one regression test proves the
notadb case is quarantined with its sidecars and the new DB passes
integrity_check.
Refs #102198 (the write-after-SIGTERM that clobbers page 0 is not addressed
here; this preserves the evidence instead of destroying it).
Follow-ups on the salvage: regular-file guard before the zeroed byte-probe (a FIFO at the state.db path would block startup forever — #98017 review P2), plus an on-main-reproducing UnicodeDecodeError fixture for #98924 (raw bytes in sqlite_master, not messages.content, are what reach pysqlite error-message decode).
- Guard against concurrent-opener race where newly created 0-byte state.db was falsely quarantined before first schema write
- Wrap startup in quarantine_cross_process_lock when database is uninitialized or zeroed
- Guard is_zeroed_sqlite_file and is_zeroed_state_db against active live connections in current process
- Add concurrent-opener and live-connection regression tests
Reviewer egilewski identified that the 5s lock timeout fell open:
quarantine_zeroed_state_db() logged 'proceeding without the cross-
process lock' and continued to re-check + rename state.db. A slow or
paused startup that still owns the lock can overlap this fallback and
the two processes can again act on the same live file.
Fix: fail closed — return None without moving the file when the lock
cannot be acquired within 5s. Log an error with recovery guidance
(restore from state-snapshots).
Test: test_quarantine_fails_closed_when_lock_held holds the cross-
process lock from a background thread, calls quarantine, and asserts
it returns None without moving the zeroed file.
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).
Hardening for the Windows zeroed-state.db class (#68474):
- Surface critical stdout when pre-update/quick snapshot cannot copy a
present *.db (was log-only; update still looked successful).
- Detect all-NUL SQLite header on SessionDB open, quarantine the bytes,
and open a fresh DB with recovery guidance to state-snapshots.
Does not claim storage-stack root cause.
ouyangbo
teknium1