fix(cli): hermes sessions list|stats|pinned and a plain hermes doctor no longer open state.db as a writer (#110173, salvage #110186) - #111627
Merged
kshitijk4poor merged 6 commits intoSep 15, 2026
Conversation
## What does this PR do? Makes observational CLI commands open `state.db` in read-only mode, so they can inspect a live Hermes installation without participating in writable WAL lifecycle handling. ### Symptom Running `hermes status`, `hermes doctor` without `--fix`, `hermes sessions list`, `hermes sessions stats`, or `hermes insights` while a gateway owns the store could open another writable session handle. The live turn could then lose its WAL generation and stop. ### Impact Users inspecting status or session history during an active turn could lose that in-flight turn and leave the gateway halted until recovery. ### Bug Cause **Trigger:** observational CLI helpers constructed `SessionDB()` with its writable default. **Causal chain:** 1. A live gateway holds the `state.db` WAL generation. 2. A nested observational CLI command opens a second writable handle. 3. Writable-handle close behavior can participate in WAL lifecycle work and retire the generation used by the live writer. **Why it is wrong:** these commands only query state and should not have writer privileges. **Working sibling / contrast:** repair and mutating session commands still use writable access intentionally. **Ruled out:** no state schema, migration, or WAL checkpoint implementation changes are included. ### Fix Routes status, non-fixing doctor state inspection, sessions list/stats, and both insights entrypoints through `SessionDB(read_only=True)`. Repair and mutating paths remain writable, and regression tests cover WAL preservation with a live writer. ## Related Issue Fixes NousResearch#110173 ## Type of Change - ✅ Bug fix (non-breaking change that fixes an issue) ## Changes Made - `hermes_cli/status.py`, `hermes_cli/doctor_state.py`, and insights helpers — open observational state readers read-only. - `hermes_cli/sessions_cmd.py` — make only `list` and `stats` read-only; retain writable access for mutations. - `tests/hermes_cli/test_observational_sessiondb_modes.py` — verify access modes and a live writer's WAL remains usable. ## How to Test - ✅ `scripts/run_tests.sh tests/hermes_cli/test_observational_sessiondb_modes.py tests/hermes_cli/test_cli_insights_command.py` — 9 passed. - ✅ `scripts/run_tests.sh tests/hermes_cli/test_doctor.py tests/hermes_cli/test_doctor_structural_corruption.py tests/hermes_cli/test_sessions_error_exit_codes.py` — 75 passed; two sandbox-only failures came from blocked host process/symlink operations. - ✅ A live `SessionDB` writer remains able to create and retrieve a session after `sessions stats` reads the store. ## Checklist ### Code - ✅ I've read the Contributing Guide - ✅ My commit messages follow Conventional Commits - ✅ I searched for existing PRs to make sure this isn't a duplicate - ✅ My PR contains only changes related to this fix - ✅ I've run relevant tests locally (see How to Test) - ✅ I've added tests for my changes - ✅ I've tested on my platform: macOS ### Documentation & Housekeeping - ✅ Documentation update: N/A - ✅ `cli-config.yaml.example`: N/A - ✅ `CONTRIBUTING.md` or `AGENTS.md`: N/A - ✅ Cross-platform impact considered - ✅ Tool descriptions/schemas: N/A
- _session_count: back to main's raw sqlite mode=ro COUNT(*) via as_uri() — routing it through SessionDB(read_only=True) both re-introduced the raw f-string URI ('?'/'#' in the home path truncate it) and queries columns (s.archived) an unmigrated store lacks, so doctor would report a healthy DB as broken.
- _write_health_reason: snapshot source URI built with as_uri() for the same reason; the --fix live probe (_db_opens_cleanly runs BEGIN IMMEDIATE) now falls back to the snapshot unless live_writer_holds_db proves the store quiet, matching _state_db_wal — hermes doctor --fix never becomes a second writer against a gateway's state.db (NousResearch#103339).
- SessionDB._connect_read_only: same as_uri() form so every read-only opener is safe in a home containing '?' or '#'.
- test_sessions_export_output_dir: fixture accepts the read_only kwarg the PR introduced.
- Drop the two doctor tests that pinned the SessionDB factory kwargs; main's URI-reserved-chars test covers _session_count.
Co-authored-by: Ahmett101 <Ahmett101@users.noreply.github.com>
- Keep (a) list/stats/pinned open SessionDB(read_only=True) — one parametrized test — and (b) a missing store prints empty results and is never created. - Drop the insights read-only test (already covered on main), the status test, the mutating-action/live-writer/doctor-isolation tests, and the two doctor factory tests. - Replace _EmptyObservationalStore + error-string sniffing with a plain '_default_db_path() does not exist' branch printing each action's empty output; the fake-store tests in test_sessions_pin keep working because the branch only runs when the open fails.
…apshot only read_only_db_uri() replaces four inline mode=ro URI sites (two of which still used the raw f-string that truncates on ?/# in the home path: state_db_has_structural_damage and collect_state_db_stats). The doctor write probe now applies the live-holder gate in both modes: a quiet store is probed in place as on main, a held store is probed through a read-only snapshot, and a held store over 1 GB is skipped with an info line unless --fix is given (the unconditional copy cost one full DB write per plain doctor run). Connect/backup failures propagate to the existing classification instead of being reported as FTS write-health failures. Observational sessions commands print a migration hint instead of a raw traceback when a read-only opener meets an older schema. Co-authored-by: Ahmett101 <Ahmett101@users.noreply.github.com>
… migration Other OperationalErrors (locked, disk I/O) keep their real message.
alt-glitch
added
type/bug
…tor/repair helpers
hermes_state_common pulls in agent.* at import, so the URI builder moves to
hermes_state_holders (errno/os/sqlite3/pathlib only) where the gateway
readiness probe and backup can adopt it in a follow-up sweep. The doctor
structural-damage branch is one helper instead of two copies, the holder
scan goes through hermes_state_repair._live_writer_holds_db, the migration
hint uses _schema_not_built (the startswith("no such ") check also matched
"no such module: fts5"), and the hermes_state import is hoisted so an import
failure cannot mask itself as UnboundLocalError.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Observational CLI commands inspect a live install without joining the writable WAL lifecycle:
hermes sessions list/stats/pinnedopenstate.dbread-only, andhermes doctor(without--fix) probes write health through a read-only snapshot when a gateway holds the store.Fixes #110173 (remaining half;
hermes status/insightslanded via #110718). Based on #110186 by @KoNit-K (cherry-picked to preserve authorship); #109725 by @Ahmett101 proposed thesessionshalf earlier and is credited as co-author.hermes_cli/sessions_cmd.py:_OBSERVATIONAL_DB_ACTIONSopenSessionDB(read_only=True); a fresh profile with nostate.dbprints the empty result instead of creating the store; an older schema gets a migration hint instead of a traceback.hermes_cli/doctor_state.py:_write_health_reasonprobes in place when the holder scan proves the store quiet (as main did), through a read-onlybackup()snapshot when a live writer holds it, and skips (info line) a held store over 1 GB unless--fix. Connect/backup failures reach the existing classification instead of being reported as FTS corruption.hermes_state_common.read_only_db_uri()replaces four inlinemode=roURIs — two of them (state_db_has_structural_damage,collect_state_db_stats) still used the raw f-string that truncates on?/#in the home path (the class fixed in 9b419a2).Behaviour changes to note:
sessions list/stats/pinnedno longer create or migratestate.db; a plainhermes doctorcopies a HELD store (≤ 1 GB) to a temp dir before the write probe.Validation:
scripts/run_tests.shon 18 files — green except twotest_hermes_state.py::TestPerformancePragmasEndToEndcases and onetest_doctor_structural_corruption.pycase that fail identically on pristine origin/main under this runner (fd-limit / SQLite-version environment). Real-import probe: read-only open in aq?h#zhome works and INSERT raises; old URI form →no such table(the truncation). Mutation (main'ssessions_cmd.py+doctor_state.py): 5 failed → restored: 5 passed.