Skip to content

fix(dashboard): preserve maintenance writes on read polling (salvage #67903) - #76895

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage-67903-readonly-dashboard
Aug 2, 2026
Merged

fix(dashboard): preserve maintenance writes on read polling (salvage #67903)#76895
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage-67903-readonly-dashboard

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvages #67903 by @joelbrilliant — both commits cherry-picked to preserve authorship, plus one fix commit for a connection-leak finding from review.

Context — what this fixes, for whom

Every Desktop user's dashboard polls /api/sessions every 10s. On current main each poll opens a writable SessionDB, and close() unconditionally runs PRAGMA wal_checkpoint(TRUNCATE) — so read-only polling repeatedly checkpoints and truncates the WAL out from under the real writer (the running gateway/agent). Measured side-by-side: a writer's 6,451,952-byte pending WAL is truncated to 0 bytes by a single dashboard-style GET on main; preserved byte-identical on this branch. A second latent bug fixed here: SessionDB(read_only=True) on main never sets FTS capability flags, so read-only search_messages silently returns 0 hits (ASCII and CJK) — this branch probes existing FTS tables on RO opens and both search paths work.

What the fix does (from #67903, kept verbatim)

  • _open_session_db_for_profile now requires an explicit read_only keyword; every read endpoint (list, search, stats, detail, messages, export, analytics, cron runs, resume resolution) opens read-only; writes (delete, rename, prune, import) stay writable. An AST test enforces the keyword on every call site.
  • Read-only close() never requests a WAL checkpoint (writable close keeps TRUNCATE).
  • Read-only opens: one-time bootstrap of fresh/zero-byte stores under a lock (8-thread concurrent first-load test), stale-schema healing via one writable open (_reconcile_columns) then read-only reopen, FTS/trigram capability probing.
  • Auto-archive moved off the listing connection to its own throttled maintenance writer.

Review fix added in this salvage (commit 3)

The RO branch's new FTS probe raises sqlite3.DatabaseError on a malformed store, and __init__'s outer handler re-raised without closing the connection — leaking a tracked connection that makes _backup_db_file refuse its raw-copy for the process lifetime, so the writable heal that follows would repair the store WITHOUT its forensic backup (live-repro'd: backup files created: [] + refusal log before; backup file present after). Fixed with close-then-reraise mirroring _open_probed's own cleanup, plus a mutation-checked regression test.

Verification

  • tests/test_hermes_state.py: 146 passed (incl. the new leak-regression test); event-loop + session-search suites: 4 passed; web_server endpoint tests: 34 passed, 1 pre-existing skip
  • WAL preservation, RO FTS (ASCII+CJK), fresh-store, zero-byte, stale-schema heal, and 8-thread concurrent first-load all verified with live probes
  • Per-poll overhead measured: 0.08ms (stat + RO open + probe) on a 500K-message store
  • Monkeypatch-seam sweep: 143 adjacent tests green; ruff clean

Closes #67903 (superseded by this salvage — original author credited via cherry-pick authorship).

joelbrilliant and others added 3 commits August 2, 2026 21:24
Signed-off-by: joelbrilliant <joelbrilliant1@gmail.com>
Signed-off-by: joelbrilliant <joelbrilliant1@gmail.com>
The RO branch's new FTS capability probe raises sqlite3.DatabaseError on
a malformed store (the probe itself only catches OperationalError). The
outer __init__ handler re-raises without closing self._conn, leaking a
tracked connection for the process lifetime — which makes
_backup_db_file refuse its raw-copy, so the writable heal that follows
(web_server's stale-schema/malformed reopen) repairs the store WITHOUT
the forensic backup repair_state_db_schema promises. Close-then-reraise
on any probe failure, mirroring _open_probed's cleanup discipline.

Regression test: corrupt sqlite_master (duplicate messages_fts row),
assert the failed RO open leaves no live tracked connection and the
subsequent writable heal creates its malformed-backup file. Mutation-
checked: no-oping the cleanup handler makes the test fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants