Skip to content

fix: state_sync.py -- correct class name, constructor type, title API, connection leak#95

Merged
nesquena-hermes merged 1 commit intomasterfrom
fix/state-sync-bugs
Apr 5, 2026
Merged

fix: state_sync.py -- correct class name, constructor type, title API, connection leak#95
nesquena-hermes merged 1 commit intomasterfrom
fix/state-sync-bugs

Conversation

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Fix three bugs in api/state_sync.py found during code review of PR #93.

The original state_sync.py referenced the wrong class name, passed the wrong type to the constructor, used a private method with the wrong call signature, and leaked SQLite connections. None of these surfaced in tests because the try/except blocks catch silently -- the feature would have been a complete no-op on every real install.

Changes:

  • from hermes_state import SessionDB (was HermesState -- class does not exist)
  • SessionDB(db_path) (was str(db_path) -- SessionDB.init takes Path, not str)
  • db.set_session_title(session_id, title) (was db._execute_write(sql, params) -- wrong signature and private method)
  • Added try/finally db.close() in both sync functions to prevent WAL connection leak

424 tests pass.

…str, use set_session_title(), close() after each call

Three bugs found during review:
1. Class is SessionDB not HermesState -- would silently no-op on every install
2. SessionDB.__init__ takes Path not str -- would crash with AttributeError
3. _execute_write() takes a callable not SQL+params -- wrong signature.
   Replaced with public set_session_title() API.
4. Each call opened a persistent SQLite connection and never closed it.
   Added try/finally db.close() to prevent WAL leak under sustained load.
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