fix(middleware): report an unresolved tool lookup instead of a read only denial

[Om-singhaI]

fix(middleware): report an unresolved tool lookup instead of a read only denial

Fixes #387

Summary

Changes

ToolFilteringMiddleware.on_call_tool in mcp_proxy_for_aws/middleware/tool_filter.py resolves the requested tool with fastmcp.get_tool() before allowing a tools/call request in read only mode. On the proxy that lookup is served by the upstream MCP endpoint. With fastmcp's default provider_error_strategy="warn", AggregateProvider._get_highest_version_result logs and swallows any non NotFoundError exception raised by ProxyProvider._get_tool (which refreshes its list cache through client.list_tools()), so an upstream HTTP 429 makes get_tool() return None. The middleware collapsed "the lookup returned None" and "the tool resolved but has no readOnlyHint" into one error, so a throttled lookup of a tool that is annotated readOnlyHint=True was reported as Tool 'X' is not available in read-only mode., and the outer ToolErrorMiddleware forwarded that text to the caller.

This change separates the two conditions:

• When get_tool() returns None, the middleware logs a warning and raises ToolError("Tool 'X' could not be resolved from the upstream server, so its read-only hint could not be verified."). Through ToolErrorMiddleware the caller now sees Tool call 'X' failed: Tool 'X' could not be resolved from the upstream server, so its read-only hint could not be verified.. Please retry.
• When the tool resolves and lacks readOnlyHint, the existing Tool 'X' is not available in read-only mode. message is kept unchanged.
• An exception raised by the lookup itself was already propagated unchanged; a test now pins that behaviour.

No server wiring changes. The larger alternative, constructing FastMCPProxy with provider_error_strategy="raise" so the upstream 429 propagates as its own HTTPStatusError instead of a None lookup, would change error behaviour for every aggregate operation on the proxy and is better decided by the maintainers, so it is not part of this change. I also did not add caching of readOnlyHint values seen in on_list_tools, since #350 deliberately moved away from caching ("check tool annotations directly on call instead of caching").

Out of scope: the secondary report in #387 about 41 failures rendering as Error calling tool 'read_k8s_resource': . with an empty message. That text is produced below this middleware (the proxied tool call itself), not by the read only check, and is not addressed here.

User experience

Before: in a --read-only session, once the upstream endpoint starts returning 429, every tools/call is refused with Tool 'X' is not available in read-only mode. even for tools annotated readOnlyHint=True that had just returned data. Agents read that as the toolset being withdrawn and stop retrying.

After: the same situation is reported as Tool 'X' could not be resolved from the upstream server, so its read-only hint could not be verified. (wrapped by ToolErrorMiddleware with the usual Please retry. suffix), so the caller can back off and retry. Tools that actually lack readOnlyHint still get the read only message.

I reproduced both messages end to end against a real FastMCPProxy (fastmcp 3.4.7, the version in uv.lock) with ToolErrorMiddleware and ToolFilteringMiddleware(read_only=True) installed in the same order as server.py, a backend tool annotated readOnlyHint=True, and a client whose list_tools raises httpx.HTTPStatusError 429 after the first successful call with the provider list cache expired:

On main:

1) tools/list while healthy: ['read_k8s_resource']
2) tools/call while healthy: resource pod-a
3) tools/call during 429: Tool call 'read_k8s_resource' failed: Tool 'read_k8s_resource' is not available in read-only mode.. Please retry.
4) proxy.get_tool during 429 -> None

With this change:

3) tools/call during 429: Tool call 'read_k8s_resource' failed: Tool 'read_k8s_resource' could not be resolved from the upstream server, so its read-only hint could not be verified.. Please retry.

Checklist

• I have reviewed the contributing guidelines
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented (no user facing documentation mentions the read only error text; CHANGELOG.md is generated by commitizen on version bump, so it was not edited)

Is this a breaking change? (Y/N)

• Yes
• No

Please add details about how this change was tested.

• Did integration tests succeed? (not run; they require a live AWS hosted MCP endpoint)
• If the feature is a new use case, is it necessary to add a new integration test case? (no; the behaviour is unit tested at the middleware boundary)

Unit tests in tests/unit/test_tool_filter.py:

• test_read_only_true_rejects_unresolved_tool_without_read_only_message (replaces test_read_only_true_rejects_unknown_tool, which asserted the old collapsed message for a None lookup): a None lookup raises ToolError containing could not be resolved and not containing not available in read-only mode, and call_next is not called. Fails on main, passes with the change.
• test_read_only_true_propagates_lookup_error (new): an exception raised by get_tool propagates unchanged and call_next is not called.
• test_read_only_true_rejects_write_tool (existing, one assertion added): a resolved tool with readOnlyHint=False still gets the read only message and not the new one.
• test_read_only_true_allows_read_only_tool (existing, unchanged): a resolved tool with readOnlyHint=True passes through.

Results with Python 3.10.6, fastmcp 3.4.7, pytest 9.1.1, pytest asyncio 1.4.0:

• tests/unit/test_tool_filter.py on main: 18 passed.
• tests/unit/test_tool_filter.py with the new tests but the source change reverted: 1 failed (test_read_only_true_rejects_unresolved_tool_without_read_only_message), 18 passed.
• tests/unit/test_tool_filter.py with the change: 19 passed.
• tests/unit with the change: 261 passed, 2 failed. The two failures, tests/unit/test_server.py::TestFix2EnvVarPrecedence::test_env_var_profiles_wins_over_aws_profile and tests/unit/test_server.py::TestEnvVarActivatesMiddleware::test_env_var_activates_profile_middleware, fail identically on pristine main in this environment. They are async tests decorated with unittest.mock.patch.dict; on the local Python 3.10.6 interpreter that decorator returns a non coroutine wrapper (inspect.iscoroutinefunction is False), so pytest asyncio reports "async def functions are not natively supported". This is unrelated to the files touched here.

ruff check and ruff format --check pass on both changed files. pyright was not run locally (not installed in this environment).

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[Om-singhaI]

There are two ways to fix this and I'd rather the call got made before it goes further.

The narrow one is what's here: when get_tool returns None, say the lookup failed instead of reporting a read only denial. The broader one is to let it propagate by building the proxy with provider_error_strategy="raise", which changes error behavior for every aggregate operation, so I didn't want to pick that for you.

@arnewouters this lands on you because your review on #350 asked for the annotation to be checked on the tool at call time rather than kept in a list, and that live lookup is what the 429 in #387 breaks.

No workflow has run on this yet and #395 is the same, so I think it needs someone with write access to release it. Branch is current with main.

[Om-singhaI]

Could someone take a look at this when you get a chance? I'm happy to rework it if the approach isn't right.