Skip to content

Commit 1163025

Browse files
bruceswclaude
andauthored
[PFR-4635] Tell MCP clients which tools only read (#20)
* [PFR-4635] Tell MCP clients which tools only read Each OpenAPI tool now carries ToolAnnotations and a title from the operation summary. brain-backend's x-cortex-mcp-access extension sets the hints; without it a GET is read-only and any other method is a destructive write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [PFR-4635] Mark the bundled spec's POST reads and warn on unknown access getMyWorkspace and queryPointInTimeMetrics only read, but as POSTs without x-cortex-mcp-access they were published as destructive. The bundled swagger.json now declares them read. An unknown x-cortex-mcp-access value still counts as destructive, and now logs a warning so a typo in the spec is visible. Tests cover that, GET overridden to destructive, and the default for every write method. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [PFR-4635] Require MCP writes in the bundled spec to declare their access A non-GET tool without x-cortex-mcp-access is published as destructive, so a read POST that loses the key in a spec refresh would quietly start asking for approval. The new test fails for any MCP-enabled non-GET operation in swagger.json that doesn't declare it. The unknown-value test now also checks the warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [PFR-4635] Check x-cortex-mcp-access values in the bundled spec A typo such as "readonly" passed the spec test but made the tool destructive at runtime. The test now checks the value against the same ACCESS_VALUES the customizer uses, on GETs too, and says what to set. The warning test pins its logger's level so it doesn't depend on the root logger. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [PFR-4635] Leave idempotentHint unset A PUT is idempotent by HTTP semantics, but not every Cortex PUT has been checked, and a client may retry a tool marked idempotent. Unset, the hint keeps the MCP default of false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [PFR-4635] Build the tool annotations from booleans The read-only and destructive hints are now worked out once and passed to a single ToolAnnotations. The hints sent to clients are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 2425107 commit 1163025

5 files changed

Lines changed: 141 additions & 0 deletions

File tree

‎src/components/customizers.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
OpenAPITool,
77
)
88
from fastmcp.utilities.openapi.models import HTTPRoute
9+
from mcp.types import ToolAnnotations
910

1011
from ..utils.logging import get_logger
1112

1213
logger = get_logger(__name__)
1314

1415
ComponentType = OpenAPITool | OpenAPIResource | OpenAPIResourceTemplate
1516

