feat(plugins): propagate session context to plugin hooks - #42416
feat(plugins): propagate session context to plugin hooks#42416Gerkinfeltser wants to merge 2 commits into
Conversation
360d8e7 to
04f2b0e
Compare
…sion_key to plugin hooks Resolved conflicts with upstream/main (_ts_scope_block feature, expanded _emit_terminal_post_tool_call params, conversation_loop additions). 8 files, +365/-11
51f0677 to
2bd87f5
Compare
|
Ran into the same gap building a session-scoped plugin command and arrived at essentially the same helper design before finding this PR — so +1 on the approach, especially routing everything through one signature-inspecting call site instead of an env bridge. Heads-up though: the branch head currently doesn't parse, which is probably why CI hasn't said anything useful. Two files have flush-left lines that look like a patch-tool dedent artifact:
I opened #44563 scoped to just the TUI-gateway command-handler half (the path with no env fallback at all), with gateway-level regression tests for both dispatch sites — happy to close it in favor of this once the hooks half is green, or to lift those tests onto your branch if that's useful. |
|
Thanks for catching that. You were right — the patch had indentation fallout in `agent/conversation_loop.py` and `agent/tool_executor.py`. I’ve fixed those issues and added regression tests that exercise both sequential and concurrent executor paths, verifying `_gateway_session_key` reaches `get_pre_tool_call_block_message`. The tests also caught one extra issue in the sequential path, where `gateway_session_key` still wasn’t passed through the real plugin-block call. That’s fixed as well. Targeted local checks are passing; I’ll push shortly for CI. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the session-context gap; the command-handler premise is still present on current main.
Problems
agent/conversation_loop.py:436adds a second per-turn prologue afterbuild_turn_context()has already done that work. Current main performs message initialization, user-turn accounting, andpre_llm_callinagent/turn_context.py:271-318,478-529; retaining the copied block would run those effects twice.- The hook changes target the former
get_pre_tool_call_block_message()route. Current main dispatches throughresolve_pre_tool_block()atagent/tool_executor.py:449,1107,agent/agent_runtime_helpers.py:2193, andmodel_tools.py:1185, so the gateway-key plumbing needs to be ported through that central helper instead.
Suggested changes
- Remove the copied conversation-loop prologue and reapply only the intended hook argument at the current
agent/turn_context.pyboundary. - Port the command helper to the four current raw handler calls (
cli.py:8980,gateway/run.py:10067, and both TUI paths), with integration coverage. The related TUI-focused PR #44563 is useful context.
Automated hermes-sweeper review.
| _ext_prefetch_cache = _ctx.ext_prefetch_cache | ||
|
|
||
| # Main conversation loop counters (pure locals consumed by the loop below). | ||
| # Initialize conversation (copy to avoid mutating the caller's list) |
There was a problem hiding this comment.
build_turn_context() has already completed this prologue immediately above. Repeating the old body rebuilds messages, increments turn counters, and fires pre_llm_call a second time; please remove this copied block and port the hook change to the current turn-context boundary.
|
Superseded by the current-main draft port: #65188 This replacement carries the session-context work through the current |
feat(plugins): propagate session_id to command handlers and hooks
Summary
Plugin command handlers and
pre_tool_callhooks now receive correct session context, enabling per-session stateful plugins.Problem
Two gaps in Hermes plugin session propagation:
Command handlers never received
session_id. All four dispatch sites (CLI, Gateway/Discord, TUI×2) calledplugin_handler(raw_args)with only the raw argument string.kwargs.get("session_id")was alwaysNone, so every session fell back to"_default"— making per-session state impossible.pre_tool_callhooks receivedsession_id="". Three call sites intool_executor.pyandagent_runtime_helpers.pypassed an empty string. This is partially addressed by upstream PR fix(plugins): propagate session_id to pre_tool_call hook from all 3 callsites #34622 (open since May 29) which covers the hook side, but that PR does not cover command dispatch.Gateway key ≠ agent session_id. Gateway command dispatch resolves to a Discord session key (e.g.
agent:main:discord:group:...) when no agent is running yet. Hooks later receive the agent's UUID (20260608_122644_7ab1b0c3). Plugins tracking per-session state see two different keys for the same conversation.Changes
Commit 1: Propagate session_id to command handlers and pre_tool_call hooks
hermes_cli/plugins.py— Newcall_plugin_command_handler()helper that usesinspect.signature()to passsession_id(and future context kwargs) to handlers that accept them, while legacy handlers that only takeraw_argskeep working. BroadTypeErrorfallback is intentionally avoided to avoid masking real plugin bugs.cli.py— Already passessession_idin v0.15.2 (no change needed).gateway/run.py— Command dispatch now resolvesagent.session_idwith fallback to_quick_key, passes it viacall_plugin_command_handler().tui_gateway/server.py— Both dispatch sites use_session_context_id()helper to resolve the live agent'ssession_id, falling back to session key.agent/tool_executor.py— Bothget_pre_tool_call_block_message()call sites now passsession_id=agent.session_id.agent/agent_runtime_helpers.py— Same for its single call site.Commit 2: Pass gateway_session_key to hooks for session bridging
agent/conversation_loop.py—pre_llm_callhook invocation now passesgateway_session_key=getattr(agent, "_gateway_session_key", "").hermes_cli/plugins.py—get_pre_tool_call_block_message()accepts and forwardsgateway_session_keytoinvoke_hook("pre_tool_call", ...).agent/tool_executor.py— Both callers passgateway_session_key.agent/agent_runtime_helpers.py— Same.This lets plugins bridge the Discord/gateway key (used at command dispatch time when no agent exists yet) to the agent UUID (used at hook invocation time).
Commit 3: Add focused session-context tests
tests/hermes_cli/test_plugins.py— Adds coverage forcall_plugin_command_handler()preserving legacy raw-args handlers, forwarding accepted context kwargs, and forwarding all context to**kwargshandlers.tests/hermes_cli/test_plugins.py— Adds coverage thatget_pre_tool_call_block_message()forwards bothsession_idandgateway_session_keyintoinvoke_hook().tui_gateway/server.py— Initializescall_plugin_command_handler = Nonealongside the other optional plugin dispatch imports for cleaner fallback handling.Relationship to upstream PR #34622
Upstream PR #34622 (
fix(plugins): propagate session_id to pre_tool_call hook from all 3 callsites) covers the hook-sidesession_idpropagation. This PR:gateway_session_keybridging for gateway sessionsinspect.signature()instead of broadTypeErrorfallbackThe two PRs are complementary. If #34622 merges first, this PR's hook-side changes can be rebased out, leaving only the dispatch and bridging work.
Files changed
hermes_cli/plugins.pycall_plugin_command_handler()helper +gateway_session_keyinget_pre_tool_call_block_message()cli.pygateway/run.pytui_gateway/server.py_session_context_id()helper, both dispatch sitesagent/conversation_loop.pygateway_session_keyinpre_llm_callagent/tool_executor.pysession_id+gateway_session_keyin both call sitesagent/agent_runtime_helpers.pysession_id+gateway_session_keytests/hermes_cli/test_plugins.py8 files, +155/-6
Testing
py_compilepasses on all changed runtime files and focused tests74 passed, 2 pre-existing failures(same 2TestPluginDiscoverytests fail on basev0.15.2— unrelated to this PR)8 passedcovering legacy handlers, context kwargs,**kwargshandlers, and hook context forwardingBreaking changes
None. Legacy plugin handlers that only accept
raw_argscontinue to work — thecall_plugin_command_handler()helper inspects the signature and calls accordingly.Update after review feedback
After review feedback, this PR also fixes indentation fallout from the initial patch application in
agent/conversation_loop.pyandagent/tool_executor.py.I added regression coverage for the executor plugin-block call sites to verify
_gateway_session_keyis passed through in both sequential and concurrent tool execution paths. The new tests also caught and fixed a missinggateway_session_keyargument in the sequential path.Additional targeted checks run locally:
python -m pytest tests/run_agent/test_tool_call_guardrail_runtime.py -q --no-headerpython -m pytest tests/hermes_cli/test_plugins.py -q --no-header -k "block or gateway or session"python -m py_compile agent/tool_executor.py agent/conversation_loop.py tests/run_agent/test_tool_call_guardrail_runtime.pygit diff --check -- agent/tool_executor.py agent/conversation_loop.py tests/run_agent/test_tool_call_guardrail_runtime.py