diff --git a/pyproject.toml b/pyproject.toml index cf92891..0b15a20 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "agentcat" -version = "2.0.0" +version = "2.0.1" description = "Analytics tool for MCP (Model Context Protocol) servers, Claude Connectors, and ChatGPT Plugins - tracks tool usage patterns and provides insights" authors = [ { name = "AgentCat, Inc.", email = "support@agentcat.com" }, diff --git a/src/agentcat/modules/adapters/community.py b/src/agentcat/modules/adapters/community.py index d3cded4..de395be 100644 --- a/src/agentcat/modules/adapters/community.py +++ b/src/agentcat/modules/adapters/community.py @@ -12,7 +12,7 @@ :mod:`agentcat.modules.callpath` and is not re-derived here — the v1 middleware this replaces reimplemented all of it inline. -Three invariants the middleware exists to protect: +Four invariants the middleware exists to protect: - **Nothing customer-owned is mutated.** Tool schemas are copied before the injection pipeline rewrites them in place, and the request message is cloned @@ -25,6 +25,14 @@ never caches our injected schemas. - **The event records the call as the agent made it.** Raw (unstripped) arguments and the customer's undecorated result; the mint-back is wire-only. +- **A server-internal call is not an agent.** While a tracked call is in + flight, a nested ``tools/list`` on the same server is served verbatim (no + injection, no registry rebuild) and a nested ``tools/call`` joins the + enclosing call's session, is tagged ``agentcat_nested``, and is never + decorated — a code-mode sandbox reading its own catalog or chaining its own + tools must see the customer's data, not AgentCat's, and must never + overwrite the registries the AGENT-facing listing built. See the + re-entrancy block above ``_CallFrame``. ``AgentCatMiddleware`` deliberately does not subclass ``fastmcp``'s ``Middleware``: that would require a module-scope ``fastmcp`` import, and @@ -35,8 +43,12 @@ from __future__ import annotations +import contextlib +import contextvars import copy import weakref +from collections.abc import Iterator +from dataclasses import replace from typing import Any from agentcat.modules.adapters._common import ( @@ -57,6 +69,7 @@ ) from agentcat.modules.constants import GET_MORE_TOOLS_NAME from agentcat.modules.exceptions import capture_exception +from agentcat.modules.handles import HandleResolution from agentcat.modules.injection import ToolSpec, build_injected_schemas from agentcat.modules.logging import write_to_log from agentcat.modules.request_extra import extra_from_request_context @@ -153,6 +166,85 @@ def _flattened(result: Any) -> Any: return result +# ── re-entrancy: telling the agent's traffic from the server's own ─────────── +# +# A customer tool can drive its own server mid-call: FastMCP's +# ``CatalogTransform.get_tool_catalog`` lists the raw backend catalog with +# ``run_middleware=True`` (code mode's discovery and execute tools do this on +# every call), and a tool body may call ``ctx.fastmcp.call_tool`` for a +# sibling tool. Both re-enter this middleware, and both must be recognized: +# a nested listing rebuilt the injection registries from the backend catalog +# (clobbering the agent-facing ones, after which the strip removed nothing and +# the agent's next echoed ``session_id`` failed FastMCP validation), and a +# nested call minted its own session and carried mint-back decoration into a +# sandbox where agent-authored code tripped over it. +# +# The marker cannot live on the middleware object — one instance serves every +# connection concurrently. It is a ContextVar tuple-stack of frames, the same +# token set/reset pattern as ``_inner_tap``'s ``_open_seams``: a nested call +# runs in the same task (or a child task's copied context) as the call that +# made it, so the outer frame is visible exactly as far down as the nesting +# reaches, and two top-level calls — separate tasks, separate contexts — can +# never see each other's. FastMCP's own ``Context._request_state`` was +# considered as the carrier (nested contexts share it by reference) and +# rejected: the sharing only exists from mid-3.x, so on earlier releases the +# frame would silently never propagate. A ContextVar has the same reach on +# every release, and concurrent inner calls (``asyncio.gather`` in a tool +# body) each get their own context copy rather than racing writes in a shared +# dict. +# +# Frames still carry their server: one request can nest calls ACROSS servers +# (mounted and multi-server setups), and a frame from server A must be +# invisible to B's middleware — readers compare identity, never equality. + +_call_frames: contextvars.ContextVar[tuple[_CallFrame, ...]] = contextvars.ContextVar( + "agentcat_community_call_frames", default=() +) + + +class _CallFrame: + """One in-flight ``on_call_tool`` on one server. + + ``resolution`` starts None and is assigned once the call resolves; the + stack holds a REFERENCE to the frame, so the late write is visible to + nested readers. A nested call that arrives before the outer resolve + completes (a customer hook driving its own server) sees None and simply + behaves as a top-level call — the listing pass-through, which protects the + registries, needs only the frame itself. + """ + + __slots__ = ("server", "resolution") + + def __init__(self, server: Any) -> None: + self.server = server + self.resolution: HandleResolution | None = None + + +def _enclosing_frame(server: Any) -> _CallFrame | None: + """The innermost open frame belonging to ``server``, or None.""" + for frame in reversed(_call_frames.get()): + if frame.server is server: + return frame + return None + + +@contextlib.contextmanager +def _call_frame(server: Any) -> Iterator[tuple[_CallFrame | None, _CallFrame]]: + """Push this call's frame; yield ``(enclosing, frame)``. + + A token reset, not a pop: it restores exactly the stack that was open + before, on every exit path including a raise, so an enclosing call gets + its own stack back and a sibling that follows starts clean. + """ + enclosing = _enclosing_frame(server) + frame = _CallFrame(server) + token = _call_frames.set((*_call_frames.get(), frame)) + try: + yield enclosing, frame + finally: + _call_frames.reset(token) + + class AgentCatMiddleware: """One middleware for both community eras; ``era`` selects the field bridge. @@ -317,6 +409,14 @@ async def on_list_tools(self, context: Any, call_next: Any) -> Any: Publishes nothing: v2 intercepts ``tools/list`` for schema injection only. On any failure the customer's own list is served unmodified. """ + # A listing the customer's own tool is making mid-call — a code-mode + # catalog fetch, most commonly — is served VERBATIM: no injection, no + # registry writes, no concession. Injecting would hand agent-facing + # parameters to a sandbox that cannot echo them, and rebuilding the + # registries from the raw backend catalog would replace the ones the + # agent-facing listing built (see the re-entrancy block above). + if _enclosing_frame(self._server) is not None: + return await call_next(context) # The list handed back here has passed through every layer below us, so # it may hold COPIES of our own tool — hence authoritative=False. tools = await self._concede_get_more_tools( @@ -478,101 +578,138 @@ async def rebuild() -> list[ToolSpec]: ) return [spec for spec in specs if spec is not None] - stripped = await get_stripped_arguments( - tracking, options, name, raw_arguments, rebuild - ) - stripped_context = context.copy( - message=message.model_copy(update={"arguments": stripped}) - ) - - # Tracing off: still strip what we injected, so the customer's tool runs - # exactly as it would untracked — then get out of the way. No handle - # resolution, no mint-back (there is no session_id parameter to echo), and - # no event. - if not options.enable_tracing: - return await call_next(stripped_context) - - request_context = _request_context(context) - try: - # Resolved fresh every round: the ResolvedCall carries this round's - # message/extra for the customer's tag and property callbacks. - resolved = await resolve_call( - tracking, - name, - raw_arguments, - message, - request_context, - meta_sources=self._meta_sources(message, request_context), - legacy_client=lambda: self._legacy_client_info(request_context), + # The frame is open for the whole call — including the tracing-off and + # degraded returns below, whose nested listings must still pass through + # (context injection is independent of tracing, so a clobbered registry + # would eat the next call's `context` there too). + with _call_frame(self._server) as (enclosing, frame): + stripped = await get_stripped_arguments( + tracking, options, name, raw_arguments, rebuild ) - except Exception as e: - # Belt and braces at the customer boundary: the resolvers are all - # documented not to raise, but a tool call must never fail because - # analytics did. Degrade to an untraced call. - write_to_log( - f"Warning: AgentCat resolution failed for tool '{name}', running " - f"it untraced - {e}" + stripped_context = context.copy( + message=message.model_copy(update={"arguments": stripped}) ) - return await call_next(stripped_context) - started = now_ms() - extra_params = extra_from_request_context( - request_context, getattr(context, "fastmcp_context", None) - ) + # Tracing off: still strip what we injected, so the customer's tool + # runs exactly as it would untracked — then get out of the way. No + # handle resolution, no mint-back (there is no session_id parameter + # to echo), and no event. + if not options.enable_tracing: + return await call_next(stripped_context) - # The tap's slot is open for exactly the rest of the chain and the - # publish that reads it, and closes on every exit path including the - # raise below. - with inner_tap() as tap: + request_context = _request_context(context) try: - result = await call_next(stripped_context) + # Resolved fresh every round: the ResolvedCall carries this + # round's message/extra for the customer's tag and property + # callbacks. + resolved = await resolve_call( + tracking, + name, + raw_arguments, + message, + request_context, + meta_sources=self._meta_sources(message, request_context), + legacy_client=lambda: self._legacy_client_info(request_context), + ) except Exception as e: - # FastMCP surfaces a failing tool as a raised error, so unlike - # the official adapters this path holds the live exception, - # with its type and traceback intact. + # Belt and braces at the customer boundary: the resolvers are + # all documented not to raise, but a tool call must never fail + # because analytics did. Degrade to an untraced call. + write_to_log( + f"Warning: AgentCat resolution failed for tool '{name}', " + f"running it untraced - {e}" + ) + return await call_next(stripped_context) + + # A nested call is the customer's own server calling itself from + # inside the enclosing call — one logical agent action. It joins + # the enclosing session rather than minting its own, and + # `prompts_session_id=False` keeps the tags honest: there is no + # parameter here for an agent to echo, because the enclosing frame + # is the only caller. Everything per-call — actor, client, intent — + # keeps its own resolution. + inherited = enclosing.resolution if enclosing is not None else None + nested = inherited is not None + if inherited is not None: + resolved.resolution = replace( + resolved.resolution, + session_id=inherited.session_id, + session_source=inherited.session_source, + prompts_session_id=False, + ) + # Assigned post-override, so nesting of nesting inherits the + # OUTERMOST session: the whole tree is one logical call. + frame.resolution = resolved.resolution + + started = now_ms() + extra_params = extra_from_request_context( + request_context, getattr(context, "fastmcp_context", None) + ) + + # The tap's slot is open for exactly the rest of the chain and the + # publish that reads it, and closes on every exit path including + # the raise below. + with inner_tap() as tap: + try: + result = await call_next(stripped_context) + except Exception as e: + # FastMCP surfaces a failing tool as a raised error, so + # unlike the official adapters this path holds the live + # exception, with its type and traceback intact. + await self._publish( + tracking, + resolved, + name, + raw_arguments, + response=None, + is_error=True, + error=capture_exception(e), + started=started, + mrtr=None, + extra_params=extra_params, + nested=nested, + ) + raise + + mrtr = detect_mrtr( + self._result_type(result), + getattr(message, "input_responses", None) is not None, + getattr(message, "request_state", None) is not None, + ) + is_error = bool(getattr(result, "is_error", False)) await self._publish( tracking, resolved, name, raw_arguments, - response=None, - is_error=True, - error=capture_exception(e), + response=response_payload(result), + is_error=is_error, + # An `is_error` result that never raised THROUGH us: a + # layer below answered with one. The tap holds the + # exception when a local one produced it, and there simply + # is none when a proxy passed an upstream error through — + # then the surfaced message is the whole truth. + error=tap.error(_flattened(result)) if is_error else None, started=started, - mrtr=None, + mrtr=mrtr, extra_params=extra_params, + nested=nested, ) - raise - - mrtr = detect_mrtr( - self._result_type(result), - getattr(message, "input_responses", None) is not None, - getattr(message, "request_state", None) is not None, - ) - is_error = bool(getattr(result, "is_error", False)) - await self._publish( - tracking, - resolved, - name, - raw_arguments, - response=response_payload(result), - is_error=is_error, - # An `is_error` result that never raised THROUGH us: a layer - # below answered with one. The tap holds the exception when a - # local one produced it, and there simply is none when a proxy - # passed an upstream error through — then the surfaced message - # is the whole truth. - error=tap.error(_flattened(result)) if is_error else None, - started=started, - mrtr=mrtr, - extra_params=extra_params, - ) - # An intermediate multi-round-trip round is never decorated: only the - # completing round carries the mint-back (changelog 6.4). - if mrtr == "input_required": - return result - return self._decorated(result, resolved, name, tracking) + # An intermediate multi-round-trip round is never decorated: only + # the completing round carries the mint-back (changelog 6.4). + if mrtr == "input_required": + return result + # A nested call's "wire" is the customer's own tool body — a + # code-mode sandbox, a composing tool. Decoration there is not an + # instruction to an agent, it is corrupted data: agent-authored + # code chokes on the extra key, and a sandbox that returns the + # inner dict verbatim advertises a stale handle on the real wire. + # `prompts_session_id=False` already silences the session mint-back; + # this return also closes the agent_id-confirmed mirror branch. + if nested: + return result + return self._decorated(result, resolved, name, tracking) def _result_type(self, result: Any) -> str | None: """The 2026-era ``resultType`` discriminator, however it is spelled.""" @@ -597,6 +734,7 @@ async def _publish( started: int, mrtr: str | None, extra_params: dict[str, Any], + nested: bool = False, ) -> None: await publish_tool_call_event( self._server, @@ -610,6 +748,7 @@ async def _publish( duration_ms=now_ms() - started, mrtr=mrtr, extra_params=extra_params, + nested=nested, ) def _decorated( diff --git a/src/agentcat/modules/callpath.py b/src/agentcat/modules/callpath.py index 1ad9b02..93ce149 100644 --- a/src/agentcat/modules/callpath.py +++ b/src/agentcat/modules/callpath.py @@ -272,6 +272,7 @@ async def publish_tool_call_event( duration_ms: int | None, mrtr: str | None, extra_params: dict[str, Any] | None, + nested: bool = False, ) -> None: """Publish the one event a v2 tool call produces. Never raises. @@ -312,7 +313,7 @@ async def publish_tool_call_event( # from the customer cap. event.tags = { **(event.tags or {}), - **build_handle_tags(rc.resolution, rc.protocol_version, mrtr), + **build_handle_tags(rc.resolution, rc.protocol_version, mrtr, nested), } event_queue.publish_event(server_key, event) except Exception as e: diff --git a/src/agentcat/modules/constants.py b/src/agentcat/modules/constants.py index 1e5462a..2b433a8 100644 --- a/src/agentcat/modules/constants.py +++ b/src/agentcat/modules/constants.py @@ -34,6 +34,10 @@ AGENTCAT_TAG_AGENT_SOURCE = "agentcat_agent_id_source" AGENTCAT_TAG_PROTOCOL_VERSION = "agentcat_protocol_version" AGENTCAT_TAG_MRTR = "agentcat_mrtr" +# Presence-gated: rides only on tools/call events the customer's own server +# made from inside another tracked call (a code-mode sandbox, a tool calling +# a sibling tool). Value is always "true"; absent on agent-facing calls. +AGENTCAT_TAG_NESTED = "agentcat_nested" AGENTCAT_CUSTOM_EVENT_TYPE = "agentcat:custom" # ── Explicit handles: agent-facing copy (byte-identical to TS constants.ts) ── diff --git a/src/agentcat/modules/handles.py b/src/agentcat/modules/handles.py index 1c11d5b..e630bc8 100644 --- a/src/agentcat/modules/handles.py +++ b/src/agentcat/modules/handles.py @@ -15,6 +15,7 @@ AGENTCAT_TAG_AGENT_ID, AGENTCAT_TAG_AGENT_SOURCE, AGENTCAT_TAG_MRTR, + AGENTCAT_TAG_NESTED, AGENTCAT_TAG_PROTOCOL_VERSION, AGENTCAT_TAG_SESSION_SOURCE, MCP_INSTRUCTIONS_KEY, @@ -304,6 +305,7 @@ def build_handle_tags( res: HandleResolution, protocol_version: str | None = None, mrtr: str | None = None, + nested: bool = False, ) -> dict[str, str]: tags: dict[str, str] = {AGENTCAT_TAG_SESSION_SOURCE: res.session_source} if res.agent_id and res.agent_source: @@ -313,4 +315,6 @@ def build_handle_tags( tags[AGENTCAT_TAG_PROTOCOL_VERSION] = _clamp_tag_value(protocol_version) if mrtr: tags[AGENTCAT_TAG_MRTR] = mrtr + if nested: + tags[AGENTCAT_TAG_NESTED] = "true" return tags diff --git a/tests/community/test_community_v3_nested_calls.py b/tests/community/test_community_v3_nested_calls.py new file mode 100644 index 0000000..46d1709 --- /dev/null +++ b/tests/community/test_community_v3_nested_calls.py @@ -0,0 +1,403 @@ +"""Server-internal (nested) traffic on the community adapter. + +A customer tool can drive its own server mid-call — fastmcp's code mode does +it on every `execute`: `CatalogTransform.get_tool_catalog` performs a nested +`tools/list` with `run_middleware=True`, and the sandbox's `call_tool` a +nested `tools/call`. This file pins the re-entrancy contract those paths get: +the nested listing is served verbatim (no injection, no registry rebuild — the +rebuild is what clobbered the agent-facing registries and made the agent's +echoed `session_id` fail FastMCP validation), and the nested call joins the +enclosing session, is tagged `agentcat_nested`, and is never decorated. + +The server shape comes from `tests.test_utils.community_catalog_server`: the +code-mode mechanism reproduced without pydantic-monty, with an `observed` dict +recording what the "sandbox" side actually saw. +""" + +import pytest + +from agentcat import AgentCatOptions, track +from agentcat.modules.constants import ( + AGENTCAT_TAG_NESTED, + AGENTCAT_TAG_SESSION_SOURCE, + MCP_INSTRUCTIONS_KEY, +) + +from ..test_utils.community_catalog_server import ( + BOOM_TEXT, + HAS_CATALOG_TRANSFORM, + HAS_COMMUNITY_NESTING, + create_catalog_meta_server, + create_composing_server, +) +from ..test_utils.community_client import ( + HAS_COMMUNITY_CLIENT, + create_community_test_client, +) +from ..test_utils.community_todo_server import ( + HAS_COMMUNITY_FASTMCP, + create_community_todo_server, +) +from ..test_utils.flavors import tracking_data + +pytestmark = pytest.mark.skipif( + not (HAS_COMMUNITY_FASTMCP and HAS_COMMUNITY_CLIENT and HAS_COMMUNITY_NESTING), + reason="Community FastMCP not available", +) + +# Only the hidden-catalog shape needs `CatalogTransform` (mid-3.x); the +# composing-server tests below run on every release the community extra +# allows, so the daily version sweep keeps its nested-call coverage there. +catalog = pytest.mark.skipif( + not HAS_CATALOG_TRANSFORM, + reason="fastmcp CatalogTransform not available", +) + +MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +CONTEXT = "Driving the hidden catalog through the meta tool to exercise nesting" + + +@pytest.fixture(autouse=True) +def capture(monkeypatch): + """Collect every event the queue is handed, without touching the network.""" + events: list = [] + from agentcat.modules import event_queue + + monkeypatch.setattr(event_queue.event_queue, "add", events.append) + return events + + +def _text(result) -> str: + return "".join(c.text for c in result.content if hasattr(c, "text")) + + +def _call_events(capture) -> list: + return [e for e in capture if e.event_type == "mcp:tools/call"] + + +def _minted_from(result) -> str: + minted = _text(result).split("session_id=")[1].split(" ")[0] + assert minted.startswith("ses_") + return minted + + +@catalog +async def test_nested_listing_leaves_the_registries_alone(): + """The clobber itself: a mid-call catalog fetch must not replace the + registries the agent-facing listing built.""" + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + data = tracking_data(server) + assert set(data.injected_params_registry) == {"run"} + + await client.call_tool("run", {"program": "first", "context": CONTEXT}) + + # The nested get_tool_catalog listing DID happen... + assert set(observed["catalog"]) == {"echo", "compose", "get_more_tools"} + # ...and the agent-facing registries survived it untouched. + assert set(data.injected_params_registry) == {"run"} + assert set(data.output_injection_registry) == {"run"} + assert data.declared_session_params == set() + + +@catalog +async def test_the_echoed_session_id_still_strips_after_a_nested_call(capture): + """THE repro: list → call → echo the minted handle on the next call. + + Before the fix, the nested catalog fetch replaced the registry with the + raw backend catalog, nothing stripped `session_id`/`context` off the + second call, and FastMCP failed it with "Unexpected keyword argument". + """ + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + listed = await client.list_tools() + (run_tool,) = [t for t in listed if t.name == "run"] + assert {"session_id", "context"} <= set(run_tool.inputSchema["properties"]) + + r1 = await client.call_tool("run", {"program": "first", "context": CONTEXT}) + minted = _minted_from(r1) + + # A raise here is the regression: the strip must consume both params. + await client.call_tool( + "run", {"program": "second", "session_id": minted, "context": CONTEXT} + ) + + # The bodies received exactly their own arguments (community FastMCP would + # have raised on an unstripped extra, but pin the delivered shape anyway). + assert ("run", {"program": "second"}) in observed["delivered"] + outer = [e for e in _call_events(capture) if e.resource_name == "run"] + assert [e.session_id for e in outer] == [minted, minted] + assert outer[1].tags[AGENTCAT_TAG_SESSION_SOURCE] == "supplied" + + +@catalog +async def test_a_nested_call_joins_the_session_and_is_never_decorated(capture): + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool("run", {"program": "hello", "context": CONTEXT}) + + events = _call_events(capture) + assert [e.resource_name for e in events] == ["echo", "run"] + inner, outer = events + + # One logical call: the inner event rides the outer's session, keeps the + # outer's provenance, and carries the nested marker; the outer does not. + assert inner.session_id == outer.session_id + assert inner.tags[AGENTCAT_TAG_NESTED] == "true" + assert ( + inner.tags[AGENTCAT_TAG_SESSION_SOURCE] + == outer.tags[AGENTCAT_TAG_SESSION_SOURCE] + == "minted" + ) + assert AGENTCAT_TAG_NESTED not in outer.tags + + # The inner result, as agent-authored code would consume it, is the + # customer's data and nothing else: no mint-back block, no mirror key. + assert observed["inner_text"] == "echo:hello" + assert "[MCP INSTRUCTIONS]" not in observed["inner_text"] + assert observed["inner_structured"] == {"result": "echo:hello"} + + +@catalog +async def test_the_outer_call_is_still_fully_decorated(): + """The registry survives the nested fetch, so the outer mint-back keeps + both of its forms — the text block and the structured mirror (which the + clobber used to silently drop).""" + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + result = await client.call_tool("run", {"program": "hello", "context": CONTEXT}) + + assert MINT_BACK_HEADER in _text(result) + mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + assert mint["session_id"] == _minted_from(result) + assert result.structured_content["result"] == "ran:hello" + + +@catalog +async def test_the_nested_listing_is_served_uninjected(): + """The sandbox-facing catalog is the customer's raw view: parameters a + sandbox cannot echo must never appear in it.""" + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool("run", {"program": "hello", "context": CONTEXT}) + + catalog = observed["catalog"] + assert catalog["echo"] == ["text"] + assert catalog["compose"] == ["text"] + # get_more_tools' `context` is its own real parameter, not an injection. + assert catalog["get_more_tools"] == ["context"] + assert not any("session_id" in props for props in catalog.values()) + + +@catalog +async def test_another_server_is_untouched_by_this_servers_frame(): + """The frame is scoped by server identity: a listing on server B while a + call on server A is in flight still injects and still writes B's + registries.""" + observed: dict = {} + other = create_community_todo_server() + server = create_catalog_meta_server(observed, also_list=other.list_tools) + track(server, "proj_test", AgentCatOptions()) + track(other, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool("run", {"program": "hello", "context": CONTEXT}) + + add_todo = next(t for t in observed["also_listed"] if t.name == "add_todo") + assert {"session_id", "context"} <= set(add_todo.parameters["properties"]) + assert "add_todo" in tracking_data(other).injected_params_registry + + +@catalog +async def test_nesting_of_nesting_shares_the_outermost_session(capture): + observed: dict = {} + server = create_catalog_meta_server(observed, target="compose") + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool("run", {"program": "deep", "context": CONTEXT}) + + events = _call_events(capture) + assert [e.resource_name for e in events] == ["echo", "compose", "run"] + assert len({e.session_id for e in events}) == 1 + assert [AGENTCAT_TAG_NESTED in e.tags for e in events] == [True, True, False] + + +@catalog +async def test_a_failing_nested_call_publishes_and_surfaces(capture): + """The inner failure is recorded as a nested error event, and the raise + reaches the customer's tool body unchanged — analytics never rewrites the + failure a composing tool sees.""" + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + with pytest.raises(Exception, match="inner tool failed"): + await client.call_tool("run", {"program": BOOM_TEXT, "context": CONTEXT}) + + events = _call_events(capture) + assert [e.resource_name for e in events] == ["echo", "run"] + inner, outer = events + assert inner.is_error and inner.tags[AGENTCAT_TAG_NESTED] == "true" + assert inner.session_id == outer.session_id + assert outer.is_error + + +@catalog +async def test_tracing_off_still_shields_the_registry(): + """Context injection is independent of tracing, so the frame must cover + the tracing-off path too: a nested fetch there would otherwise eat the + registry entry that strips the next call's `context`.""" + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions(enable_tracing=False)) + + async with create_community_test_client(server) as client: + await client.list_tools() + data = tracking_data(server) + assert data.injected_params_registry["run"] == {"context"} + + await client.call_tool("run", {"program": "first", "context": CONTEXT}) + # The strip consumed `context` (a raise here would be the regression), + # and the registry still carries the entry for the next call. + await client.call_tool("run", {"program": "second", "context": CONTEXT}) + + assert data.injected_params_registry["run"] == {"context"} + assert ("run", {"program": "second"}) in observed["delivered"] + + +@catalog +async def test_sibling_calls_under_one_context_are_not_nested(capture): + """Restore-not-pop: a parent fastmcp Context can outlive one call, and the + sibling that follows must come up top-level — fresh session, no marker.""" + import fastmcp.server.context as fastmcp_context + + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + await server.list_tools() + async with fastmcp_context.Context(fastmcp=server): + await server.call_tool("run", {"program": "one", "context": CONTEXT}) + await server.call_tool("run", {"program": "two", "context": CONTEXT}) + + outer = [e for e in _call_events(capture) if e.resource_name == "run"] + assert len(outer) == 2 + assert outer[0].session_id != outer[1].session_id + assert all(AGENTCAT_TAG_NESTED not in e.tags for e in outer) + # Each run's inner echo still joined its OWN outer call's session. + inner = [e for e in _call_events(capture) if e.resource_name == "echo"] + assert [e.session_id for e in inner] == [e.session_id for e in outer] + + +# ── plain composing servers: nesting without any transform ────────────────── +# These run on EVERY community fastmcp release (no CatalogTransform guard): +# `ctx.fastmcp.call_tool` from a tool body is the ordinary nesting shape, and +# the daily version sweep must keep covering it on pre-transform releases. + + +async def test_a_composing_tool_is_nested_without_any_transform(capture): + """The frame does not depend on transforms: a listed tool calling a + sibling gets the same treatment, and per-call resolution (the actor) is + still resolved on the nested event rather than copied from the outer.""" + from agentcat.types import UserIdentity + + observed: dict = {} + server = create_composing_server(observed) + track( + server, + "proj_test", + AgentCatOptions( + identify=lambda request, extra: UserIdentity( + user_id="actor-1", user_name="Composer", user_data=None + ) + ), + ) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool( + "compose", + {"text": "hi", "context": "Composing one tool from another to test nesting"}, + ) + + events = _call_events(capture) + assert [e.resource_name for e in events] == ["echo", "compose"] + inner, outer = events + assert inner.session_id == outer.session_id + assert inner.tags[AGENTCAT_TAG_NESTED] == "true" + assert AGENTCAT_TAG_NESTED not in outer.tags + # Undecorated inner result, exactly as the composing body consumed it. + assert observed["inner_structured"] == {"result": "echo:hi"} + # Actor resolution stayed per-call: the nested event carries its own. + assert inner.identify_actor_given_id == "actor-1" + assert outer.identify_actor_given_id == "actor-1" + + +async def test_concurrent_inner_calls_all_join_the_outer_session(capture): + """Two inner calls running CONCURRENTLY (asyncio.gather in the tool body) + install and restore their frames in the shared request state in whatever + order they complete. Every one of them must still inherit the outer + session, and a later sequential call on the same server must come up + clean — no stale frame from the concurrent interleaving.""" + observed: dict = {} + server = create_composing_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + await client.call_tool( + "fanout", + {"a": "left", "b": "right", "context": "Fanning out two inner calls"}, + ) + await client.call_tool( + "compose", + {"text": "after", "context": "A sequential call after the fan-out"}, + ) + + events = _call_events(capture) + fan_outer = next(e for e in events if e.resource_name == "fanout") + fan_inner = [ + e + for e in events + if e.resource_name == "echo" and e.session_id == fan_outer.session_id + ] + assert len(fan_inner) == 2 + assert all(e.tags[AGENTCAT_TAG_NESTED] == "true" for e in fan_inner) + assert AGENTCAT_TAG_NESTED not in fan_outer.tags + # The sandbox-side results came back clean from both concurrent calls. + assert observed["fanned"] == [{"result": "echo:left"}, {"result": "echo:right"}] + + # The follow-up call is its own top-level call on a fresh session. + compose_outer = next(e for e in events if e.resource_name == "compose") + compose_inner = [ + e + for e in events + if e.resource_name == "echo" and e.session_id == compose_outer.session_id + ] + assert compose_outer.session_id != fan_outer.session_id + assert AGENTCAT_TAG_NESTED not in compose_outer.tags + assert len(compose_inner) == 1 diff --git a/tests/conftest.py b/tests/conftest.py index f00f51e..3f4191a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,6 +57,7 @@ # dereferencing middleware and real multi-round-trip results, none of which # exist on the 3.x line this suite's community/ directory covers. "test_community_v4_handles.py", + "test_community_v4_nested_calls.py", ) # Needs community FastMCP importable at all, independent of era. The diff --git a/tests/test_community_v4_nested_calls.py b/tests/test_community_v4_nested_calls.py new file mode 100644 index 0000000..dca22a7 --- /dev/null +++ b/tests/test_community_v4_nested_calls.py @@ -0,0 +1,109 @@ +"""Server-internal (nested) traffic on FastMCP 4. + +The v4 sibling of `tests/community/test_community_v3_nested_calls.py` — it +does not re-prove the v3 contract, only that the re-entrancy frame holds on +the 4.x line, whose `Context.__aenter__` shares `_request_state` the same way +but whose dispatch differs (second pass, default dereferencing middleware). +Covers the breakage repro, the nested-call session join, and the outer +decoration surviving a nested catalog fetch. +""" + +import pytest + +from agentcat import AgentCatOptions, track +from agentcat.modules.constants import ( + AGENTCAT_TAG_NESTED, + AGENTCAT_TAG_SESSION_SOURCE, + MCP_INSTRUCTIONS_KEY, +) + +from .test_utils.community_catalog_server import ( + HAS_CATALOG_TRANSFORM, + create_catalog_meta_server, +) +from .test_utils.community_client import ( + HAS_COMMUNITY_CLIENT, + create_community_test_client, +) +from .test_utils.community_todo_server import ( + HAS_COMMUNITY_FASTMCP, +) + +pytestmark = pytest.mark.skipif( + not (HAS_COMMUNITY_FASTMCP and HAS_COMMUNITY_CLIENT and HAS_CATALOG_TRANSFORM), + reason="Community FastMCP with CatalogTransform not available", +) + +MINT_BACK_HEADER = "[MCP INSTRUCTIONS]: session_id issued." +CONTEXT = "Driving the hidden catalog through the meta tool to exercise nesting" + + +@pytest.fixture(autouse=True) +def capture(monkeypatch): + """Collect every event the queue is handed, without touching the network.""" + events: list = [] + from agentcat.modules import event_queue + + monkeypatch.setattr(event_queue.event_queue, "add", events.append) + return events + + +def _text(result) -> str: + return "".join(c.text for c in result.content if hasattr(c, "text")) + + +def _call_events(capture) -> list: + return [e for e in capture if e.event_type == "mcp:tools/call"] + + +async def test_the_echoed_session_id_still_strips_after_a_nested_call(capture): + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + listed = await client.list_tools() + (run_tool,) = [t for t in listed if t.name == "run"] + assert {"session_id", "context"} <= set(run_tool.input_schema["properties"]) + + r1 = await client.call_tool("run", {"program": "first", "context": CONTEXT}) + minted = _text(r1).split("session_id=")[1].split(" ")[0] + assert minted.startswith("ses_") + + # A raise here is the regression: the nested catalog fetch inside call + # one must not have clobbered the registry that strips these. + await client.call_tool( + "run", {"program": "second", "session_id": minted, "context": CONTEXT} + ) + + outer = [e for e in _call_events(capture) if e.resource_name == "run"] + assert [e.session_id for e in outer] == [minted, minted] + assert outer[1].tags[AGENTCAT_TAG_SESSION_SOURCE] == "supplied" + + +async def test_a_nested_call_joins_the_session_and_is_never_decorated(capture): + observed: dict = {} + server = create_catalog_meta_server(observed) + track(server, "proj_test", AgentCatOptions()) + + async with create_community_test_client(server) as client: + await client.list_tools() + result = await client.call_tool("run", {"program": "hello", "context": CONTEXT}) + + events = _call_events(capture) + assert [e.resource_name for e in events] == ["echo", "run"] + inner, outer = events + assert inner.session_id == outer.session_id + assert inner.tags[AGENTCAT_TAG_NESTED] == "true" + assert AGENTCAT_TAG_NESTED not in outer.tags + + # The sandbox-side view is the customer's data and nothing else... + assert observed["inner_text"] == "echo:hello" + assert observed["inner_structured"] == {"result": "echo:hello"} + assert not any( + "session_id" in props for props in observed["catalog"].values() + ) + # ...while the outer wire result keeps both mint-back forms. + assert MINT_BACK_HEADER in _text(result) + mint = result.structured_content[MCP_INSTRUCTIONS_KEY] + assert mint["session_id"] == outer.session_id diff --git a/tests/test_handles.py b/tests/test_handles.py index b844ea3..75738d5 100644 --- a/tests/test_handles.py +++ b/tests/test_handles.py @@ -427,6 +427,16 @@ def test_tag_clamp(): assert build_handle_tags(HandleResolution("s", "minted")) == {"agentcat_session_id_source": "minted"} # noqa: E501 +# The nested marker is presence-gated like mrtr: "true" on a server-internal +# call, absent everywhere else — the default keeps every existing event's tag +# map byte-identical. +def test_tags_nested_marker_is_presence_gated(): + res = HandleResolution(sid("T"), "supplied") + assert build_handle_tags(res, nested=True)["agentcat_nested"] == "true" + assert "agentcat_nested" not in build_handle_tags(res) + assert "agentcat_nested" not in build_handle_tags(res, nested=False) + + # TS handles.test.ts:174-240 — the full tag map for a normal agent_id, and the # clamp/newline-strip applying to the tag copy only, never the resolution. def test_tags_pass_a_normal_agent_id_through(): diff --git a/tests/test_utils/community_catalog_server.py b/tests/test_utils/community_catalog_server.py new file mode 100644 index 0000000..250e59a --- /dev/null +++ b/tests/test_utils/community_catalog_server.py @@ -0,0 +1,180 @@ +"""A community FastMCP server whose catalog is a code-mode-shaped transform. + +Reproduces the mechanism of fastmcp's code mode WITHOUT pydantic-monty: a +``CatalogTransform`` subclass replaces the listing with one synthetic ``run`` +tool whose body — exactly like code mode's ``execute`` and discovery tools — +fetches the real catalog via ``get_tool_catalog(ctx)`` (a nested ``tools/list`` +with ``run_middleware=True``) and then drives a hidden backend tool via +``ctx.fastmcp.call_tool`` (a nested ``tools/call``). Everything the "sandbox" +observes — the catalog it was served, the arguments the bodies received, the +inner result as agent-authored code would see it — is recorded into the +``observed`` dict the factory takes, so tests can assert on the inside view as +well as the wire. + +Two guard flags, deliberately separate: ``CatalogTransform`` is imported from +its module path (not in ``fastmcp.server.transforms.__all__``) and first +shipped mid-3.x, so only the catalog-fetch factory needs it — the composing +factories below use nothing newer than ``ctx.fastmcp.call_tool`` and must keep +running on every fastmcp release the community extra allows. A single guard +would silently shrink the daily version sweep's nested-call coverage to the +transform-era releases. +""" + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from fastmcp import FastMCP + +try: + from fastmcp import FastMCP as CommunityFastMCP + from fastmcp.server.context import Context + + HAS_COMMUNITY_NESTING = True +except ImportError: + CommunityFastMCP = None # type: ignore + Context = None # type: ignore + HAS_COMMUNITY_NESTING = False + +try: + from fastmcp.server.transforms.catalog import CatalogTransform + from fastmcp.tools import Tool + + HAS_CATALOG_TRANSFORM = True +except ImportError: + CatalogTransform = object # type: ignore + Tool = None # type: ignore + HAS_CATALOG_TRANSFORM = False + + +# The text `echo` refuses, so error-path tests have one failure shape. +BOOM_TEXT = "boom" +META_TOOL_NAME = "run" + + +class InnerToolFailed(RuntimeError): + """Raised by the hidden `echo` on the sentinel text.""" + + +def create_composing_server( + observed: dict[str, Any], name: str = "composing" +) -> "FastMCP": + """A plain FastMCP server whose LISTED tools call each other — no + transform, no hidden catalog. This is the ordinary shape of nesting + (`ctx.fastmcp.call_tool` from a tool body), available on every community + fastmcp release, so the tests built on it run across the whole version + sweep. + + - ``compose(text)`` calls ``echo`` once: one level of nesting. + - ``fanout(a, b)`` runs two ``echo`` calls CONCURRENTLY via + ``asyncio.gather``: the inner frames install and restore in the shared + request state in completion order, which is exactly the interleaving + the re-entrancy machinery has to survive. + """ + import asyncio + + server = CommunityFastMCP(name) + + @server.tool + async def echo(text: str) -> str: + """Echo the text back.""" + observed.setdefault("delivered", []).append(("echo", {"text": text})) + if text == BOOM_TEXT: + raise InnerToolFailed("the inner tool failed") + return f"echo:{text}" + + @server.tool + async def compose(text: str, ctx: Context = None) -> str: # type: ignore[assignment] + """Call echo from inside another listed tool.""" + observed.setdefault("delivered", []).append(("compose", {"text": text})) + inner = await ctx.fastmcp.call_tool("echo", {"text": text}) + observed["inner_structured"] = inner.structured_content + return f"composed:{text}" + + @server.tool + async def fanout(a: str, b: str, ctx: Context = None) -> str: # type: ignore[assignment] + """Call echo twice, concurrently.""" + results = await asyncio.gather( + ctx.fastmcp.call_tool("echo", {"text": a}), + ctx.fastmcp.call_tool("echo", {"text": b}), + ) + observed["fanned"] = [r.structured_content for r in results] + return f"fanned:{a},{b}" + + return server + + +def create_catalog_meta_server( + observed: dict[str, Any], + name: str = "catalog-meta", + *, + target: str = "echo", + also_list: Any = None, +) -> "FastMCP": + """A FastMCP server serving only ``run``, over hidden ``echo``/``compose``. + + ``run(program)`` fetches the catalog, then calls ``target`` with the + program text — ``"echo"`` for one level of nesting, ``"compose"`` for two + (compose itself calls echo). ``also_list``, if given, is an async callable + the body awaits after the catalog fetch — the multi-server isolation tests + hand it another server's ``list_tools``. + """ + server = CommunityFastMCP(name) + + @server.tool + async def echo(text: str) -> str: + """Echo the text back.""" + observed.setdefault("delivered", []).append(("echo", {"text": text})) + if text == BOOM_TEXT: + raise InnerToolFailed("the inner tool failed") + return f"echo:{text}" + + @server.tool + async def compose(text: str, ctx: Context = None) -> str: # type: ignore[assignment] + """Call echo from inside another hidden tool (nesting of nesting).""" + observed.setdefault("delivered", []).append(("compose", {"text": text})) + inner = await ctx.fastmcp.call_tool("echo", {"text": text}) + return f"composed:{inner.structured_content}" + + class _MetaCatalog(CatalogTransform): + """The catalog collapsed to one meta tool, code-mode style.""" + + def __init__(self) -> None: + super().__init__() + self._meta_tool = self._make_meta_tool() + + def _make_meta_tool(self) -> Any: + transform = self + + async def run(program: str, ctx: Context = None) -> str: # type: ignore[assignment] + """Run a program against the hidden catalog.""" + observed.setdefault("delivered", []).append( + (META_TOOL_NAME, {"program": program}) + ) + catalog = await transform.get_tool_catalog(ctx) + observed["catalog"] = { + tool.name: sorted((tool.parameters or {}).get("properties", {})) + for tool in catalog + } + if also_list is not None: + observed["also_listed"] = list(await also_list()) + inner = await ctx.fastmcp.call_tool(target, {"text": program}) + observed["inner_text"] = "".join( + block.text + for block in inner.content + if hasattr(block, "text") + ) + observed["inner_structured"] = inner.structured_content + return f"ran:{program}" + + return Tool.from_function(fn=run, name=META_TOOL_NAME) + + async def transform_tools(self, tools: Any) -> Any: + return [self._meta_tool] + + async def get_tool(self, name: str, call_next: Any, **kwargs: Any) -> Any: + if name == META_TOOL_NAME: + return self._meta_tool + return await call_next(name, **kwargs) + + server.add_transform(_MetaCatalog()) + return server