test(tool-service): cover _looks_like_mcp_envelope layer-2 branches and BaseModel fallback

Five new tests in ``TestCoerceToToolResult`` close the coverage gaps
flagged by the tool_service.py coverage report (86.2% → higher on the
helper):

* ``test_accepts_dict_with_structured_content_marker_only`` — exercises
  the ``structuredContent`` positive-marker branch of
  ``_looks_like_mcp_envelope`` (was uncovered).
* ``test_accepts_dict_with_snake_case_structured_content_marker`` — the
  snake-case sibling; forwarded-worker responses use this form.
* ``test_accepts_dict_with_only_typed_content_items`` — the weakest
  positive marker (content items carrying a string ``type`` field);
  without this a future tightening could drop the branch silently.
* ``test_rejects_dict_with_only_meta_marker`` — the final ``return
  False`` branch when all keys are envelope-shaped but no positive
  marker fires. ``_meta`` alone is not sufficient.
* ``test_basemodel_with_content_that_fails_validation_falls_back`` —
  pins the ``except ValidationError`` branch inside
  ``_coerce_to_tool_result``'s BaseModel dispatch; asserts WARNING-level
  log with ``exc_info=True``.

While writing the BaseModel fallback test it surfaced a real bug: a
Pydantic ``BaseModel`` that failed ``ToolResult.model_validate`` and
fell through to the opaque-JSON fallback caused ``orjson.dumps`` to
raise ``TypeError`` — breaking the helper's "always returns a valid
ToolResult" invariant. Fixed by dumping BaseModel payloads via
``model_dump(mode="json", by_alias=True)`` before serialising, and
adding a last-resort ``TypeError`` catch that falls back to
``str(payload)`` with a WARNING log. ``test_non_json_serialisable_payload_falls_back_to_str``
pins the new branch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>

Author: jonpspri

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "^.secrets.baseline|package-lock.json|Cargo.lock|scripts/sign_image.sh|scripts/zap|sonar-project.properties|uv.lock|go.sum|mcpgateway/sri_hashes.json|^.secrets.baseline$",
     "lines": null
   },
-  "generated_at": "2026-04-14T23:02:21Z",
+  "generated_at": "2026-04-14T23:22:05Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -8666,95 +8666,95 @@
         "hashed_secret": "2dd2850bcc4ae4e74154aed99ee5f68292ecfec5",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 2731,
+        "line_number": 2887,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "d92342976d720ff38cf5dcb329be41959ab1ba6c",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 3418,
+        "line_number": 3574,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "f93640458964af294a485bc6d597694cf3308e1f",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 3427,
+        "line_number": 3583,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "0857abc2d90c5d9821229fc0a880f1c38ffb0e04",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 3818,
+        "line_number": 3974,
         "type": "Hex High Entropy String",
         "verified_result": null
       },
       {
         "hashed_secret": "99834bc4eff3f1e1c1e4692d2476b593b501d045",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6746,
+        "line_number": 6902,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "f2b14f68eb995facb3a1c35287b778d5bd785511",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6764,
+        "line_number": 6920,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "b8cec023ad982a1355abb9e7c7700e42d7e6fac3",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6788,
+        "line_number": 6944,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "0614eb27c6e0d2f4ed9a1cce2a5bcbbbc17aa556",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6852,
+        "line_number": 7008,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "a38fea79a043f0c9a62f851659d459dc3b716ea9",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6863,
+        "line_number": 7019,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "a50da1fb101595ac5158f2c9394a65a84061b56c",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6904,
+        "line_number": 7060,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "ed3e0017cb8e4b06a59af1a441f62cbe58d2ef59",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 6910,
+        "line_number": 7066,
         "type": "Secret Keyword",
         "verified_result": null
       },
       {
         "hashed_secret": "9bdc408babb4c5740aa9ee42e65b9308bd01f5f9",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 8033,
+        "line_number": 8189,
         "type": "Secret Keyword",
         "verified_result": null
       }

mcpgateway/services/tool_service.py

@@ -1678,7 +1678,23 @@ def _coerce_to_tool_result(self, payload: Any) -> ToolResult:
         # indicates an upstream whose shape the heuristic ought to learn
         # to recognise).
         logger.debug("Coercing %s payload to opaque text content", type(payload).__name__)
