From 188f0d4251d69fbafdab97396fc4bcfca84c254e Mon Sep 17 00:00:00 2001 From: thomasscottbeck-sudo <244846147+thomasscottbeck-sudo@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:55:25 -0600 Subject: [PATCH] fix(plugins): fire transform_tool_result for agent-runtime tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-runtime tools (todo_list, session_search, memory, clarify, delegate_task, the preview/terminal readers, context-engine and memory-provider tools) are dispatched inline and never reach handle_function_call, which is the only place transform_tool_result ran. A registered transform silently did nothing for them, although the hook is documented as applying to every tool. Apply the same helper on both runtime executor paths, after the terminal post_tool_call so the observer still sees the untransformed result: the concurrent path in invoke_tool's inline branch, and the sequential path in _publish_sequential_result. The sequential registry dispatch marks itself transform_applied so handle_function_call stays the single invocation for registry tools and nothing double-fires. The sequential result classification (failure detection and the logged result length) moves below the transform so it reads the result the model actually sees, matching the registry and concurrent paths. Co-Authored-By: Claude Fable 5.1 (cherry picked from commit bc31efff0a6e2fd8177edd73cfe1a7112df9f78d) Fixes #72836 Salvages #115397 (thomasscottbeck-sudo); supersedes #72860 (webtecnica, earliest — sequential path only). (cherry picked from commit d6b5e969b3e08a557a62ed85c8b828537857b3d8) --- agent/agent_runtime_helpers.py | 16 +++++--- agent/inline_tool_executors.py | 25 ++++++++++++ agent/tool_executor.py | 23 ++++++++--- tests/agent/test_run_agent.py | 70 ++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 11 deletions(-) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 74986d6a9d..5fb2dee9d6 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -2370,7 +2370,8 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i no display logic. Used by the concurrent path; the sequential path keeps its own inline invocation for display.""" from agent.inline_tool_executors import ( - InlineToolContext, emit_terminal_post_tool_call, resolve_invoke_tool_executor, tool_hook_ids + InlineToolContext, apply_transform_tool_result, emit_terminal_post_tool_call, + resolve_invoke_tool_executor, tool_hook_ids ) if not isinstance(function_args, dict): function_args = {} @@ -2407,14 +2408,17 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i def _execute(next_args: dict) -> Any: result = inline_executor(agent, next_args, inline_ctx) + call_args = next_args if isinstance(next_args, dict) else function_args + duration_ms = int((time.monotonic() - tool_start_time) * 1000) emit_terminal_post_tool_call( - agent, function_name=function_name, - function_args=next_args if isinstance(next_args, dict) else function_args, + agent, function_name=function_name, function_args=call_args, result=result, effective_task_id=effective_task_id, tool_call_id=tool_call_id, - duration_ms=int((time.monotonic() - tool_start_time) * 1000), - middleware_trace=_tool_middleware_trace, + duration_ms=duration_ms, middleware_trace=_tool_middleware_trace, + ) + return apply_transform_tool_result( + agent, function_name=function_name, function_args=call_args, result=result, + effective_task_id=effective_task_id, tool_call_id=tool_call_id, duration_ms=duration_ms, ) - return result else: def _execute(next_args: dict) -> Any: dispatch_kwargs = dict( diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py index 10d22c7d9a..76a675e8f5 100644 --- a/agent/inline_tool_executors.py +++ b/agent/inline_tool_executors.py @@ -58,6 +58,31 @@ def emit_terminal_post_tool_call( pass +def apply_transform_tool_result( + agent, + *, + function_name: str, + function_args: dict, + result: Any, + effective_task_id: str, + tool_call_id: Optional[str], + duration_ms: int = 0, +) -> Any: + """Apply ``transform_tool_result`` to an inline-dispatched tool's result. + + Registry tools get this inside ``handle_function_call``; inline executors never + reach it, so the agent paths call the same helper (after the terminal + ``post_tool_call``) to keep the hook's "every tool" contract. Fail-open.""" + try: + from model_tools import _CallIds, _apply_transform_tool_result_hook + return _apply_transform_tool_result_hook( + function_name, function_args, result, duration_ms, + _CallIds(**tool_hook_ids(agent, effective_task_id, tool_call_id)), + ) + except Exception: + return result + + @dataclass class InlineToolContext: """Per-call state an inline executor may need beyond its arguments.""" diff --git a/agent/tool_executor.py b/agent/tool_executor.py index f6f84dcf11..26779b0107 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -33,6 +33,7 @@ from agent.message_sanitization import coalesce_tool_call_id from agent.inline_tool_executors import ( INLINE_TOOL_EXECUTORS, InlineToolContext, + apply_transform_tool_result, emit_terminal_post_tool_call, tool_hook_ids, ) @@ -1592,6 +1593,7 @@ class _SequentialDispatch: is_delegate: bool = False finish_spinner: bool = True finish_in_finally: bool = True # inline tools print their completion line only on success + transform_applied: bool = False # True when execute already fired transform_tool_result def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _SequentialDispatch: @@ -1656,6 +1658,7 @@ def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _S error_log="handle_function_call raised for %s: %s", handles_keyboard_interrupt=True, finish_spinner=bool(agent.quiet_mode), + transform_applied=True, # handle_function_call fires transform_tool_result itself ) @@ -1726,19 +1729,28 @@ def _run_sequential_call( return managed, tool_duration -def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed: _ManagedToolResult, *, tool_duration: float, index: int, budget: BudgetConfig) -> bool: +def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed: _ManagedToolResult, *, tool_duration: float, index: int, budget: BudgetConfig, transform_applied: bool) -> bool: """Terminal hook → observe → commit → completion callbacks/print for one sequential result; False when the incremental flush failed (the caller must stop the batch).""" ref.args, ref.trace, function_result = managed.args, managed.middleware_trace, managed.result _execution_timed_out = isinstance(function_result, (_ToolTimeoutResult, _ToolCancelledResult)) - # Multimodal dict results (_multimodal=True) are not sliceable as strings. - _result_len = len(function_result) if isinstance(function_result, str) else len(str(function_result)) - _is_error_result, _ = _detect_tool_failure(ref.name, function_result) # Inline-dispatched runtime tools never reach handle_function_call, so the # executor owns the one terminal post_tool_call per tool_call_id (the inner # observer is suppressed); also stops an abandoned timeout worker reporting late. + # transform_tool_result follows the observer, unless the dispatch already fired it. if not managed.blocked and not _execution_timed_out: ref.emit_post(agent, function_result, duration_ms=int(tool_duration * 1000)) + if not transform_applied: + function_result = apply_transform_tool_result( + agent, function_name=ref.name, function_args=ref.args, result=function_result, + effective_task_id=ref.task_id, tool_call_id=ref.call_id, + duration_ms=int(tool_duration * 1000), + ) + # Classify the result the model will actually see, i.e. after any transform; the + # registry and concurrent paths both classify post-transform. + # Multimodal dict results (_multimodal=True) are not sliceable as strings. + _result_len = len(function_result) if isinstance(function_result, str) else len(str(function_result)) + _is_error_result, _ = _detect_tool_failure(ref.name, function_result) committed = _commit_tool_result( agent, messages, ref, function_result, budget=budget, tool_duration=tool_duration, is_error=_is_error_result, blocked=managed.blocked, @@ -1809,7 +1821,8 @@ def _execute_tool_calls_sequential(agent, assistant_message, messages: list, eff display_index=i, tool_start_time=tool_start_time, ) - if not _publish_sequential_result(agent, messages, ref, managed, tool_duration=tool_duration, index=i, budget=_tool_budget): + if not _publish_sequential_result(agent, messages, ref, managed, tool_duration=tool_duration, index=i, + budget=_tool_budget, transform_applied=dispatch.transform_applied): return if agent._interrupt_requested and i < len(tool_calls): diff --git a/tests/agent/test_run_agent.py b/tests/agent/test_run_agent.py index f3ae1534fc..c5f54eeeeb 100644 --- a/tests/agent/test_run_agent.py +++ b/tests/agent/test_run_agent.py @@ -2612,6 +2612,76 @@ class TestAgentRuntimePostHookOwnershipSync: } +class TestRuntimeToolTransformToolResult: + """A registered ``transform_tool_result`` replaces what the model sees for an + agent-runtime tool, on both the sequential and the concurrent executor path.""" + + @staticmethod + def _install_rewriting_transform(agent, monkeypatch): + monkeypatch.setattr( + "hermes_cli.plugins._dispatch_pre_tool_call_hooks", + lambda *args, **kwargs: (None, None), + ) + monkeypatch.setattr("hermes_cli.lifecycle.has_hook", lambda name: True) + monkeypatch.setattr( + "hermes_cli.lifecycle.invoke_hook", + lambda hook_name, **kwargs: ( + [f'REWRITTEN[{kwargs["tool_name"]}]{kwargs["result"]}'] + if hook_name == "transform_tool_result" + else [] + ), + ) + monkeypatch.setattr("tools.todo_tool.todo_tool", lambda **kwargs: '{"ok":true}') + agent._memory_manager = None + + def test_concurrent_path_applies_transform(self, agent, monkeypatch): + self._install_rewriting_transform(agent, monkeypatch) + messages = [] + + agent._execute_tool_calls_concurrent( + _mock_assistant_msg( + content="", + tool_calls=[ + _mock_tool_call( + name="todo_list", arguments=json.dumps({"todos": []}), call_id=call_id + ) + for call_id in ("todo-c1", "todo-c2") + ], + ), + messages, + "task-concurrent", + ) + + tool_results = [m for m in messages if m.get("role") == "tool"] + assert [m["tool_call_id"] for m in tool_results] == ["todo-c1", "todo-c2"] + # Exactly once per call: a second invocation would nest the prefix. + assert [str(m["content"]) for m in tool_results] == ['REWRITTEN[todo_list]{"ok":true}'] * 2 + + def test_sequential_path_applies_transform(self, agent, monkeypatch): + self._install_rewriting_transform(agent, monkeypatch) + messages = [] + + agent._execute_tool_calls_sequential( + _mock_assistant_msg( + content="", + tool_calls=[ + _mock_tool_call( + name="todo_list", + arguments=json.dumps({"todos": []}), + call_id="todo-sequential", + ) + ], + ), + messages, + "task-sequential", + ) + + tool_results = [m for m in messages if m.get("role") == "tool"] + assert tool_results, "sequential path appended no tool result" + # Exactly once: a second invocation would nest the prefix. + assert str(tool_results[-1]["content"]) == 'REWRITTEN[todo_list]{"ok":true}' + + class TestPathsOverlap: """Unit tests for the _paths_overlap helper."""