fix(agent): notify context_compressor on commit_memory_session - #22433
Closed
wesleysimplicio wants to merge 1 commit into
Closed
fix(agent): notify context_compressor on commit_memory_session#22433wesleysimplicio wants to merge 1 commit into
wesleysimplicio wants to merge 1 commit into
Conversation
commit_memory_session() is called when the session_id rotates without tearing providers down (CLI /new, gateway session expiry, in-flight context compression). It correctly notified _memory_manager but never called context_compressor.on_session_end(), so plugin context engines like hermes-lcm missed the final flush: - messages that arrived after the last compress() call were never persisted to lcm.db - the LCM lifecycle store kept the session marked active forever - shutdown_memory_provider() already does both calls; commit_memory_session was the asymmetric outlier Mirror the shutdown_memory_provider() pattern so the compressor receives on_session_end(self.session_id or "", messages or []), guarded by the same hasattr / try-except as the shutdown path. Each backend is now checked independently — a missing _memory_manager no longer skips the compressor flush. Closes NousResearch#22394
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an asymmetry in AIAgent.commit_memory_session() by ensuring it notifies the configured context engine (context_compressor) on session rotation, matching the behavior already implemented in shutdown_memory_provider(). This prevents plugin-based context engines (e.g., hermes-lcm) from missing the final session-end flush when sessions rotate without provider teardown (e.g., /new, gateway expiry, compression-driven rollover).
Changes:
- Update
AIAgent.commit_memory_session()to independently flush_memory_managerandcontext_compressor(each guarded and exception-swallowed). - Add regression tests covering fan-out behavior, failure isolation, and edge cases (missing backends,
session_id=None).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
run_agent.py |
Ensures commit_memory_session() also calls context_compressor.on_session_end(...) under the same guards used elsewhere, and does not skip compressor flush when _memory_manager is absent. |
tests/agent/test_commit_memory_session_compressor.py |
Adds targeted regression tests verifying both backends are notified and failures in one backend do not block the other. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Contributor
Author
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
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.
Summary
AIAgent.commit_memory_session()is invoked when the session_id rotateswithout tearing providers down — CLI
/new, gateway session expiry, andin-flight context compression. It already calls
_memory_manager.on_session_end(), but never calledcontext_compressor.on_session_end(), so plugin context engines likehermes-lcm miss the final session-end flush:
compress()call are neverpersisted to
lcm.db.shutdown_memory_provider()already does both calls;commit_memory_sessionwas the asymmetric outlier.
Fix
Mirror the
shutdown_memory_provider()pattern: the compressor flush isnow invoked under the same
hasattr(...) and self.context_compressorguard, and each backend is checked independently so a missing
_memory_managerno longer skips the compressor.Tests
New regression tests in
tests/agent/test_commit_memory_session_compressor.py:test_notifies_both_memory_manager_and_context_compressortest_compressor_flushed_even_when_memory_manager_absenttest_memory_manager_failure_does_not_block_compressortest_compressor_failure_is_swallowedtest_no_op_when_neither_backend_presenttest_uses_empty_string_session_id_when_noneStash-verified: 4/6 fail without the fix (the two no-op cases pass either
way), all 6 pass with it.
Test plan
pytest tests/agent/test_memory_provider.py tests/run_agent/ tests/cli/test_cli_new_session.py tests/agent/test_commit_memory_session_compressor.py→ 1332 passed, 9 skippedpytest→ 21240 passed; the 115 failures match the pre-existing flaky baseline onorigin/main(test_discord_, test_gateway_service, test_gateway_wsl, test_file_, test_terminal_tool_requirements, test_tts_media_routing, test_setup_openclaw_migration, test_tencent_tokenhub_provider, test_list_picker_providers, test_model_switch_custom_providers, test_tui_gateway_server, test_web_server::test_pub_broadcasts) — none touched by this PRCloses #22394