fix(context-engine): clone plugin engines per agent - #42683
Conversation
|
Positive verification — context engine per-agent cloning Reviewed the diff across agent/agent_init.py, agent/context_engine.py, run_agent.py, and tests. Problem: Plugin context engines are registered process-wide, but gateway runtimes may keep multiple cached AIAgent instances alive simultaneously (different platforms, chats, cron jobs). Engines that maintain mutable session state (cursors, bindings) on self would share that state across agents — causing cross-session contamination. Fix: clone_for_agent() method on ContextEngine (default returns self for backward compatibility). init_agent() calls it before binding session state; if the clone is a different object, the agent sets _owns_context_engine = True and takes ownership of its lifecycle. AIAgent.close() calls the shutdown method only on owned engines, leaving shared registered singletons alive for other agents. Test coverage:
The error handling in init_agent() gracefully falls back to the registered instance if clone_for_agent() raises or returns None. LGTM. ✅ |
|
Downstream validation from the hermes-lcm side: this is the host-side piece for stephenschoettler/hermes-lcm#243 / stephenschoettler/hermes-lcm#247. I checked current head Local validation:
Current |
Local backport of upstream PR NousResearch#42683 (commit 2b2a590). Adapted for our codebase. Adds ContextEngine.clone_for_agent() and shutdown(), per-agent engine isolation in AIAgent init and close, and 9 new tests. Related: stephenschoettler/hermes-lcm#243, stephenschoettler/hermes-lcm#247
|
Tested on my environment running multiple concurrent agents (WebUI session + background cron jobs + subagents) against the same hermes-agent process. Before the fix, background tasks calling After applying the clone fix, each agent gets its own engine instance. Compression now correctly attributes DAG nodes to the originating session. Verified by running lcm_expand on post-compression nodes - all resolve cleanly with the correct session ID. |
|
Superseded by #62374, rebuilt on current main. This replacement preserves and credits the per-agent clone contract direction from this PR while incorporating the deepcopy baseline and current agent lifecycle. |
1 similar comment
|
Superseded by #62374, rebuilt on current main. This replacement preserves and credits the per-agent clone contract direction from this PR while incorporating the deepcopy baseline and current agent lifecycle. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the process-wide context-engine lifecycle issue. The current branch cannot be applied safely to current main as written.
Problems
agent/context_engine.py:120makes the defaultclone_for_agent()returnself. The initialization change then accepts that alias, while current main explicitly usescopy.deepcopy(_candidate)atagent/agent_init.py:1785-1805to prevent a child agent's model/session binding from mutating the registered template (#42449).- The branch tears down an owned clone only in
AIAgent.close(). The replacement implementation in #62374 also handles the softrelease_clients()lifecycle, which is needed when cached agents are evicted without a hard close.
Suggested changes
- Please use the rebuilt #62374 approach: preserve deepcopy as the default clone contract, reject aliases of the registered template, and centralize idempotent teardown for both soft release and close.
This is an automated hermes-sweeper review.
| platforms, chats, cron jobs, etc.). Engines that keep mutable session | ||
| binding or cursor state on ``self`` should override this method and | ||
| return a fresh engine instance that shares durable storage/configuration | ||
| as needed. Stateless engines can use the default, which preserves the |
There was a problem hiding this comment.
Returning the registered template here regresses current main's deepcopy isolation: init_agent subsequently calls update_model() and on_session_start() on this object. Default behavior must create an isolated copy (and init must reject template aliases), as in the rebuilt #62374 approach.
|
Closing as superseded by #62374, which rebuilds this fix on current main and preserves credit for the original direction. Thanks for carrying it forward. |
Summary
ContextEngine.clone_for_agent()hook for plugin context engines.AIAgentinitialization before model/session binding.AIAgent.close()only shuts down cloned per-agent engines, not process-wide registered singletons.Why
Plugin context engines are registered process-wide, but gateway runtimes can keep multiple cached
AIAgentinstances alive at once (different chats, platforms, cron jobs, etc.). Engines with mutable session binding or ingest cursor state need a per-agent runtime instance; otherwise one cached agent can rebind another agent's context engine state.The default hook returns
self, preserving existing shared-instance behavior for stateless engines and old plugins.Pairs with stephenschoettler/hermes-lcm#247, which implements the hook for LCM.
Test plan
python -m pytest tests/agent/test_context_engine_host_contract.py tests/agent/test_context_engine.py tests/run_agent/test_compression_boundary_hook.py -q -o 'addopts='