fix(tools): heal stale cross-profile schema in session_search's read paths - #81004
Open
pierrenode wants to merge 1 commit into
Open
pierrenode wants to merge 1 commit into
pierrenode wants to merge 1 commit into
Conversation
…paths hermes_cli.web_server._open_session_db_at_path (bdee489, merged today) fixed dashboard read routes hard-failing on a profile's state.db that predates a schema addition -- read-only opens skip SessionDB's column- reconciliation path by design, so a store untouched since the last `hermes update` raises "no such column" until something opens it writable. The fix derives its read probe from SCHEMA_SQL so any future column is covered automatically, and does a one-time writable reopen to heal a stale store before serving reads. tools/session_search_tool.py's two cross-profile read-only opens -- _resolve_profile_db() (session_search(profile=...), used for @session:<profile>/<id> link resolution) and _locate_session_db() (the bare-id fallback scan across every profile) -- were never migrated to this heal and construct a raw SessionDB(read_only=True) directly. A maintainer-bot review comment on a since-superseded competing PR (NousResearch#79200, closed in favor of NousResearch#80797/bdee48928) independently confirmed "search" as one of the surfaces this bug class affects. Reproduced: a profile whose store predates a schema column, browsed cross-profile via session_search(profile=...), fails with "Failed to list recent sessions: no such column: last_activity_at" instead of returning results -- the exact dashboard-sidebar symptom bdee489 just fixed, on the agent-facing tool surface instead. Fix adds _open_cross_profile_session_db() to tools/session_search_tool.py, mirroring hermes_cli.web_server._open_session_db_at_path's bootstrap-if- missing + probe-then-one-time-writable-reopen contract, and routes both call sites through it. Deliberately NOT sharing state with web_server's copy: the dashboard and agent are separate OS processes in normal operation, so there's no process memory to coordinate a single registry through -- each side owns its own bootstrap-lock/heal-exhausted set. hermes_cli.web_server itself sits behind the optional `web` extra (fastapi), so importing it from this core tool module would break minimal installs; the shared piece that IS safe to import, hermes_state_schema.schema_read_probe_statements(), has no such dependency. Testing: - Added TestCrossProfileStaleSchemaHeal to tests/tools/test_session_search.py: browse-mode heal (the reproduced crash), locate-scan wiring (defense- in-depth -- this specific column doesn't happen to break that call chain, documented honestly in the test's own docstring), and a direct unit test of the heal-exhausted give-up path mirroring hermes_cli.web_server's own equivalent test. - Mutation-verified: reverting just the two call sites to a bare SessionDB(read_only=True) reproduces the exact original error ("no such column: last_activity_at") in the browse-mode test. - Full tests/tools/test_session_search.py (42), tests/test_schema_read_probe.py, tests/hermes_cli/test_web_server.py, and tests/test_hermes_state.py (333 total) all pass. - ruff check clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes_cli.web_server._open_session_db_at_path(bdee48928, merged today) fixed dashboard read routes hard-failing on a profile'sstate.dbthat predates a schema addition — read-only opens skipSessionDB's column-reconciliation path by design, so a store untouched since the lasthermes updateraises "no such column" until something opens it writable. The fix derives its read probe fromSCHEMA_SQLso any future column is covered automatically, and does a one-time writable reopen to heal a stale store before serving reads.tools/session_search_tool.py's two cross-profile read-only opens —_resolve_profile_db()(session_search(profile=...), used for@session:<profile>/<id>link resolution) and_locate_session_db()(the bare-id fallback scan across every profile) — were never migrated to this heal and construct a rawSessionDB(read_only=True)directly. A maintainer-bot review comment on a since-superseded competing PR (#79200, closed in favor of #80797/bdee48928) independently confirmed "search" as one of the surfaces this bug class affects.Reproduced: a profile whose store predates a schema column, browsed cross-profile via
session_search(profile=...), fails with"Failed to list recent sessions: no such column: last_activity_at"instead of returning results — the exact dashboard-sidebar symptombdee48928just fixed, on the agent-facing tool surface instead.Fix
Adds
_open_cross_profile_session_db()totools/session_search_tool.py, mirroringhermes_cli.web_server._open_session_db_at_path's bootstrap-if-missing + probe-then-one-time-writable-reopen contract, and routes both call sites through it. Deliberately not sharing state withweb_server's copy: the dashboard and agent are separate OS processes in normal operation, so there's no process memory to coordinate a single registry through — each side owns its own bootstrap-lock/heal-exhausted set.hermes_cli.web_serveritself sits behind the optionalwebextra (fastapi), so importing it from this core tool module would break minimal installs; the shared piece that IS safe to import,hermes_state_schema.schema_read_probe_statements(), has no such dependency.Testing
TestCrossProfileStaleSchemaHealtotests/tools/test_session_search.py: browse-mode heal (the reproduced crash), locate-scan wiring (defense-in-depth — this specific column doesn't happen to break that call chain, documented honestly in the test's own docstring), and a direct unit test of the heal-exhausted give-up path mirroringhermes_cli.web_server's own equivalent test.SessionDB(read_only=True)reproduces the exact original error ("no such column: last_activity_at") in the browse-mode test.tests/tools/test_session_search.py(42),tests/test_schema_read_probe.py,tests/hermes_cli/test_web_server.py, andtests/test_hermes_state.py(333 total) all pass.ruff checkclean.