-        serialized = orjson.dumps(payload, option=orjson.OPT_INDENT_2)
+        # orjson cannot natively serialise arbitrary Pydantic ``BaseModel``
+        # instances. A BaseModel reaching this branch means it either
+        # failed the earlier ``ToolResult.model_validate`` step *or* did
+        # not expose a ``content`` attribute at all — dump it first so
+        # we don't raise ``TypeError`` and break the "always returns a
+        # valid ``ToolResult``" invariant.
+        serializable_payload = payload.model_dump(mode="json", by_alias=True) if isinstance(payload, BaseModel) else payload
+        try:
+            serialized = orjson.dumps(serializable_payload, option=orjson.OPT_INDENT_2)
+        except TypeError:
+            # Very last resort — something non-JSON-serialisable slipped
+            # through (e.g. ``set``, datetime without isoformat, a custom
+            # object without ``__json__``). Fall back to ``str(payload)``
+            # so the caller still sees *something* and the pipeline stays
+            # on contract.
+            logger.warning("Payload of type %s is not JSON-serialisable; using str() fallback", type(payload).__name__, exc_info=True)
+            return ToolResult(content=[TextContent(type="text", text=str(payload))])
         return ToolResult(content=[TextContent(type="text", text=serialized.decode())])
 
     async def register_tool(

tests/unit/mcpgateway/services/test_tool_service_coverage.py

@@ -2247,6 +2247,162 @@ def test_accepts_dict_with_mcp_markers(self, tool_service):
         assert result.structured_content == {"recognitionId": "rec-1"}
         assert result.content[0].text == "ok"
 
+    def test_accepts_dict_with_structured_content_marker_only(self, tool_service):
+        """Dict with ``structuredContent`` (camelCase) but no ``isError`` is an envelope.
+
+        Covers the ``structuredContent`` branch of ``_looks_like_mcp_envelope``'s
+        layer-2 positive-marker check (search the helper for ``"structuredContent" in keys"``).
+        This exercises the camelCase wire form; the snake-case sibling is
+        covered by the next test.
+        """
+        # No ``isError`` — so the heuristic must admit via structuredContent.
+        payload = {
+            "content": [{"type": "text", "text": "ok"}],
+            "structuredContent": {"recognitionId": "rec-42"},
+        }
+
+        result = tool_service._coerce_to_tool_result(payload)
+
+        assert isinstance(result, ToolResult)
+        assert result.structured_content == {"recognitionId": "rec-42"}
+        assert result.content[0].text == "ok"
+        # ToolResult.is_error defaults to False when unset in the payload.
+        assert result.is_error is False
+
+    def test_accepts_dict_with_snake_case_structured_content_marker(self, tool_service):
+        """Dict with ``structured_content`` (snake_case, gateway-internal) is an envelope.
+
+        Covers the ``structured_content`` half of the layer-2 check. Both
+        spellings live in ``_MCP_ENVELOPE_KEYS`` because forwarded-worker
+        responses and internal serialisations may emit either form.
+        """
+        payload = {
+            "content": [{"type": "text", "text": "ok"}],
+            "structured_content": {"recognitionId": "rec-99"},
+        }
+
+        result = tool_service._coerce_to_tool_result(payload)
+
+        assert isinstance(result, ToolResult)
+        assert result.structured_content == {"recognitionId": "rec-99"}
+
+    def test_accepts_dict_with_only_typed_content_items(self, tool_service):
+        """Dict whose only MCP signal is typed ``content`` items is still admitted.
+
+        Covers the weakest positive marker in ``_looks_like_mcp_envelope`` —
+        ``content`` as a non-empty list of dicts each carrying a string
+        ``type`` field (MCP ``ContentBlock`` shape). No ``isError``,
+        no ``structuredContent``, no ``_meta`` — this test exists so a
+        future tightening that accidentally drops the typed-content branch
+        is caught.
+        """
+        payload = {
+            "content": [
+                {"type": "text", "text": "line 1"},
+                {"type": "text", "text": "line 2"},
+            ],
+        }
+
+        result = tool_service._coerce_to_tool_result(payload)
+
+        assert isinstance(result, ToolResult)
+        assert len(result.content) == 2
+        assert result.content[0].text == "line 1"
+        assert result.content[1].text == "line 2"
+
+    def test_rejects_dict_with_only_meta_marker(self, tool_service):
+        """Dict with only ``_meta`` and no positive signal is not an MCP envelope.
+
+        Covers the final ``return False`` branch of
+        ``_looks_like_mcp_envelope`` — all keys are in the envelope set
+        (layer 1 passes) but no positive marker fires (``isError`` /
+        ``structuredContent`` / typed content items). ``_meta`` alone
+        doesn't make a payload MCP-shaped. The payload is therefore
+        coerced via the opaque-JSON fallback so the caller can inspect
+        the ``_meta`` contents without them being reinterpreted as
+        protocol metadata.
+        """
+        payload = {"_meta": {"trace_id": "abc"}, "content": []}
+
+        result = tool_service._coerce_to_tool_result(payload)
+
+        # Falls through to text-serialisation; ``_meta`` preserved as data.
+        assert isinstance(result, ToolResult)
+        assert result.content[0].type == "text"
+        roundtripped = orjson.loads(result.content[0].text)
+        assert roundtripped == payload
+
+    def test_non_json_serialisable_payload_falls_back_to_str(self, tool_service, caplog):
+        """``orjson.dumps`` TypeError is caught and we fall back to ``str(payload)``.
+
+        ``_coerce_to_tool_result``'s contract is to always return a valid
+        ``ToolResult``, even for payloads that aren't natively JSON-
+        serialisable (e.g. sets, circular references, custom objects
+        without a ``__json__`` method). The last-resort ``TypeError``
+        fallback at the end of the helper catches the ``orjson.dumps``
+        failure, logs at WARNING with ``exc_info=True``, and wraps
+        ``str(payload)`` into a single ``TextContent``. Without this
+        guard the helper would raise and break the pipeline's
+        "always-produces-a-ToolResult" invariant.
+        """
+
+        class NotJSONSerialisable:
+            """Plain Python class with no JSON representation."""
+
+            def __repr__(self) -> str:
+                return "NotJSONSerialisable(opaque)"
+
+        payload = NotJSONSerialisable()
+
+        with caplog.at_level("WARNING", logger="mcpgateway.services.tool_service"):
+            result = tool_service._coerce_to_tool_result(payload)
+
+        assert isinstance(result, ToolResult)
+        assert result.content[0].type == "text"
+        # ``str(payload)`` should be visible in the fallback text.
+        assert "NotJSONSerialisable" in result.content[0].text
+        relevant = [r for r in caplog.records if "not JSON-serialisable" in r.message]
+        assert relevant, "Expected a WARNING-level log from the str() fallback"
+        assert relevant[-1].levelname == "WARNING"
+        assert relevant[-1].exc_info is not None
+
+    def test_basemodel_with_content_that_fails_validation_falls_back(self, tool_service, caplog):
+        """Pydantic ``BaseModel`` path that fails ``ToolResult`` validation falls back to text.
+
+        Covers the ``except ValidationError`` branch inside
+        ``_coerce_to_tool_result``'s BaseModel dispatch. A caller could
+        conceivably pass a non-MCP Pydantic model that happens to expose
+        a ``content`` attribute but whose ``model_dump(by_alias=True)``
+        produces a shape that ``ToolResult.model_validate`` refuses
+        (e.g. ``content`` is a scalar rather than a list).
+
+        The helper must not raise — it must log at WARNING (so operators
+        see the protocol/schema drift) and fall through to the opaque
+        JSON-serialisation path so the rest of the pipeline still gets
+        a valid ``ToolResult``.
+        """
+        # Third-Party
+        from pydantic import BaseModel  # pylint: disable=import-outside-toplevel
+
+        class BadUpstreamModel(BaseModel):
+            """Stand-in for a third-party model that has ``content`` but wrong shape."""
+
+            content: int = 42  # ``ToolResult.content`` requires ``list[ContentBlock]``
+
+        bad_model = BadUpstreamModel()
+
+        with caplog.at_level("WARNING", logger="mcpgateway.services.tool_service"):
+            result = tool_service._coerce_to_tool_result(bad_model)
+
+        assert isinstance(result, ToolResult)
+        assert result.content[0].type == "text"
+        # The WARNING-severity reshape log must have fired (see #4202 egress
+        # review round — user-visible reshapes promoted from DEBUG).
+        relevant = [r for r in caplog.records if "did not validate as ToolResult" in r.message]
+        assert relevant, "Expected a WARNING-level reshape log from the BaseModel ValidationError branch"
+        assert relevant[-1].levelname == "WARNING"
+        assert relevant[-1].exc_info is not None, "Expected exc_info=True for post-mortem diagnosis"
+
     def test_validation_error_falls_back_to_text_serialization(self, tool_service, caplog):
         """Dicts admitted by the heuristic but rejected by Pydantic fall back to text.