17+
ACCESS_VALUES = ("read", "additive", "destructive")
18+
1619
def customize_components(
1720
route: HTTPRoute,
1821
component: ComponentType,
@@ -51,6 +54,11 @@ def customize_components(
5154
# Hide this fully for now.
5255
component.output_schema = None
5356

57+
# Without a title, FastMCP derives one from the operationId: "Addcustomdataforentity".
58+
if route.summary:
59+
component.title = route.summary
60+
component.annotations = _tool_annotations(route)
61+
5462
# SCHEMA REFERENCE HANDLING:
5563
# OpenAPI-generated MCP tool schemas contain $defs with nested $ref references
5664
# (e.g., $ref -> $defs -> $ref chains).
@@ -100,6 +108,32 @@ def customize_components(
100108
logger.debug(f"Final tags: {component.tags}")
101109

102110

111+
def _tool_annotations(route: HTTPRoute) -> ToolAnnotations:
112+
"""Hint whether a tool only reads, only adds, or may overwrite data.
113+
114+
The x-cortex-mcp-access extension ("read", "additive" or "destructive") wins,
115+
and any other value counts as destructive. Without it a GET reads and anything
116+
else may be destructive, which is also what MCP clients assume.
117+
"""
118+
method = route.method.upper()
119+
access = route.extensions.get("x-cortex-mcp-access")
120+
if access is None:
121+
access = "read" if method == "GET" else "destructive"
122+
elif access not in ACCESS_VALUES:
123+
logger.warning(
124+
f"Unknown x-cortex-mcp-access {access!r} on {method} {route.path}, "
125+
"treating it as destructive"
126+
)
127+
access = "destructive"
128+
read_only = access == "read"
129+
# MCP defines destructiveHint only for tools that aren't read-only.
130+
destructive = None if read_only else access == "destructive"
131+
return ToolAnnotations(
132+
read_only_hint=read_only,
133+
destructive_hint=destructive,
134+
)
135+
136+
103137
def _format_tool_description(route: HTTPRoute, original: str) -> str:
104138
"""Format description for tool components."""
105139
method = route.method.upper()

‎swagger.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9611,6 +9611,7 @@
96119611
"tags": [
96129612
"Eng Intel: 2-Metrics"
96139613
],
9614+
"x-cortex-mcp-access": "read",
96149615
"x-cortex-mcp-description": "Execute point-in-time queries for one or more engineering metrics.\n\n Returns current metric values for specified time periods, with support for batch queries\n and optional period-over-period comparisons. Time range (startTime/endTime) cannot exceed 6 months (180 days).\n\n PREREQUISITES - Follow this workflow:\n 1. Discover all available metrics ONCE: Call listMetricDefinitions (view\u003d\u0027basic\u0027) - cache this response\n 2. Get metric query metadata ONCE per metric: Call listMetricDefinitions (view\u003d\u0027full\u0027, key\u003dMETRIC_KEY)\n - supportedAggregations: Valid aggregation methods\n - orderByAttribute: Attribute path for sorting by metric values\n - groupByOptions[].key: Valid groupBy keys (use exact values, do NOT guess)\n - filterOptions[].key: Valid filter keys (use exact values, do NOT guess)\n Cache the full view response for each metric. Reuse the metadata from cached responses for subsequent queries on the same metric.\n 3. Construct query: Use the query metadata from the full view responses in step 2 to build valid point-in-time requests\n\n IMPORTANT: Cache only results from listMetricDefinitions. Do NOT cache point-in-time query results - always execute fresh queries for current data.\n Only refresh cached listMetricDefinitions responses if no longer in your context window or explicitly requested.\n Do NOT guess attribute names - always use exact values from listMetricDefinitions responses.\n\n Response includes:\n - Lightweight metadata: Column definitions optimized for programmatic use\n - Row data: Actual metric values and dimensional data\n - No heavy schemas: Source definitions excluded (get from listMetricDefinitions instead)\n\n Error responses:\n - 400: Invalid metric names, date range, validation errors, or unsupported metric combinations\n - 403: Feature not enabled (contact help@cortex.io)",
96159616
"x-cortex-mcp-enabled": "true",
96169617
"x-internal": true
@@ -12410,6 +12411,7 @@
1241012411
"tags": [
1241112412
"My Workspace"
1241212413
],
12414+
"x-cortex-mcp-access": "read",
1241312415
"x-cortex-mcp-description": "TOOL for retrieving current user\u0027s owned resources and work items across the Cortex workspace.\n\n FLEXIBLE REQUEST STRUCTURE:\n The request accepts an object with optional fields for each resource type:\n - myEntitiesRequest: Fetch entities (services, resources, domains) owned by the user\n - myTeamsRequest: Fetch teams the user belongs to\n - myScorecardsRequest: Fetch scorecards associated with user\u0027s entities\n - myOpenPRsRequest: Fetch user\u0027s open pull requests across all Git repositories\n - myRequestedReviewsRequest: Fetch PRs where user is requested as a reviewer\n - myWorkItemsRequest: Fetch work items (Jira, Linear, Azure DevOps) assigned to the user\n\n Set one or more fields to fetch multiple resource types in a single call.\n All results are combined into a single response array with mixed item types.\n\n RESPONSE STRUCTURE:\n Returns a single array of items where each item has an \u0027itemType\u0027 field indicating its type:\n - \u0027entity\u0027: Catalog entities (services, resources, domains)\n - \u0027team\u0027: Teams the user belongs to\n - \u0027scorecard\u0027: Scorecards for user\u0027s entities\n - \u0027pull_request\u0027: Open pull requests authored by the user\n - \u0027review_request\u0027: Pull requests where user is requested as reviewer\n - \u0027work_item\u0027: Work items (issues/tickets) assigned to the user\n\n EXAMPLES:\n 1. Fetch only services:\n {\"myEntitiesRequest\": {\"entityTypes\": [\"service\"]}}\n\n 2. Fetch teams and their entities:\n {\"myTeamsRequest\": {\"includeMembers\": true}, \"myEntitiesRequest\": {}}\n\n 3. Get all work (PRs, reviews, Jira tickets):\n {\"myOpenPRsRequest\": {}, \"myRequestedReviewsRequest\": {}, \"myWorkItemsRequest\": {}}\n\n 4. Search entities with filters:\n {\"myEntitiesRequest\": {\"query\": \"payment\", \"ownershipSource\": \"DIRECT\", \"entityTypes\": [\"service\", \"resource\"]}}\n ",
1241412416
"x-cortex-mcp-enabled": "true",
1241512417
"x-internal": true

‎tests/conftest.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ def sample_route():
2727
route = Mock()
2828
route.path = "/api/v1/resources"
2929
route.method = "GET"
30+
route.summary = "List resources"
3031
route.tags = ["api", "resources"]
3132
route.extensions = {"x-cortex-mcp-enabled": "true"}
3233
return route

‎tests/test_components.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"""Tests for component customization."""
2+
import logging
23
from unittest.mock import Mock
34

5+
import pytest
46
from fastmcp.server.providers.openapi import (
57
OpenAPIResource,
68
OpenAPIResourceTemplate,
@@ -87,3 +89,71 @@ def test_preserve_original_description(self, sample_route):
8789

8890
assert original in tool.description
8991
assert tool.description != original
92+
93+
94+
class TestToolAnnotations:
95+
"""Hints that let clients approve read tools apart from write tools."""
96+
97+
@staticmethod
98+
def _tool(route):
99+
tool = Mock(spec=OpenAPITool)
100+
tool.description = ""
101+
tool.tags = set()
102+
customize_components(route, tool)
103+
return tool
104+
105+
def test_get_is_read_only(self, sample_route):
106+
"""Test that a GET without x-cortex-mcp-access is read-only."""
107+
assert self._tool(sample_route).annotations.read_only_hint is True
108+
109+
@pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"])
110+
def test_other_methods_default_to_destructive(self, sample_route, method):
111+
"""Test that other methods without x-cortex-mcp-access are destructive."""
112+
sample_route.method = method
113+
114+
annotations = self._tool(sample_route).annotations
115+
116+
assert annotations.read_only_hint is False
117+
assert annotations.destructive_hint is True
118+
assert annotations.idempotent_hint is None
119+
120+
def test_access_extension_marks_a_post_read_only(self, sample_route):
121+
"""Test that x-cortex-mcp-access: read makes a POST read-only."""
122+
sample_route.method = "POST"
123+
sample_route.extensions = {"x-cortex-mcp-access": "read"}
124+
125+
assert self._tool(sample_route).annotations.read_only_hint is True
126+
127+
def test_access_extension_marks_a_write_additive(self, sample_route):
128+
"""Test that x-cortex-mcp-access: additive is a non-destructive write."""
129+
sample_route.method = "POST"
130+
sample_route.extensions = {"x-cortex-mcp-access": "additive"}
131+
132+
annotations = self._tool(sample_route).annotations
133+
134+
assert annotations.read_only_hint is False
135+
assert annotations.destructive_hint is False
136+
137+
def test_access_extension_marks_a_get_destructive(self, sample_route):
138+
"""Test that x-cortex-mcp-access: destructive overrides the GET default."""
139+
sample_route.extensions = {"x-cortex-mcp-access": "destructive"}
140+
141+
annotations = self._tool(sample_route).annotations
142+
143+
assert annotations.read_only_hint is False
144+
assert annotations.destructive_hint is True
145+
146+
def test_unknown_access_is_destructive(self, sample_route, caplog):
147+
"""Test that an unknown x-cortex-mcp-access value warns and is destructive."""
148+
caplog.set_level(logging.WARNING, logger="src.components.customizers")
149+
sample_route.extensions = {"x-cortex-mcp-access": "Read"}
150+
151+
annotations = self._tool(sample_route).annotations
152+
153+
assert annotations.read_only_hint is False
154+
assert annotations.destructive_hint is True
155+
assert "Unknown x-cortex-mcp-access 'Read'" in caplog.text
156+
157+
def test_title_is_the_summary(self, sample_route):
158+
"""Test that the tool title is the operation summary."""
159+
assert self._tool(sample_route).title == "List resources"

‎tests/test_spec.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
"""Tests for the bundled OpenAPI spec."""
2+
import json
3+
from pathlib import Path
4+
5+
from src.components.customizers import ACCESS_VALUES
6+
7+
SPEC_PATH = Path(__file__).parent.parent / "swagger.json"
8+
9+
10+
class TestBundledSpec:
11+
"""Test suite for the swagger.json the server ships with."""
12+
13+
def test_mcp_tools_declare_valid_access(self):
14+
"""Test that MCP-enabled operations set a valid x-cortex-mcp-access.
15+
16+
Every operation other than a GET must set it. Without it, or with an
17+
unknown value, the tool is published as destructive even if it only reads.
18+
"""
19+
spec = json.loads(SPEC_PATH.read_text())
20+
21+
invalid = [
22+
f"{method.upper()} {path}"
23+
for path, item in spec["paths"].items()
24+
for method, operation in item.items()
25+
if isinstance(operation, dict)
26+
and operation.get("x-cortex-mcp-enabled") == "true"
27+
and (method != "get" or "x-cortex-mcp-access" in operation)
28+
and operation.get("x-cortex-mcp-access") not in ACCESS_VALUES
29+
]
30+
31+
assert not invalid, (
32+
f"Set x-cortex-mcp-access to one of {', '.join(ACCESS_VALUES)} on: "
33+
f"{', '.join(invalid)}"
34+
)

0 commit comments

Comments
 (0)