Skip to content

fix(cli): hermes insights opens state.db read-only (salvage #109737) - #110026

Closed
kshitijk4poor wants to merge 5 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/109737-insights-readonly
Closed

kshitijk4poor wants to merge 5 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/109737-insights-readonly

Conversation

@kshitijk4poor

Copy link
Copy Markdown

hermes insights and /insights are readers; they no longer open state.db as a writer.

Salvage of #109737 by @B0on (cherry-picked with authorship preserved, reduced to the insights conversion), then three follow-up commits.

Changes

  • hermes_cli/cli_info_mixin.py and hermes_cli/main_agent_cmds.py: SessionDB(read_only=True) (main's existing read-only path).
  • A fresh install with no state.db prints No session data yet. instead of Error generating insights: unable to open database file (the read-only open requires the file; the writer path used to create it). The check uses hermes_state._default_db_path(), the same resolver the constructor uses.
  • 1 test covering both entry points and both states.

Why the subset: #109737 also pinned every healthy writer open on Python 3.11 when other holders exist and added a NO_CKPT wrapper. The field failure it targeted (a sibling CLI close unlinking the live gateway's WAL) was root-caused and fixed by e16f686 (#109841), and the pin leaks one connection per transient SessionDB in long-lived processes. The read-only hygiene is what remains worth landing.

Validation

Check Result
scripts/run_tests.sh tests/cli/test_cli_insights_command.py 4 passed
same test with main's copy of either prod file 1 failed
real-import probe, both entry points, temp HERMES_HOME main: read_only flags [False, False]; branch: [True, True]

Follow-up (not this PR): samalone's report on #109737 that tui_gateway/server.py::_profile_db acquires a writer on a foreign profile's live store.

kshitijk4poor and others added 5 commits September 13, 2026 21:01
`hermes insights` and the `/insights` slash command are pure readers,
but they constructed `SessionDB()` read-write, so a one-shot CLI
invocation took a writer connection on the live gateway's state.db.

The field failure that motivated NousResearch#109737 (a sibling CLI close unlinking
the live gateway WAL) was root-caused and fixed in e16f686 (NousResearch#109841);
the Python 3.11 pin-every-writer branch and the NO_CKPT wrapper from the
original PR are unnecessary and leak a connection per transient
SessionDB, so they are not carried here. What remains is the hygiene:
a reader must not open the store read-write. `SessionDB(read_only=True)`
is the existing read-only path on main (NousResearch#77627).

Salvaged subset of NousResearch#109737.

Co-authored-by: Cursor <cursoragent@cursor.com>
Both `/insights` and `hermes insights` must open SessionDB read-only for
the same reason, so one parametrised-by-loop test keeps the invariant
count at one per fix and avoids two near-identical bodies drifting apart.
SessionDB(read_only=True) requires an existing state.db (the writer path
used to create it), so a brand-new install would have printed
"Error generating insights: unable to open database file". Check for
the file first, as update_cmd_maint does before its read-only open.
hermes_state._default_db_path() honours a re-pointed DEFAULT_DB_PATH;
re-deriving get_hermes_home()/state.db could disagree with the read-only
open one line later (the test sandbox re-points it, which is how the
mismatch surfaced). Same pre-check web_routers/status.py already uses.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 13, 2026
yoniebans added a commit that referenced this pull request Sep 14, 2026
SessionDB(read_only=True) cannot create a missing store, so a fresh install
running hermes insights / /insights errored instead of reporting no data
(reported by @ehz0ah on #110718; guard shape from @kshitijk4poor's #110026).

Co-authored-by: kshitijk4poor <kshitijk4poor@users.noreply.github.com>
teknium1 pushed a commit that referenced this pull request Sep 14, 2026
SessionDB(read_only=True) cannot create a missing store, so a fresh install
running hermes insights / /insights errored instead of reporting no data
(reported by @ehz0ah on #110718; guard shape from @kshitijk4poor's #110026).

Co-authored-by: kshitijk4poor <kshitijk4poor@users.noreply.github.com>
@teknium1

Copy link
Copy Markdown
Collaborator

The insights read-only conversion landed on main via #110718 (9d75f20) with @B0on's commit kept under his authorship, plus the missing-store short-circuit. Redundant now; closing with thanks @kshitijk4poor.

@teknium1 teknium1 closed this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants