diff --git a/sdks/python/agenta/sdk/agents/tools/models.py b/sdks/python/agenta/sdk/agents/tools/models.py index aa40a0a58f..0de761a677 100644 --- a/sdks/python/agenta/sdk/agents/tools/models.py +++ b/sdks/python/agenta/sdk/agents/tools/models.py @@ -446,6 +446,10 @@ class ResolvedToolSet(BaseModel): validation_alias=AliasChoices("tool_specs", "custom_tools"), ) tool_callback: Optional[ToolCallback] = None + # Human-facing warnings raised during resolution that did not fail the run — e.g. a + # gateway tool dropped because its action 404s. Each names the affected tool. Surfaced so + # a degraded resolution is never silent; empty on a fully clean resolve. + warnings: List[str] = Field(default_factory=list) @field_validator("tool_specs", mode="before") @classmethod diff --git a/sdks/python/agenta/sdk/agents/tools/resolver.py b/sdks/python/agenta/sdk/agents/tools/resolver.py index 187714840c..9c1459befe 100644 --- a/sdks/python/agenta/sdk/agents/tools/resolver.py +++ b/sdks/python/agenta/sdk/agents/tools/resolver.py @@ -10,6 +10,7 @@ from .errors import ( DuplicateToolNameError, + GatewayToolResolutionError, MissingToolSecretError, ReservedToolNameError, UnsupportedToolProviderError, @@ -135,6 +136,19 @@ def _validate_unique_names(tool_specs: Sequence[ToolSpec]) -> None: _check_tool_name(tool_spec.name, seen) +def _dropped_gateway_tool_warning( + tool_config: GatewayToolConfig, error: GatewayToolResolutionError +) -> str: + """Message for a gateway tool dropped because it failed to resolve. + + Names the tool (its declared name, else its full reference) and carries the resolver's + own reason string, which already names the failing action for the stale-action (404) + case. Surfaced as a warning so a dropped tool is never silent. + """ + label = tool_config.name or tool_config.reference + return f"gateway tool '{label}' failed to resolve and was dropped: {error}" + + class ToolResolver: """Resolve canonical tool configuration through injected secret and gateway adapters.""" @@ -203,6 +217,7 @@ async def resolve(self, tool_configs: Sequence[ToolConfig]) -> ResolvedToolSet: ) tool_specs: list[ToolSpec] = [] + warnings: list[str] = [] for tool_config in code_configs: missing = [ secret_name @@ -257,14 +272,37 @@ async def resolve(self, tool_configs: Sequence[ToolConfig]) -> ResolvedToolSet: if gateway_configs: if self._gateway_resolver is None: raise UnsupportedToolProviderError(gateway_configs[0].provider) - gateway_resolution = await self._gateway_resolver.resolve(gateway_configs) - tool_specs = [*gateway_resolution.tool_specs, *tool_specs] - # Gateway, workflow, and platform callbacks all point at ``{api}/tools/call`` with the - # same per-request auth, so the single shared callback is identical; keep one. - tool_callback = gateway_resolution.tool_callback or tool_callback + # Resolve each gateway tool independently so one dead tool (e.g. a Composio action + # that has left the catalog and now 404s) is dropped with a named warning instead of + # bricking the whole run. The tools that do resolve are kept and the run proceeds. + gateway_specs: list[ToolSpec] = [] + for gateway_config in gateway_configs: + try: + gateway_resolution = await self._gateway_resolver.resolve( + [gateway_config] + ) + except GatewayToolResolutionError as error: + # Only a per-tool "action not found" (HTTP 404, the F-019 stale-action case) + # is dropped: the backend has told us this specific action left the catalog. + # Any other failure — missing API base, transport error, HTTP 400/500, or a + # malformed backend response — is systemic (it would hit every tool), so it + # must fail the run loudly rather than silently drop every tool. + if error.status != 404: + raise + warning = _dropped_gateway_tool_warning(gateway_config, error) + log.warning("agent: %s", warning) + warnings.append(warning) + continue + gateway_specs.extend(gateway_resolution.tool_specs) + # Gateway, workflow, and platform callbacks all point at ``{api}/tools/call`` + # with the same per-request auth, so the single shared callback is identical; + # keep one. + tool_callback = gateway_resolution.tool_callback or tool_callback + tool_specs = [*gateway_specs, *tool_specs] _validate_unique_names(tool_specs) return ResolvedToolSet( tool_specs=tool_specs, tool_callback=tool_callback, + warnings=warnings, ) diff --git a/sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py b/sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py index 2d4afd5d0a..4b1e329680 100644 --- a/sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py +++ b/sdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.py @@ -13,6 +13,7 @@ coerce_tool_configs, GatewayToolConfig, GatewayToolResolution, + GatewayToolResolutionError, MissingSecretPolicy, MissingToolSecretError, PlatformToolConfig, @@ -166,6 +167,71 @@ async def test_gateway_metadata_survives_resolution(): assert spec.render == {"kind": "component", "component": "User"} +class PartialGatewayResolver(FakeGatewayResolver): + """Resolves like the fake, but raises for any tool whose action is in ``dead_actions``. + + Models the platform ``/tools/resolve`` 404 for a Composio action that has left the + catalog. Because the resolver is now called once per tool, one dead action fails only + its own resolution. + """ + + def __init__(self, dead_actions: Sequence[str]): + self.dead_actions = set(dead_actions) + + async def resolve( + self, + tools: Sequence[GatewayToolConfig], + ) -> GatewayToolResolution: + for tool in tools: + if tool.action in self.dead_actions: + raise GatewayToolResolutionError( + f"Gateway tool resolution failed: Action not found: " + f"composio/{tool.integration}/{tool.action} (HTTP 404)", + status=404, + ) + return await super().resolve(tools) + + +async def test_one_dead_gateway_tool_is_dropped_and_the_rest_resolve(caplog): + # A gateway agent with two Composio tools where one action 404s must not brick the run: + # the good tool resolves, the dead one is dropped with a warning that names it. + resolver = ToolResolver( + gateway_resolver=PartialGatewayResolver(dead_actions=["COMMIT_MULTIPLE_FILES"]) + ) + with caplog.at_level("WARNING"): + resolved = await resolver.resolve( + [ + GatewayToolConfig( + integration="github", action="GET_USER", connection="c1" + ), + GatewayToolConfig( + integration="github", + action="COMMIT_MULTIPLE_FILES", + connection="c1", + ), + ] + ) + + names = [spec.name for spec in resolved.tool_specs] + assert names == ["github__GET_USER"] # the good tool survived; the dead one is gone + + assert len(resolved.warnings) == 1 + warning = resolved.warnings[0] + # The warning names the dropped tool (by its reference) and carries the backend's 404 reason. + assert "tools.composio.github.COMMIT_MULTIPLE_FILES.c1" in warning + assert "Action not found: composio/github/COMMIT_MULTIPLE_FILES" in warning + # And it is also emitted to the logs, so a degraded resolution is never silent. + assert any("COMMIT_MULTIPLE_FILES" in r.message for r in caplog.records) + + +async def test_a_clean_gateway_resolution_carries_no_warnings(): + resolved = await ToolResolver(gateway_resolver=FakeGatewayResolver()).resolve( + [GatewayToolConfig(integration="github", action="GET_USER", connection="c1")] + ) + assert resolved.warnings == [] + assert [spec.name for spec in resolved.tool_specs] == ["github__GET_USER"] + + async def test_authored_permission_lands_on_resolved_code_spec_wire(): # An author's Layer-3 permission on a config rides through resolution onto the wire. resolved = await ToolResolver().resolve( diff --git a/services/oss/tests/pytest/integration/agent/conftest.py b/services/oss/tests/pytest/integration/agent/conftest.py index ba8821a017..69ad8cf343 100644 --- a/services/oss/tests/pytest/integration/agent/conftest.py +++ b/services/oss/tests/pytest/integration/agent/conftest.py @@ -10,7 +10,7 @@ from __future__ import annotations import json -from typing import Any, Dict, Optional +from typing import Any, Callable, Dict, Optional import pytest @@ -27,7 +27,7 @@ def json(self) -> Any: return self._payload -def _fake_async_client(*, response, raises, capture: Dict[str, Any]): +def _fake_async_client(*, make_response, raises, capture: Dict[str, Any]): class _Client: def __init__(self, *args, **kwargs) -> None: pass @@ -42,13 +42,13 @@ async def post(self, url, json=None, headers=None): capture.update(method="POST", url=url, json=json, headers=headers) if raises: raise raises - return response + return make_response(json) async def get(self, url, headers=None): capture.update(method="GET", url=url, headers=headers) if raises: raise raises - return response + return make_response(None) return _Client @@ -62,6 +62,7 @@ def _install( payload: Any = None, text: Optional[str] = None, raises: Optional[BaseException] = None, + responder: Optional[Callable[[Any], Any]] = None, api_base: Optional[str] = "https://api.x/api", authorization: Optional[str] = "Access tok", ) -> Dict[str, Any]: @@ -70,11 +71,27 @@ def _install( monkeypatch.setattr( platform_connection, "_derive_authorization", lambda: authorization ) - response = _FakeResponse(status, payload, text) + + def make_response(request_json): + # Tools now resolve one per HTTP call, so a test can vary the response by request + # via ``responder`` (which receives the outgoing request body). Without one, the + # single static ``payload`` is returned for every call, as before. A responder may + # return a ``(status, payload)`` tuple to control the per-call status (e.g. a 404 for + # one tool and a 200 for the next), or a bare payload to reuse the default ``status``. + if responder is not None: + result = responder(request_json) + if isinstance(result, tuple): + call_status, call_payload = result + return _FakeResponse(call_status, call_payload, text) + return _FakeResponse(status, result, text) + return _FakeResponse(status, payload, text) + monkeypatch.setattr( module.httpx, "AsyncClient", - _fake_async_client(response=response, raises=raises, capture=capture), + _fake_async_client( + make_response=make_response, raises=raises, capture=capture + ), ) return capture diff --git a/services/oss/tests/pytest/integration/agent/tools/test_gateway_http.py b/services/oss/tests/pytest/integration/agent/tools/test_gateway_http.py index 4d8d8cba97..9952beebdf 100644 --- a/services/oss/tests/pytest/integration/agent/tools/test_gateway_http.py +++ b/services/oss/tests/pytest/integration/agent/tools/test_gateway_http.py @@ -68,26 +68,33 @@ async def test_gateway_metadata_and_description_fallback_are_preserved(install_h assert capture["json"]["tools"][0]["type"] == "gateway" -async def test_gateway_specs_are_joined_by_call_ref_not_position(install_http): - install_http( - gateway, - payload={ - "custom": [ - { - "name": "second", - "description": "Second", - "input_schema": {}, - "call_ref": "tools.composio.github.SECOND.c2", - }, - { - "name": "first", - "description": "First", - "input_schema": {}, - "call_ref": "tools.composio.github.FIRST.c1", - }, - ] +async def test_each_gateway_tool_resolves_on_its_own_call_carrying_its_metadata( + install_http, +): + # Gateway tools now resolve one per HTTP call (so one dead action can be dropped without + # bricking the rest). Each returned spec is matched to its requesting ref by call_ref, and + # each tool's authored metadata (permission, render) lands on its own resulting spec. + specs_by_ref = { + "FIRST": { + "name": "first", + "description": "First", + "input_schema": {}, + "call_ref": "tools.composio.github.FIRST.c1", }, - ) + "SECOND": { + "name": "second", + "description": "Second", + "input_schema": {}, + "call_ref": "tools.composio.github.SECOND.c2", + }, + } + + def responder(request_json): + # The resolver posts one ref per call; echo back the single matching spec. + action = request_json["tools"][0]["action"] + return {"custom": [specs_by_ref[action]]} + + install_http(gateway, responder=responder) resolved = await resolve_tools( [ { @@ -111,6 +118,74 @@ async def test_gateway_specs_are_joined_by_call_ref_not_position(install_http): assert second.name == "second" assert second.permission is None # unset inherits: rules, then the policy default assert second.render == {"kind": "component", "component": "Second"} + assert resolved.warnings == [] + + +async def test_one_dead_action_is_dropped_and_the_rest_resolve(install_http): + # The F-019 fix, end to end through the real gateway HTTP path: when the backend answers + # one tool's /tools/resolve with a 404 (the action left the catalog), that tool is dropped + # with a warning that names it, and the sibling tool still resolves and runs. + def responder(request_json): + action = request_json["tools"][0]["action"] + if action == "COMMIT_MULTIPLE_FILES": + return ( + 404, + {"detail": "Action not found: composio/github/COMMIT_MULTIPLE_FILES"}, + ) + return { + "custom": [ + { + "name": "get_user", + "description": "Get user", + "input_schema": {}, + "call_ref": "tools.composio.github.GET_USER.c1", + } + ] + } + + install_http(gateway, responder=responder) + resolved = await resolve_tools( + [ + {**_GATEWAY, "action": "GET_USER", "connection": "c1"}, + {**_GATEWAY, "action": "COMMIT_MULTIPLE_FILES", "connection": "c1"}, + ] + ) + + assert [spec.name for spec in resolved.tool_specs] == ["get_user"] + assert len(resolved.warnings) == 1 + warning = resolved.warnings[0] + assert "tools.composio.github.COMMIT_MULTIPLE_FILES.c1" in warning + assert "Action not found: composio/github/COMMIT_MULTIPLE_FILES" in warning + assert isinstance(resolved.tool_callback, ToolCallback) + + +async def test_a_non_404_gateway_failure_still_fails_the_run(install_http): + # Only a per-tool 404 is tolerated. A systemic failure (here HTTP 400) would hit every tool, + # so it must fail the whole resolution loudly rather than silently drop tools. + def responder(request_json): + action = request_json["tools"][0]["action"] + if action == "COMMIT_MULTIPLE_FILES": + return (400, {"detail": "Connection is inactive"}) + return { + "custom": [ + { + "name": "get_user", + "description": "Get user", + "input_schema": {}, + "call_ref": "tools.composio.github.GET_USER.c1", + } + ] + } + + install_http(gateway, responder=responder) + with pytest.raises(GatewayToolResolutionError) as caught: + await resolve_tools( + [ + {**_GATEWAY, "action": "GET_USER", "connection": "c1"}, + {**_GATEWAY, "action": "COMMIT_MULTIPLE_FILES", "connection": "c1"}, + ] + ) + assert caught.value.status == 400 async def test_transport_failure_is_logged_and_normalized(