fix(web_server): auto-migrate lagging profile DBs in /api/profiles/sessions - #42487
fix(web_server): auto-migrate lagging profile DBs in /api/profiles/sessions#42487liuhao1024 wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real regression in the read-only cross-profile path. Current main still opens each profile DB read-only at hermes_cli/web_server.py:4137, and SessionDB intentionally skips reconciliation on that path at hermes_state.py:957-974; the s.archived filters at hermes_state.py:3330-3333 can therefore hide a lagging profile behind errors[].
Problems
- The proposed broad
sqlite3.OperationalErrorcatch (hermes_cli/web_server.py:1937in this PR's diff) also catches lock and other operational failures, then opens the live profile read-write. That conflicts with the read-only path's stated no-write-lock purpose. A writableSessionDBruns full_init_schema()(hermes_state.py:992-994), not only column reconciliation. - The migration exception is swallowed, so the retry loses the original failure context.
Suggested changes
- Restrict recovery to a verified missing-column schema error; retain normal handling for lock/contention and other operational errors.
- Add a test proving a non-schema OperationalError never opens the profile writable.
- Rebase the logic manually onto the current endpoint while preserving
compact_rows=not fullathermes_cli/web_server.py:4151-4152.
Automated hermes-sweeper review.
| min_message_count=min_message_count, | ||
| include_archived=include_archived, | ||
| archived_only=archived_only, | ||
| exclude_children=True, |
There was a problem hiding this comment.
This catches every SQLite operational failure, including database is locked, then opens another profile's DB read-write. Please limit the migration retry to a verified missing-column/schema error and preserve normal degradation for lock/contention failures; the read-only aggregation exists specifically to avoid write-locking live profile DBs.
…olumn errors only Address feedback from NousResearch#42487 review: - Restrict OperationalError catch to verified missing-column schema errors (check error message for "no such column") - Retain normal handling for lock/contention and other operational errors - Preserve read-only path's no-write-lock purpose by not opening profile writable on non-schema errors - Add regression tests for auto-migration and idempotent retries Fixes: NousResearch#42487
4ef257d to
0a9b361
Compare
|
This PR would prevent a recurring multi-profile Desktop support issue. Current tree still opens profile DBs
Without this (or equivalent migrate-on-first-sidebar-load / post-update migrate-all-profiles), Desktop users still see empty sidebars after schema-bumping updates while CLI proves sessions intact. Fresh report with multi-profile repro + update-path asks: If this PR is still viable, please also cover the sidebar batched endpoint, not only the older list endpoint. |
|
Thank you for tracing the read-only migration gap. I prepared #80030 against current main, using the shared profile open helper and covering both /api/profiles/sessions and /api/profiles/sessions/sidebar. I added your name as a co-author because this follows the approach you introduced here. |
|
Closing as superseded by #80797 (merged, The stale-schema heal now lives in one place — Your premise was correct, and this is the bug that bit users after the 0.20.0 update (#79531 / #80037); the centralized version is just the wider fix. Thanks for filing it as early as you did. |
Problem
On a multi-profile install, the Desktop sidebar shows no sessions for a profile when that profile's
state.dbschema lags the default DB's schema. This happens after an update bumps the SessionDB schema — the default DB is migrated on gateway startup, but per-profile DBs are only ever opened read-only by the/api/profiles/sessionsaggregation endpoint.When
list_sessions_richqueriess.archivedagainst a DB missing that column, it raisessqlite3.OperationalError. The endpoint silently swallows the error intoerrors[], contributing zero sessions for that profile — the Desktop renders an empty sidebar with no indication of what went wrong.Closes #42467
Solution
Catch
sqlite3.OperationalErrorfrom the read-only query path. When a schema mismatch is detected:_init_schema()→_reconcile_columns()(adds missing columns viaALTER TABLE ADD COLUMN)This is a one-time cost per lagging profile. After migration, the read-only path works normally. The write lock is brief (column reconciliation only) and only happens when the schema is actually outdated.
Changes
hermes_cli/web_server.py— Wraplist_sessions_rich/session_countcalls in an inner try/except forsqlite3.OperationalError; on schema mismatch, open read-write to trigger migration, then retrytests/hermes_cli/test_profile_schema_migration.py— 2 new tests:test_profiles_sessions_auto_migrates_lagging_schema— verifies a lagging profile DB returns sessions after auto-migrationtest_profiles_sessions_migration_is_idempotent— verifies second call works without re-migrationTesting