fix(cli): initialize session_id before OpenViking orphan commit - #31493
Closed
Ramadas108 wants to merge 2 commits into
Closed
fix(cli): initialize session_id before OpenViking orphan commit#31493Ramadas108 wants to merge 2 commits into
Ramadas108 wants to merge 2 commits into
Conversation
added 2 commits
May 24, 2026 12:10
…cess takes over
Adds _commit_orphaned_openviking_sessions() called from HermesCLI.__init__
that discovers sessions left behind by killed or crashed Hermes processes
(ended_at=NULL or end_reason='new_session' without commit) and sends
POST /api/v1/sessions/{sid}/commit to OpenViking to finalize memory
extraction.
Fixes two scenarios:
- Process A killed (SIGKILL/terminal close) while session active —
session never receives final commit
- Process B starts via /new but crashes before OpenViking HTTP response
returns — session committed in state.db but not in OpenViking
Testing: 4 orphaned sessions recovered from state.db, 346 abandoned
CLI sessions closed with orphaned_restart end_reason.
The _commit_orphaned_openviking_sessions() call in HermesCLI.__init__ referenced self.session_id before it was assigned, causing AttributeError on startup. Fix: initialize self.session_id= as a safe default at the top of the constructor, move the orphan commit to after the formal session_id assignment, wrap it in try/except so an OpenViking failure never blocks startup, and use getattr(self, session_id, ) as an additional defensive fallback.
alt-glitch
added
type/bug
Collaborator
Contributor
|
Thanks for identifying the startup ordering problem. The specific Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-session-state
Collaborator
|
Merged via #69191. Your PR's insight — initializing session_id before the orphan commit runs — was addressed by performing recovery inside the provider's |
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
4 participants
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.
This fixes a startup crash introduced when OpenViking orphan-session recovery was called before
HermesCLI.session_idwas assigned.On a fresh CLI startup,
_commit_orphaned_openviking_sessions()can referenceself.session_idbefore the constructor has created it, causing:The fix moves the orphan commit call after
session_idassignment, initializessession_id=""as a safe default at the top of__init__, and wraps the call in try/except so that an OpenViking failure never blocks CLI startup. A defensivegetattr(self, "session_id", "")fallback is also used as an additional guard against future reordering.Changes:
cli.py— safe default initialization, deferred call ordering, non-fatal error handlingtests/cli/test_cli_startup.py— regression smoke tests provingHermesCLI()can initialize withoutAttributeErrorand survives a simulated orphan-commit crashThis keeps orphan-session recovery behavior unchanged, but prevents it from blocking CLI startup.