fix(tui): resolve in-memory session UUID in session.resume for crash recovery #39013 - #39167
fix(tui): resolve in-memory session UUID in session.resume for crash recovery #39013#39167kyssta-exe wants to merge 1 commit into
Conversation
…y crash recovery After a TUI gateway child crashes and respawns, the recovery path calls session.resume with the in-memory session UUID (the TUI's sid). However, the DB stores sessions under a different primary key (stored_session_id), so the lookup fails with 'session not found'. Add an in_memory_id column to the sessions table that stores the TUI's in-memory UUID. session.resume now falls back to this column when the primary and title lookups fail, allowing crash recovery to find and resume the session correctly. Fixes NousResearch#39013
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the runtime-versus-durable session identity mismatch. The stale-ID premise is still present on current main: ui-tui/src/app/useSessionLifecycle.ts:223-227 stores the runtime session_id, ui-tui/src/app/useMainApp.ts:801 carries it into recovery, and tui_gateway/server.py:5582-5601 cannot resolve it from the DB.
Problems
tui_gateway/server.py:5290-5291already returns the durablestored_session_id, butSessionCreateResponseexposes onlysession_id(ui-tui/src/gatewayTypes.ts:225-228). Persisting another DB alias for the runtime ID is unnecessary extra schema surface when recovery can retain and submit the existing durable ID.- The added tests cover only the new
SessionDBhelpers. They do not exercise the create → first-prompt persistence → fresh gateway → recovery-resume path. Existing recovery coverage mocksresumeByIdatui-tui/src/__tests__/createGatewayEventHandler.test.ts:758-766.
Suggested changes
- Preserve both identities in the TUI and use
stored_session_idexclusively for crash recovery; retainsidfor live RPC dispatch. - Add a protocol/UI regression test covering the complete recovery boundary.
Automated hermes-sweeper review.
| @@ -3959,7 +3977,7 @@ def _(rid, params: dict) -> dict: | |||
| _start_inflight_turn(session, text) | |||
There was a problem hiding this comment.
Please avoid persisting the runtime sid as a second DB identity here. session.create already returns the durable stored_session_id; expose and retain that value in the TUI recovery state, then pass it to session.resume. This keeps runtime and durable identities explicit without a schema migration.
|
Stale — oldest open PR, no merge activity for weeks. |
Summary
After a TUI gateway child crashes and respawns, the recovery path calls
session.resumewith the in-memory session UUID (the TUI'ssid). However, the DB stores sessions under a different primary key (stored_session_id), so the lookup fails with "session not found".Root Cause
session.createreturns two IDs:session_id: an in-memory UUID used as the key in the Python_sessionsdictstored_session_id: the DB primary key stored in SQLite'ssessions.idcolumnThe TUI stores
session_idassidin its UI state. When the gateway child exits (e.g., due to rapid successive messages causing stdin EOF), the crash recovery path stores thissidinrecoverSidRefand sends it tosession.resumeon the respawned gateway. Since the DB row is keyed bystored_session_id(not the in-memory UUID),db.get_session(target)returns None, and the user sees "session not found".Fix
Three changes across 3 files (+85 lines):
hermes_state.py: Added
in_memory_id TEXTcolumn to the sessions table schema (auto-migrated via declarative reconciliation). Addedget_session_by_in_memory_id()andset_session_in_memory_id()methods toSessionDB.tui_gateway/server.py: Modified
_ensure_session_db_row()to store the in-memory UUID in thein_memory_idcolumn when first persisting the session. Modifiedsession.resumeto fall back toget_session_by_in_memory_id()when the primary ID and title lookups fail.tests/test_hermes_state.py: Added 4 tests for the new in_memory_id lookup functionality.
Testing
test_goal_command.py::test_goal_bare_shows_status_when_none_setexcluded (fails without changes too)Issue
Fixes #39013