Skip to content

fix(desktop): derive the stale-schema read probe from SCHEMA_SQL - #80797

Merged
jquesnelle merged 1 commit into
mainfrom
fix/sidebar-stale-schema-probe
Aug 7, 2026
Merged

fix(desktop): derive the stale-schema read probe from SCHEMA_SQL#80797
jquesnelle merged 1 commit into
mainfrom
fix/sidebar-stale-schema-probe

Conversation

@jquesnelle

Copy link
Copy Markdown
Collaborator

After hermes update, the desktop sidebar showed "No sessions yet" until the user's first message. #72424 added sessions.last_activity_at, which list_sessions_rich now selects — but column adds only land through _reconcile_columns() in the writable _init_schema, and read-only opens skip that by design. Every sidebar read path opens state.db read-only, so each poll raised "no such column: s.last_activity_at" until the first prompt's lazy session-row persist forced a writable open and reconciled.

A heal for exactly this class already existed (_open_session_db_for_profile probes the read-only handle and does a one-time writable reopen on staleness), but its probe was a hand-written four-column list that never learned last_activity_at — it went stale three days after shipping. And the batched sidebar route (/api/profiles/sessions/sidebar) bypassed the helper entirely, swallowing per-profile failures into an errors array the desktop never surfaces, so the incident produced an empty sidebar with clean logs.

