Skip to content

fix(cli): open remaining observational session stores read-only - #110252

Closed
Halldrix wants to merge 1 commit into
NousResearch:mainfrom
Halldrix:fix/observational-sessiondb-readonly-remaining
Closed

Halldrix wants to merge 1 commit into
NousResearch:mainfrom
Halldrix:fix/observational-sessiondb-readonly-remaining

Conversation

@Halldrix

Copy link
Copy Markdown
Contributor

What does this PR do?

Opens the remaining observational CLI session stores read-only, so nested inspection commands never mint a second writable WAL handle while a gateway owns the store.

Complements the read-only conversion family — #109725 (sessions), #110026 (insights, salvage of #109737), #110186 (status/doctor/sessions/insights) — covering only the openers outside all three diffs: main._session_db() (MRU search, title/ID resolve, cwd restore), terminal breadcrumb resolution, the TUI exit summary, and console sessions list/stats/export. Console mutating paths (rename, optimize, repair) stay writable via a split helper.

Draft status: opened as a draft while the sibling family (#109725 / #110026 / #110186) resolves landing order. No review requested yet; CI on this fork head reports no checks until a maintainer approves the run.

Symptom

Running hermes -c <name>, resolving a bare -c breadcrumb, exiting the TUI, or running console sessions list during a live gateway turn opened another writable handle. Close-time WAL lifecycle work on that handle could retire the live writer's generation and halt the turn.

Bug Cause

Trigger: these helpers constructed SessionDB() with its writable default.

Causal chain:

  1. A live gateway holds the state.db WAL generation.
  2. A nested observational command (-c resolve, breadcrumb, TUI epilogue, console list) opens a second writable handle.
  3. The extra handle's close participates in WAL lifecycle handling and can retire the live generation.

Why it is wrong: these paths only query state and must not hold writer privileges.

Ruled out: no schema, migration, checkpoint, or recovery-transaction changes; intentional writers (--create-if-missing, oneshot turns, foreign import, kanban retag) keep writable access.

Sibling audit (same shape, every path): main.py:1341 (titled-session create), oneshot.py:274 (live turn persistence), foreign_sessions.py:246 (import writes), kanban_db_dispatch.py:2085 (one-shot retag write) are writers by contract and stay writable. Console rename/optimize/repair keep the writable helper.

Related Issue

Related to #110173 (observational-opener class). Complements #109725 / #110026 / #110186 — disjoint file set, no overlap. No Fixes claimed; a maintainer can link as preferred.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/main.py — _session_db() yields SessionDB(read_only=True) (MRU search, title/ID resolve, cwd restore are all reads).
  • hermes_cli/terminal_breadcrumbs.py — resolve_breadcrumb_session() opens read-only (get_session + get_compression_tip only).
  • hermes_cli/main_tui_launch.py — _print_tui_exit_summary() opens read-only (get_session + get_session_title only).
  • hermes_cli/console_engine.py — new _session_db_readonly() for sessions list, sessions stats, sessions export (export guard is assert-only); fails closed with "No session database yet" instead of minting a store when none exists. rename, optimize, repair keep the writable helper.
  • tests/hermes_cli/test_observational_readonly_remaining.py — 9 tests: mode assertions per entrypoint, missing-db fail-closed without minting, live-writer WAL preservation. Sabotage (fix reverted to base): 7 fail / 2 pass; restored: 9 pass.
  • tests/hermes_cli/test_resolve_last_session.py — forwards kwargs in the SessionDB fake (lambda *a, **k) so the read-only kwarg reaches the real constructor.

How to Test

scripts/run_tests.sh tests/hermes_cli/test_observational_readonly_remaining.py
scripts/run_tests.sh tests/hermes_cli/test_console_engine.py tests/hermes_cli/test_terminal_breadcrumbs.py tests/hermes_cli/test_resolve_last_session.py tests/hermes_cli/test_resume_latest_and_in_dir.py tests/hermes_cli/test_cli_resume_command.py tests/hermes_cli/test_tui_resume_flow.py tests/hermes_cli/test_fts_optimize_notice.py tests/hermes_cli/test_resume_display.py tests/hermes_cli/test_cli_partial_update_hint.py
  • New file: 9 passed. Neighbor batch: 10 files, 93 passed, 0 failed (counts verified in the same run, not summed).
  • Live-writer proof: with a gateway-owned state.db, run -c resolve, breadcrumb resolve, console sessions list, and the TUI summary path — the live writer creates and reads a session afterwards with its generation intact.
  • Platform: Debian GNU/Linux 13 (trixie), x86_64 — Hermes Agent v0.21.2, upstream 3f86ed7.

Checklist

Code


🛠️ Dev: Halldrix
🤖 Sidekick: Hermes Agent v0.21.2
🐞 Reproduction: ✅ Confirmed (nested observational openers mint writable handles; #110173 class)

@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
@Halldrix

Copy link
Copy Markdown
Contributor Author

Superseded by #110934 (merged), which converted the same observational openers (main._session_db(), breadcrumbs, TUI exit summary, console sessions list/stats/export) to read-only, and #110544 (root-cause lockguard) which closed #110173. Closing as redundant; thanks for the audit coverage.

@Halldrix Halldrix closed this Sep 15, 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.

2 participants