fix: enable runtime final response guards - #56429
Conversation
Pass channel and user context into shell hooks and allow transform_llm_output shell hooks to return a final response replacement. Add parser regression coverage for response_text passthrough.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: LGTM
Enables runtime final response guards and adds transform_llm_output support to shell hooks. Also enriches hook context with sender/chat metadata.
Looks Good
- Transform hook correctly returns string (not dict) for response replacement
- Backward-compatible: Optional[Any] return type broadens but doesn't break
- Clean tests for both passthrough and empty-text cases
- New context fields (sender_name, chat_id, etc.) are additive
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused shell-hook fix. The underlying gap is still present on current main: agent/shell_hooks.py:566-620 does not translate transform_llm_output response_text JSON into the string contract consumed by the final-response hook.
Problems
- The final-response hunk no longer targets the live dispatch site. Refactor
55b83c3d9moved it fromagent/conversation_loop.pytoagent/turn_finalizer.py:380-391; the PR is currently reported dirty by GitHub. tests/agent/test_shell_hooks.pyadds parser-only coverage. It does not verify a configured shell hook reaches the current finalizer and replacesfinal_response.
Suggested changes
- Salvage the transform-context kwargs into
agent/turn_finalizer.py:380-391, using itsoriginal_user_messageparameter. - Add a runtime shell-hook-to-final-response regression test, including the replacement and emitted context.
Automated hermes-sweeper review.
| @@ -4411,6 +4423,14 @@ def _stop_spinner(): | |||
| session_id=agent.session_id or "", | |||
There was a problem hiding this comment.
Current main moved this live transform_llm_output dispatch to agent/turn_finalizer.py:380-391 in 55b83c3d9; port these kwargs there during salvage, otherwise this hunk does not affect the runtime final-response path.
Summary
Test Plan