The fix removes the maintenance burden instead of paying it once more:

  • hermes_state_schema.schema_read_probe_statements() derives one SELECT <every declared column> FROM <table> LIMIT 0 per table from SCHEMA_SQL via the existing _parse_schema_columns() — the same source of truth the writable reconciler diffs against, so any future ADD COLUMN is probed with no list to update. Column references are table-qualified: an unqualified double-quoted identifier that fails to resolve silently degrades to a string literal (SQLite's double-quoted-string misfeature) and would make the probe pass on exactly the store it exists to catch.

  • web_server splits the heal into a path-level _open_session_db_at_path (semantics unchanged) so the cross-profile session routes can share it; both profiles.py loops and _count_status_active_sessions (the remaining raw read-only sibling) now open through it. The heal stays a helper rather than a SessionDB classmethod on purpose: escalation-to-writable must remain an explicit caller decision — update_cmd.py opens read-only mid-update and must never write.

  • Exhaustion guard: if the writable heal SUCCEEDS and the re-probe still fails (a schema problem ADD COLUMN cannot express), the store is marked exhausted — warn once, skip the probe, serve reads probe-less — instead of re-running the full writable init on every poll against a possibly live DB. A FAILED writable open (transient lock) is deliberately not recorded, so the next poll retries the heal.

  • The per-profile swallow sites in profiles.py now also log a deduplicated warning, so a persistent read failure is loud in errors.log even though the response errors array stays invisible to the sidebar.

Tests: probe/SCHEMA_SQL coverage invariants (tests/test_schema_read_probe.py), last_activity_at added to the /api/sessions heal parametrize, a sidebar-route heal test reproducing the shipped symptom (errors == [] and the session returned against a store missing the column), and an exhaustion test pinning exactly one writable open. The sidebar and last_activity_at tests fail on main.

After `hermes update`, the desktop sidebar showed "No sessions yet" until
the user's first message. #72424 added sessions.last_activity_at, which
list_sessions_rich now selects — but column adds only land through
_reconcile_columns() in the writable _init_schema, and read-only opens
skip that by design. Every sidebar read path opens state.db read-only, so
each poll raised "no such column: s.last_activity_at" until the first
prompt's lazy session-row persist forced a writable open and reconciled.

A heal for exactly this class already existed (_open_session_db_for_profile
probes the read-only handle and does a one-time writable reopen on
staleness), but its probe was a hand-written four-column list that never
learned last_activity_at — it went stale three days after shipping. And the
batched sidebar route (/api/profiles/sessions/sidebar) bypassed the helper
entirely, swallowing per-profile failures into an errors array the desktop
never surfaces, so the incident produced an empty sidebar with clean logs.

The fix removes the maintenance burden instead of paying it once more:

- hermes_state_schema.schema_read_probe_statements() derives one
  `SELECT <every declared column> FROM <table> LIMIT 0` per table from
  SCHEMA_SQL via the existing _parse_schema_columns() — the same source of
  truth the writable reconciler diffs against, so any future ADD COLUMN is
  probed with no list to update. Column references are table-qualified:
  an unqualified double-quoted identifier that fails to resolve silently
  degrades to a string literal (SQLite's double-quoted-string misfeature)
  and would make the probe pass on exactly the store it exists to catch.

- web_server splits the heal into a path-level _open_session_db_at_path
  (semantics unchanged) so the cross-profile session routes can share it;
  both profiles.py loops and _count_status_active_sessions (the remaining
  raw read-only sibling) now open through it. The heal stays a helper
  rather than a SessionDB classmethod on purpose: escalation-to-writable
  must remain an explicit caller decision — update_cmd.py opens read-only
  mid-update and must never write.

- Exhaustion guard: if the writable heal SUCCEEDS and the re-probe still
  fails (a schema problem ADD COLUMN cannot express), the store is marked
  exhausted — warn once, skip the probe, serve reads probe-less — instead
  of re-running the full writable init on every poll against a possibly
  live DB. A FAILED writable open (transient lock) is deliberately not
  recorded, so the next poll retries the heal.

- The per-profile swallow sites in profiles.py now also log a deduplicated
  warning, so a persistent read failure is loud in errors.log even though
  the response errors array stays invisible to the sidebar.

Tests: probe/SCHEMA_SQL coverage invariants (tests/test_schema_read_probe.py),
last_activity_at added to the /api/sessions heal parametrize, a sidebar-route
heal test reproducing the shipped symptom (errors == [] and the session
returned against a store missing the column), and an exhaustion test pinning
exactly one writable open. The sidebar and last_activity_at tests fail on
main.
@jquesnelle jquesnelle changed the title fix(dashboard): derive the stale-schema read probe from SCHEMA_SQL fix(desktop): derive the stale-schema read probe from SCHEMA_SQL Aug 7, 2026
@OutThisLife
OutThisLife self-requested a review August 7, 2026 04:45
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on bdee489

⚠️ Warnings

OSV vulnerability scan · View job

50 known vulnerabilities found in pinned dependencies.

How to fix:

Review the findings in the Security tab. Update the affected dependencies if a patched version is available.


debug info

CI timings

CI timings · View report · View job

Wall time 4m1s vs 6m45s (-40.5%). 11 job(s) slower, 9 faster, 3 unchanged.

  • Python tests / Run tests slice 8/12: -28.0s
  • Python tests / Run tests slice 11/12: +25.0s
  • Python tests / Run tests slice 2/12: -23.0s
  • Python tests / Run tests slice 3/12: -20.0s
  • Python tests / Run tests slice 5/12: +19.0s

@teknium1

teknium1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review

Verified the premise, the mechanism, and the tests locally against current main (merge-base is 8 commits behind, no conflicts, applies clean).

Premise check — confirmed

  • On main, _SESSION_DB_READ_PROBE_SQL is still the hand-written four-column list (archived, pinned, active, compacted) at hermes_cli/web_server.py:11202 — it indeed never learned sessions.last_activity_at from fix(gateway): gateway sessions lack activity watchdog - agent loop stalls silently with no detection and notification #72424, so a store predating that column passes the probe and then 500s inside list_sessions_rich on every read-only poll.
  • Both profiles.py loops (/api/profiles/sessions and the batched /sidebar route) open SessionDB(db_path=..., read_only=True) raw on main, bypassing the heal entirely and swallowing failures into the errors array the desktop never renders. Confirmed at hermes_cli/web_routers/profiles.py:138 and :293. The "empty sidebar with clean logs" failure mode is real.
  • Red→green verified: checked out the PR's tests onto origin/maintest_get_sessions_heals_stale_schema_store[last_activity_at], test_profiles_sidebar_heals_stale_schema_store, and test_heal_gives_up_when_reconcile_cannot_fix_the_store all fail on main and pass on the branch. Full tests/hermes_cli/test_web_server.py + tests/test_schema_read_probe.py run: 146/146 green on the branch.

Design — right direction

  • Deriving the probe from SCHEMA_SQL via the existing _parse_schema_columns() kills the class, not the instance: the reconciler and the probe now diff against the same source of truth, so the next ADD COLUMN can't go stale. This is exactly the "fix the class" posture we want (the hand-written list went stale in three days).
  • The table-qualified column references guarding against SQLite's double-quoted-string-literal misfeature is a subtle and correct catch — an unqualified "last_activity_at" would silently degrade to a string literal and make the probe pass on precisely the stale store it exists to detect. The test asserting the failure names sessions.last_activity_at specifically (not just any error) pins this.
  • Keeping the heal a web_server helper rather than a SessionDB classmethod is the right boundary — update_cmd.py's read-only mid-update open must never escalate to writable, and making escalation a caller decision preserves that.
  • The exhaustion guard is the right shape: a schema problem ADD COLUMN can't express should not re-trigger a full writable init per sidebar poll against a live DB, and deliberately NOT recording a failed writable open (transient lock) so the next poll retries is the correct asymmetry.
  • LIMIT 0 probes resolve columns at prepare time and read zero rows — per-open cost is negligible.

Minor notes (none blocking)

  1. _session_db_heal_exhausted / _session_db_heal_warned are plain module-level sets mutated from request threads without a lock. Worst case is a benign race (duplicate warning, one redundant writable open) — acceptable for this path, just noting it's deliberate.
  2. Exhaustion keys on str(db_path) without normalization; all current callers build the path identically so this is fine today, but db_path.resolve() would make the key immune to a future caller passing an equivalent-but-differently-spelled path.
  3. One remaining raw read-only sibling stays unconverted: the FTS-rebuild status probe at web_server.py:~3300. It's wrapped in a blanket except Exception: pass so it can't surface the symptom, but it will silently return no rebuild status on a stale store until something else heals it. Fine to leave; worth remembering if an FTS-status staleness report ever comes in.
  4. tools/session_search_tool.py opens other profiles' stores read-only too and can hit the same stale-schema failure — out of scope here (heal-by-writing from an agent tool is a different risk decision than the dashboard), but it's the same class one ring out.

Duplicate cluster

This bug attracted a swarm — at least five open PRs on the same class: #79200 (@cipher416, Aug 5, earliest — degrades queries in hermes_state instead of healing), #80030 (@Tilly-YL, draft, closest in approach: extends the heal + covers profiles.py), #80047 (@ygd58, extends the hand-written probe list — the exact maintenance treadmill this PR removes), #80126 (@JuizSpeaking), #80237 (@shannonsands). This PR supersedes all of them structurally: derived-probe beats both list-extension and query-degradation (which would silently drop last_activity_at ordering rather than heal). When this merges, the cluster should be closed with credit — several of them (esp. #79200 and #80030) diagnosed the same root cause first.

Verdict

Correct root-cause fix with the anti-staleness contract pinned by invariant tests (not change-detectors), red→green verified, sibling call paths covered, and the one intentionally-excluded caller (update_cmd.py) excluded for a documented reason. CI is green across the test matrix. Ready to merge pending Teknium's approval; recommend closing the five-PR duplicate cluster with credit to the earlier submitters afterward.

@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 comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/sessions Session lifecycle, resume, persistence, history needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 7, 2026
@jquesnelle
jquesnelle merged commit db407c8 into main Aug 7, 2026
47 checks passed
@jquesnelle
jquesnelle deleted the fix/sidebar-stale-schema-probe branch August 7, 2026 04:56
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…tale-schema-probe

fix(desktop): derive the stale-schema read probe from SCHEMA_SQL
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 comp/dashboard Web dashboard / control panel UI (dashboard/, landing) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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