Skip to content

fix(cli): persist close transcript without history alias - #63274

Closed
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/cli-close-persist-alias
Closed

fix(cli): persist close transcript without history alias#63274
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/cli-close-persist-alias

Conversation

@necoweb3

Copy link
Copy Markdown
Contributor

Summary

_persist_active_session_before_close() could pass the same live transcript list as both messages and conversation_history when the CLI exits during an interrupted/in-flight turn. _flush_messages_to_session_db() treats identities present in conversation_history as already durable, so the close safety-net could write zero rows and lose the transcript on terminal close/SIGHUP/SIGTERM.

This is a residual CLI close-path data-loss case after #50004, and mirrors the dashboard/TUI disconnect alias class fixed in #62052.

Changes

  • Flush the CLI close safety-net through _persist_session(messages) without a conversation_history argument, so marker-based dedup is used.
  • Keep the existing fallback to self.conversation_history when _session_messages is not a real list.
  • Add a real SessionDB regression where cli.conversation_history aliases agent._session_messages; before the fix it stores zero rows, after the fix the interrupted transcript survives.
  • Update mock call-shape tests to enforce the no-history close-path contract.

Tests

python -m pytest -p no:cacheprovider --basetemp=.pytest_tmp_cli_close tests/cli/test_cli_shutdown_memory_messages.py -k "cli_close_persist" -q --timeout-method=thread
4 passed, 4 deselected

python -m pytest -p no:cacheprovider --basetemp=.pytest_tmp_cli_close tests/cli/test_cli_shutdown_memory_messages.py -q --timeout-method=thread
8 passed

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 12, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as duplicate of #64004, which salvages your original contribution with authorship preserved.

Your PR identified the core bug correctly: _persist_active_session_before_close() passes the same live list as both messages and conversation_history, causing _flush_messages_to_session_db() to treat every message as already durable and write zero rows. The fix (drop the conversation_history argument so marker-based dedup is used instead) is the same approach taken in #64004.

#64004 is a wider salvage that also covers resumed-prefix close, close-before-worker staging, multimodal notes, and concurrent direct/normal flush paths — all on top of your original fix. Your commit and authorship are preserved in that PR's commit history.

Credit: @necoweb3 for the original identification and fix in #63274.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants