fix(run_agent): notify context engine on commit_memory_session (#22394) - #22431
briandevans wants to merge 1 commit into
Conversation
…esearch#22394) `commit_memory_session` is the session-rotation entry point used by CLI `/new`, gateway session expiry, and in-process compression. It called `_memory_manager.on_session_end` but skipped `context_compressor.on_session_end`, so plugin context engines like hermes-lcm never received the end-of-session flush for the old session_id. Concretely, on `/new` and gateway session expiry the LCM session stays "active" in the lifecycle store and any messages that arrived after the last `compress()` call are not persisted to lcm.db. The sibling `shutdown_memory_provider` already calls both — this PR restores parity by mirroring that pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes session-rotation lifecycle notifications so plugin context engines (e.g., hermes-lcm) receive on_session_end(...) during commit_memory_session(), aligning behavior with shutdown_memory_provider() and preventing missed final-turn persistence/finalization during /new, gateway expiry, and compression-driven rotation.
Changes:
- Added
context_compressor.on_session_end(session_id, messages)fan-out inAIAgent.commit_memory_session(). - Added a new regression test suite covering expected
commit_memory_session()fan-out behavior and edge cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
run_agent.py |
Adds missing context-engine on_session_end notification during session commit/rotation. |
tests/run_agent/test_commit_memory_session.py |
Introduces regression tests validating commit_memory_session() behavior for manager/compressor notifications and safety cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self._memory_manager: | ||
| return | ||
| try: | ||
| self._memory_manager.on_session_end(messages or []) | ||
| except Exception: | ||
| pass |
| def _agent(memory_manager=None, context_compressor=None, session_id="sess-1"): | ||
| a = AIAgent.__new__(AIAgent) | ||
| a._memory_manager = memory_manager | ||
| a.context_compressor = context_compressor | ||
| a.session_id = session_id | ||
| return a | ||
|
|
|
Closing — superseded by @teknium1's #22764, which landed the same fix on main as commit e90aa7f. The merged version is strictly better: it calls |
Summary
commit_memory_sessionis the session-rotation entry point used by CLI/new, gateway session expiry (Telegram/Discord/Slack), and in-process compression. It calledself._memory_manager.on_session_end(...)but neverself.context_compressor.on_session_end(...). As a result, plugin context engines like hermes-lcm silently lost the final turns and never finalized the rotating session in their lifecycle store.The sibling
shutdown_memory_provideralready calls both. This PR restores parity by mirroring that pattern.The bug
run_agent.pycommit_memory_session(before):Compare with
shutdown_memory_providerin the same file, which correctly notifies both:Concrete impact (per the issue reporter, hermes-lcm v0.9.2):
/new: messages that arrived after the lastcompress()/ preflight call are not persisted tolcm.db; the LCM session never getslifecycle.finalize_session()and stays "active"._finalize_session()callscommit_memory_session().on_session_startfires immediately after, but the old session_id still ends without anon_session_endnotification, breaking the engine's session lifecycle pairing.The fix
Add the missing
context_compressor.on_session_end(self.session_id or "", messages or [])block after the memory-manager call, guarded the same wayshutdown_memory_providerguards it (hasattr+ truthiness +try/except Exception).Test plan
tests/run_agent/test_commit_memory_session.py(6 tests):on_session_endwith the same messages(session_id, messages)tuple shapemessages=Noneis normalized to[]for bothsession_id=Noneis normalized to""for the compressor (mirrorsshutdown_memory_provider)on_session_enddoes not break session rotation; manager call still happenscontext_compressorattribute andcontext_compressor=Noneare both safe no-opstests/agent/test_memory_provider.py(memory manager fan-out tests, including existingTestCommitMemorySessionRouting)tests/run_agent/test_compress_focus_plugin_fallback.py,test_compression_boundary.py,test_compression_persistence.pytests/cli/test_cli_new_session.py,test_cli_shutdown_memory_messages.pytests/gateway/test_shutdown_memory_provider_messages.py,test_compress_command.pyassert comp.session_end_calls == [(\"sess-1\", msgs)]—[] == [...]. With the fix restored, all 6 pass.Test command used:
Related
Fixes #22394.