diff --git a/coding-agent/src/agentkit/tools/toolset.py b/coding-agent/src/agentkit/tools/toolset.py index c64e23469..78fa6ad39 100644 --- a/coding-agent/src/agentkit/tools/toolset.py +++ b/coding-agent/src/agentkit/tools/toolset.py @@ -9,6 +9,7 @@ from typing import Any, Protocol from agentkit.directive.types import Directive +from agentkit.observability import ObservationSink, record_span from agentkit.runtime.hook_runtime import HookRuntime from agentkit.tools.schema import ToolSchema @@ -27,6 +28,35 @@ def __repr__(self) -> str: _PROXY_AFFORDANCE_NAMES = frozenset({SEARCH_TOOLS_NAME, CALL_TOOL_NAME}) +def _observation_sink(ctx: Any) -> ObservationSink | None: + config = getattr(ctx, "config", None) + if not isinstance(config, dict): + return None + sink = config.get("observation_sink") + if sink is None: + return None + if not isinstance(sink, ObservationSink): + raise TypeError("observation_sink must implement ObservationSink") + return sink + + +def _tool_call_attributes(call: "ToolCallRequest") -> dict[str, Any]: + return { + "tool.name": call.name, + "tool.call_id": call.tool_call_id, + } + + +def _set_tool_result_attributes( + span: Any, + result: "ToolExecutionResult", +) -> None: + span.set_attribute("tool.missing", result.missing) + span.set_attribute("tool.error", result.is_error) + if result.error is not None: + span.set_attribute("tool.error_type", type(result.error).__name__) + + class ToolProvider(Protocol): """Provider contract for components that expose executable tools.""" @@ -292,10 +322,14 @@ def _validate_schema_value( if isinstance(value, int | float) and not isinstance(value, bool): minimum = schema.get("minimum") if isinstance(minimum, int | float) and value < minimum: - return f"invalid argument {self._format_path(path)}: must be >= {minimum}" + return ( + f"invalid argument {self._format_path(path)}: must be >= {minimum}" + ) maximum = schema.get("maximum") if isinstance(maximum, int | float) and value > maximum: - return f"invalid argument {self._format_path(path)}: must be <= {maximum}" + return ( + f"invalid argument {self._format_path(path)}: must be <= {maximum}" + ) if isinstance(value, str): min_length = schema.get("minLength") @@ -334,13 +368,17 @@ def _validate_object_schema( if isinstance(required, list): for name in required: if isinstance(name, str) and name not in value: - return f"missing required argument: {self._format_path((*path, name))}" + return ( + f"missing required argument: {self._format_path((*path, name))}" + ) if schema.get("additionalProperties") is False: allowed = set(properties) unexpected = sorted(set(value) - allowed) if unexpected: - return f"unexpected argument: {self._format_path((*path, unexpected[0]))}" + return ( + f"unexpected argument: {self._format_path((*path, unexpected[0]))}" + ) for name, child_schema in properties.items(): if name not in value or not isinstance(name, str): @@ -579,16 +617,24 @@ async def _execute_one( ctx: Any, options: ToolExecutionOptions, ) -> ToolExecutionResult: - if call.name == SEARCH_TOOLS_NAME: - return self._execute_search_tools(call) - if call.name == CALL_TOOL_NAME: - return await self._execute_call_tool(call, ctx=ctx, options=options) - return await self._execute_one_from_hook( - "execute_tool", - call, - ctx=ctx, - options=options, - ) + with record_span( + "tool.call", + sink=_observation_sink(ctx), + attributes=_tool_call_attributes(call), + ) as span: + if call.name == SEARCH_TOOLS_NAME: + result = self._execute_search_tools(call) + elif call.name == CALL_TOOL_NAME: + result = await self._execute_call_tool(call, ctx=ctx, options=options) + else: + result = await self._execute_one_from_hook( + "execute_tool", + call, + ctx=ctx, + options=options, + ) + _set_tool_result_attributes(span, result) + return result async def _execute_one_from_hook( self, @@ -648,11 +694,7 @@ def _execute_search_tools(self, call: ToolCallRequest) -> ToolExecutionResult: error=ValueError("search_tools.limit must be an integer from 1 to 50"), ) - terms = [ - term - for term in query_value.strip().casefold().split() - if term - ] + terms = [term for term in query_value.strip().casefold().split() if term] matches: list[ToolSchema] = [] for schema in self._proxy_schemas(): haystack = f"{schema.name} {schema.description}".casefold() @@ -751,8 +793,7 @@ async def _execute_call_tool( return ToolExecutionResult( tool_call_id=call.tool_call_id, name=call.name, - error=target_result.error - or RuntimeError(target_result.error_message), + error=target_result.error or RuntimeError(target_result.error_message), ) return ToolExecutionResult( diff --git a/coding-agent/tests/agentkit/tools/test_toolset.py b/coding-agent/tests/agentkit/tools/test_toolset.py index 955dbe1c6..e3f97900b 100644 --- a/coding-agent/tests/agentkit/tools/test_toolset.py +++ b/coding-agent/tests/agentkit/tools/test_toolset.py @@ -7,6 +7,7 @@ import pytest from agentkit.directive.types import Approve, AskUser, Reject +from agentkit.observability import SpanRecord from agentkit.plugin.registry import PluginRegistry from agentkit.runtime.hook_runtime import HookRuntime from agentkit.runtime.hookspecs import HOOK_SPECS @@ -20,6 +21,22 @@ ) +class RecordingObservationSink: + def __init__(self) -> None: + self.spans: list[SpanRecord] = [] + + def record_span(self, span: SpanRecord) -> None: + self.spans.append(span) + + def record_event(self, event: Any) -> None: + del event + + +class ToolContext: + def __init__(self, sink: RecordingObservationSink) -> None: + self.config = {"observation_sink": sink} + + class SingleToolPlugin: state_key = "single_tool" @@ -427,6 +444,63 @@ async def test_toolset_executes_single_tools_into_result_envelopes() -> None: assert plugin.calls == [("known_tool", {"value": "a"})] +@pytest.mark.asyncio +async def test_toolset_records_tool_call_span_without_arguments_or_result() -> None: + plugin = SingleToolPlugin() + sink = RecordingObservationSink() + toolset = Toolset(runtime=_runtime(plugin)) + + results = await toolset.execute_tools( + [ + ToolCallRequest( + tool_call_id="tc-1", + name="known_tool", + arguments={"secret": "do-not-record"}, + ) + ], + ctx=ToolContext(sink), + ) + + assert results[0] == ToolExecutionResult( + tool_call_id="tc-1", + name="known_tool", + result="ok:None", + ) + assert len(sink.spans) == 1 + assert sink.spans[0].name == "tool.call" + assert sink.spans[0].status == "ok" + assert sink.spans[0].attributes == { + "tool.name": "known_tool", + "tool.call_id": "tc-1", + "tool.missing": False, + "tool.error": False, + } + + +@pytest.mark.asyncio +async def test_toolset_records_error_tool_call_span() -> None: + sink = RecordingObservationSink() + toolset = Toolset(runtime=_runtime(FlakyToolPlugin())) + + results = await toolset.execute_tools( + [ToolCallRequest(tool_call_id="tc-1", name="known_tool", arguments={})], + ctx=ToolContext(sink), + options=ToolExecutionOptions(max_retries=0), + ) + + assert results[0].is_error is True + assert len(sink.spans) == 1 + assert sink.spans[0].name == "tool.call" + assert sink.spans[0].status == "ok" + assert sink.spans[0].attributes == { + "tool.name": "known_tool", + "tool.call_id": "tc-1", + "tool.missing": False, + "tool.error": True, + "tool.error_type": "RuntimeError", + } + + @pytest.mark.asyncio async def test_toolset_marks_missing_tool_without_raising() -> None: toolset = Toolset(runtime=_runtime(SingleToolPlugin())) @@ -523,7 +597,9 @@ async def test_toolset_approval_rejects_when_async_hook_raises( with caplog.at_level(logging.WARNING): approval = await toolset.approve_tool_call( - ToolCallRequest(tool_call_id="tc-async-hook", name="bash_run", arguments={}), + ToolCallRequest( + tool_call_id="tc-async-hook", name="bash_run", arguments={} + ), ctx=object(), ) @@ -592,7 +668,9 @@ async def test_toolset_approval_rejects_directive_when_executor_missing() -> Non toolset = Toolset(runtime=_runtime(ApprovalPlugin(result=directive))) approval = await toolset.approve_tool_call( - ToolCallRequest(tool_call_id="tc-missing-executor", name="bash_run", arguments={}), + ToolCallRequest( + tool_call_id="tc-missing-executor", name="bash_run", arguments={} + ), ctx=object(), ) @@ -607,7 +685,9 @@ async def test_toolset_approval_rejects_ask_user_when_executor_missing() -> None toolset = Toolset(runtime=_runtime(ApprovalPlugin(result=directive))) approval = await toolset.approve_tool_call( - ToolCallRequest(tool_call_id="tc-missing-executor", name="bash_run", arguments={}), + ToolCallRequest( + tool_call_id="tc-missing-executor", name="bash_run", arguments={} + ), ctx=object(), ) @@ -631,7 +711,9 @@ async def execute(self, directive: Approve) -> bool: ) approval = await toolset.approve_tool_call( - ToolCallRequest(tool_call_id="tc-async-approval", name="bash_run", arguments={}), + ToolCallRequest( + tool_call_id="tc-async-approval", name="bash_run", arguments={} + ), ctx=object(), ) @@ -926,7 +1008,9 @@ async def test_toolset_call_tool_preserves_proxy_hook_exception_type() -> None: @pytest.mark.asyncio -async def test_toolset_call_tool_rejects_invalid_proxy_arguments_before_execution() -> None: +async def test_toolset_call_tool_rejects_invalid_proxy_arguments_before_execution() -> ( + None +): proxy = ProxyToolPlugin() approval = RecordingApprovalPlugin() toolset = Toolset(runtime=_runtime(proxy, approval)) @@ -985,7 +1069,9 @@ async def test_toolset_call_tool_rejects_when_proxy_target_approval_denies() -> @pytest.mark.asyncio -async def test_toolset_call_tool_uses_proxy_hooks_for_conflicting_target_names() -> None: +async def test_toolset_call_tool_uses_proxy_hooks_for_conflicting_target_names() -> ( + None +): direct = ConflictingDirectToolPlugin() proxy = ProxyToolPlugin() toolset = Toolset(runtime=_runtime(direct, proxy)) @@ -1190,7 +1276,9 @@ async def test_toolset_retries_transient_batch_hook_failures() -> None: @pytest.mark.asyncio -async def test_toolset_falls_back_to_single_tool_execution_when_batch_hook_declines() -> None: +async def test_toolset_falls_back_to_single_tool_execution_when_batch_hook_declines() -> ( + None +): single_tool = SingleToolPlugin() toolset = Toolset(runtime=_runtime(NoneBatchToolPlugin(), single_tool))