From ff89f4609f6b5219b434863a6bbe4f4920b9365d Mon Sep 17 00:00:00 2001 From: mocha06 <52426811+mocha06@users.noreply.github.com> Date: Sat, 18 Jul 2026 01:17:01 -0300 Subject: [PATCH] fix(mcp): scope the not-found hint away from the LLM provider list tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _provider_tool_error_from_exception appended the get_llm_providers discovery hint to every classified not_found error, including failures inside get_llm_providers itself — telling the caller to retry the tool that just failed. A keyword-only not_found_hint (default True) now gates the hint; the list tool passes False while per-id tools keep it. Mirrors the same fix on the knowledge-base tools. Closes #429 --- .../pipefy_mcp/tools/llm_provider_tools.py | 11 +++-- .../tests/tools/test_llm_provider_tools.py | 49 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/packages/mcp/src/pipefy_mcp/tools/llm_provider_tools.py b/packages/mcp/src/pipefy_mcp/tools/llm_provider_tools.py index e8e9859a..695665f2 100644 --- a/packages/mcp/src/pipefy_mcp/tools/llm_provider_tools.py +++ b/packages/mcp/src/pipefy_mcp/tools/llm_provider_tools.py @@ -25,18 +25,23 @@ ) -def _provider_tool_error_from_exception(exc: BaseException) -> dict[str, Any]: +def _provider_tool_error_from_exception( + exc: BaseException, *, not_found_hint: bool = True +) -> dict[str, Any]: """Map an SDK/GraphQL exception onto the canonical tool failure envelope. Uses the shared SDK classifier (``pipefy_sdk.classify_exception``) so the kind/code the CLI and probes see is the same one reported here. A transport-level failure with no GraphQL errors falls back to ``str(exc)``. + ``not_found_hint`` scopes the id-discovery hint to the per-id tools; the + list tool passes False so its own failure never tells the caller to retry + the call that just failed. """ problem = classify_exception(exc) if problem is None: return tool_error(str(exc)) message = problem.message - if problem.kind.value == "not_found": + if not_found_hint and problem.kind.value == "not_found": message = f"{message} {_PROVIDER_ID_DISCOVERY_HINT}" details: dict[str, Any] = {"kind": problem.kind.value} if problem.correlation_id: @@ -119,7 +124,7 @@ async def get_llm_providers( after=after, ) except Exception as exc: # noqa: BLE001 - return _provider_tool_error_from_exception(exc) + return _provider_tool_error_from_exception(exc, not_found_hint=False) return _read_success( {"providers": result["providers"]}, message="LLM providers retrieved.", diff --git a/packages/mcp/tests/tools/test_llm_provider_tools.py b/packages/mcp/tests/tools/test_llm_provider_tools.py index 67f4d20e..8a8ffe02 100644 --- a/packages/mcp/tests/tools/test_llm_provider_tools.py +++ b/packages/mcp/tests/tools/test_llm_provider_tools.py @@ -51,6 +51,18 @@ def permission_denied_error() -> TransportQueryError: ) +def not_found_error() -> TransportQueryError: + return TransportQueryError( + "missing", + errors=[ + { + "message": "Couldn't find LlmProvider with id bogus", + "extensions": {"code": "RESOURCE_NOT_FOUND"}, + } + ], + ) + + @pytest.fixture def mock_provider_client(): client = MagicMock(PipefyClient) @@ -295,6 +307,43 @@ async def test_validate_llm_provider_access_failure_maps_problem( assert payload["error"]["details"]["correlation_id"] == "corr-2" +@pytest.mark.anyio +@pytest.mark.parametrize("provider_session", [None], indirect=True) +async def test_get_llm_providers_not_found_has_no_self_referential_hint( + provider_session, mock_provider_client, extract_payload +): + """A failed list must not tell the caller to retry the list tool itself.""" + mock_provider_client.get_llm_providers = AsyncMock(side_effect=not_found_error()) + async with provider_session as session: + result = await session.call_tool( + "get_llm_providers", {"organization_uuid": "org-uuid-1"} + ) + payload = extract_payload(result) + assert payload["success"] is False + assert payload["error"]["details"]["kind"] == "not_found" + assert "get_llm_providers" not in tool_error_message(payload) + + +@pytest.mark.anyio +@pytest.mark.parametrize("provider_session", [None], indirect=True) +async def test_get_llm_provider_dependencies_not_found_adds_discovery_hint( + provider_session, mock_provider_client, extract_payload +): + """A per-id tool keeps the discovery hint so the caller can find valid IDs.""" + mock_provider_client.get_llm_provider_dependencies = AsyncMock( + side_effect=not_found_error() + ) + async with provider_session as session: + result = await session.call_tool( + "get_llm_provider_dependencies", + {"provider_id": "bogus", "organization_uuid": "org-uuid-1"}, + ) + payload = extract_payload(result) + assert payload["success"] is False + assert payload["error"]["details"]["kind"] == "not_found" + assert "get_llm_providers" in tool_error_message(payload) + + @pytest.mark.anyio @pytest.mark.parametrize("provider_session", [None], indirect=True) async def test_transport_failure_without_graphql_errors_falls_back_to_str(