From 43a3619962221d4418198127ff06834e0ceea80c Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Fri, 23 Oct 2026 15:27:17 -0400 Subject: [PATCH 1/6] [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 --- src/components/customizers.py | 27 ++++++++++++++++++++ tests/conftest.py | 1 + tests/test_components.py | 47 +++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/src/components/customizers.py b/src/components/customizers.py index 33ee728..98ff0dc 100644 --- a/src/components/customizers.py +++ b/src/components/customizers.py @@ -6,6 +6,7 @@ OpenAPITool, ) from fastmcp.utilities.openapi.models import HTTPRoute +from mcp.types import ToolAnnotations from ..utils.logging import get_logger @@ -13,6 +14,8 @@ ComponentType = OpenAPITool | OpenAPIResource | OpenAPIResourceTemplate +ACCESS_EXTENSION = "x-cortex-mcp-access" + def customize_components( route: HTTPRoute, component: ComponentType, @@ -51,6 +54,11 @@ def customize_components( # Hide this fully for now. component.output_schema = None + # Without a title, FastMCP derives one from the operationId: "Addcustomdataforentity". + if route.summary: + component.title = route.summary + component.annotations = _tool_annotations(route) + # SCHEMA REFERENCE HANDLING: # OpenAPI-generated MCP tool schemas contain $defs with nested $ref references # (e.g., $ref -> $defs -> $ref chains). @@ -100,6 +108,25 @@ def customize_components( logger.debug(f"Final tags: {component.tags}") +def _tool_annotations(route: HTTPRoute) -> ToolAnnotations: + """Hint whether a tool only reads, only adds, or may overwrite data. + + The x-cortex-mcp-access extension wins. Without it a GET reads and anything + else may be destructive, which is also what MCP clients assume. + """ + method = route.method.upper() + access = route.extensions.get(ACCESS_EXTENSION) or ( + "read" if method == "GET" else "destructive" + ) + if access == "read": + return ToolAnnotations(read_only_hint=True) + return ToolAnnotations( + read_only_hint=False, + destructive_hint=access != "additive", + idempotent_hint=method == "PUT", + ) + + def _format_tool_description(route: HTTPRoute, original: str) -> str: """Format description for tool components.""" method = route.method.upper() diff --git a/tests/conftest.py b/tests/conftest.py index eddab83..32cdd07 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ def sample_route(): route = Mock() route.path = "/api/v1/resources" route.method = "GET" + route.summary = "List resources" route.tags = ["api", "resources"] route.extensions = {"x-cortex-mcp-enabled": "true"} return route diff --git a/tests/test_components.py b/tests/test_components.py index db5beb7..0901b0c 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -87,3 +87,50 @@ def test_preserve_original_description(self, sample_route): assert original in tool.description assert tool.description != original + + +class TestToolAnnotations: + """Hints that let clients approve read tools apart from write tools.""" + + @staticmethod + def _tool(route): + tool = Mock(spec=OpenAPITool) + tool.description = "" + tool.tags = set() + customize_components(route, tool) + return tool + + def test_get_is_read_only(self, sample_route): + assert self._tool(sample_route).annotations.read_only_hint is True + + def test_other_methods_default_to_destructive(self, sample_route): + sample_route.method = "POST" + + annotations = self._tool(sample_route).annotations + + assert annotations.read_only_hint is False + assert annotations.destructive_hint is True + assert annotations.idempotent_hint is False + + def test_put_is_idempotent(self, sample_route): + sample_route.method = "PUT" + + assert self._tool(sample_route).annotations.idempotent_hint is True + + def test_access_extension_marks_a_post_read_only(self, sample_route): + sample_route.method = "POST" + sample_route.extensions = {"x-cortex-mcp-access": "read"} + + assert self._tool(sample_route).annotations.read_only_hint is True + + def test_access_extension_marks_a_write_additive(self, sample_route): + sample_route.method = "POST" + sample_route.extensions = {"x-cortex-mcp-access": "additive"} + + annotations = self._tool(sample_route).annotations + + assert annotations.read_only_hint is False + assert annotations.destructive_hint is False + + def test_title_is_the_summary(self, sample_route): + assert self._tool(sample_route).title == "List resources" From a729bb0be77a6d4b9e35f17d82c90524d5fc3e8f Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Mon, 26 Oct 2026 13:12:46 -0400 Subject: [PATCH 2/6] [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 --- src/components/customizers.py | 17 ++++++++++----- swagger.json | 2 ++ tests/test_components.py | 41 ++++++++++++++++++++++++++++------- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/components/customizers.py b/src/components/customizers.py index 98ff0dc..c05e326 100644 --- a/src/components/customizers.py +++ b/src/components/customizers.py @@ -14,8 +14,6 @@ ComponentType = OpenAPITool | OpenAPIResource | OpenAPIResourceTemplate -ACCESS_EXTENSION = "x-cortex-mcp-access" - def customize_components( route: HTTPRoute, component: ComponentType, @@ -111,13 +109,20 @@ def customize_components( def _tool_annotations(route: HTTPRoute) -> ToolAnnotations: """Hint whether a tool only reads, only adds, or may overwrite data. - The x-cortex-mcp-access extension wins. Without it a GET reads and anything + The x-cortex-mcp-access extension ("read", "additive" or "destructive") wins, + and any other value counts as destructive. Without it a GET reads and anything else may be destructive, which is also what MCP clients assume. """ method = route.method.upper() - access = route.extensions.get(ACCESS_EXTENSION) or ( - "read" if method == "GET" else "destructive" - ) + access = route.extensions.get("x-cortex-mcp-access") + if access is None: + access = "read" if method == "GET" else "destructive" + elif access not in ("read", "additive", "destructive"): + logger.warning( + f"Unknown x-cortex-mcp-access {access!r} on {method} {route.path}, " + "treating it as destructive" + ) + access = "destructive" if access == "read": return ToolAnnotations(read_only_hint=True) return ToolAnnotations( diff --git a/swagger.json b/swagger.json index 5c05d7d..beb3322 100644 --- a/swagger.json +++ b/swagger.json @@ -9611,6 +9611,7 @@ "tags": [ "Eng Intel: 2-Metrics" ], + "x-cortex-mcp-access": "read", "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)", "x-cortex-mcp-enabled": "true", "x-internal": true @@ -12410,6 +12411,7 @@ "tags": [ "My Workspace" ], + "x-cortex-mcp-access": "read", "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 ", "x-cortex-mcp-enabled": "true", "x-internal": true diff --git a/tests/test_components.py b/tests/test_components.py index 0901b0c..4726727 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -1,6 +1,7 @@ """Tests for component customization.""" from unittest.mock import Mock +import pytest from fastmcp.server.providers.openapi import ( OpenAPIResource, OpenAPIResourceTemplate, @@ -101,29 +102,34 @@ def _tool(route): return tool def test_get_is_read_only(self, sample_route): + """Test that a GET without x-cortex-mcp-access is read-only.""" assert self._tool(sample_route).annotations.read_only_hint is True - def test_other_methods_default_to_destructive(self, sample_route): - sample_route.method = "POST" + @pytest.mark.parametrize( + ("method", "idempotent"), + [("POST", False), ("PUT", True), ("PATCH", False), ("DELETE", False)], + ) + def test_other_methods_default_to_destructive( + self, sample_route, method, idempotent + ): + """Test that other methods without x-cortex-mcp-access are destructive.""" + sample_route.method = method annotations = self._tool(sample_route).annotations assert annotations.read_only_hint is False assert annotations.destructive_hint is True - assert annotations.idempotent_hint is False - - def test_put_is_idempotent(self, sample_route): - sample_route.method = "PUT" - - assert self._tool(sample_route).annotations.idempotent_hint is True + assert annotations.idempotent_hint is idempotent def test_access_extension_marks_a_post_read_only(self, sample_route): + """Test that x-cortex-mcp-access: read makes a POST read-only.""" sample_route.method = "POST" sample_route.extensions = {"x-cortex-mcp-access": "read"} assert self._tool(sample_route).annotations.read_only_hint is True def test_access_extension_marks_a_write_additive(self, sample_route): + """Test that x-cortex-mcp-access: additive is a non-destructive write.""" sample_route.method = "POST" sample_route.extensions = {"x-cortex-mcp-access": "additive"} @@ -132,5 +138,24 @@ def test_access_extension_marks_a_write_additive(self, sample_route): assert annotations.read_only_hint is False assert annotations.destructive_hint is False + def test_access_extension_marks_a_get_destructive(self, sample_route): + """Test that x-cortex-mcp-access: destructive overrides the GET default.""" + sample_route.extensions = {"x-cortex-mcp-access": "destructive"} + + annotations = self._tool(sample_route).annotations + + assert annotations.read_only_hint is False + assert annotations.destructive_hint is True + + def test_unknown_access_is_destructive(self, sample_route): + """Test that an unknown x-cortex-mcp-access value counts as destructive.""" + sample_route.extensions = {"x-cortex-mcp-access": "Read"} + + annotations = self._tool(sample_route).annotations + + assert annotations.read_only_hint is False + assert annotations.destructive_hint is True + def test_title_is_the_summary(self, sample_route): + """Test that the tool title is the operation summary.""" assert self._tool(sample_route).title == "List resources" From 5e0f5ac0ff33521475b568b21343ce11625b3ec8 Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Mon, 26 Oct 2026 13:17:30 -0400 Subject: [PATCH 3/6] [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 --- tests/test_components.py | 5 +++-- tests/test_spec.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 tests/test_spec.py diff --git a/tests/test_components.py b/tests/test_components.py index 4726727..65c4193 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -147,14 +147,15 @@ def test_access_extension_marks_a_get_destructive(self, sample_route): assert annotations.read_only_hint is False assert annotations.destructive_hint is True - def test_unknown_access_is_destructive(self, sample_route): - """Test that an unknown x-cortex-mcp-access value counts as destructive.""" + def test_unknown_access_is_destructive(self, sample_route, caplog): + """Test that an unknown x-cortex-mcp-access value warns and is destructive.""" sample_route.extensions = {"x-cortex-mcp-access": "Read"} annotations = self._tool(sample_route).annotations assert annotations.read_only_hint is False assert annotations.destructive_hint is True + assert "Unknown x-cortex-mcp-access 'Read'" in caplog.text def test_title_is_the_summary(self, sample_route): """Test that the tool title is the operation summary.""" diff --git a/tests/test_spec.py b/tests/test_spec.py new file mode 100644 index 0000000..67e0193 --- /dev/null +++ b/tests/test_spec.py @@ -0,0 +1,28 @@ +"""Tests for the bundled OpenAPI spec.""" +import json +from pathlib import Path + +SPEC_PATH = Path(__file__).parent.parent / "swagger.json" + + +class TestBundledSpec: + """Test suite for the swagger.json the server ships with.""" + + def test_mcp_writes_declare_access(self): + """Test that every MCP-enabled non-GET operation sets x-cortex-mcp-access. + + Without it the tool is published as destructive, even if it only reads. + """ + spec = json.loads(SPEC_PATH.read_text()) + + undeclared = [ + f"{method.upper()} {path}" + for path, item in spec["paths"].items() + for method, operation in item.items() + if isinstance(operation, dict) + and method != "get" + and operation.get("x-cortex-mcp-enabled") == "true" + and "x-cortex-mcp-access" not in operation + ] + + assert undeclared == [] From 4ba7275e796a8efc2588ed2ea198c0a5451078d0 Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Mon, 26 Oct 2026 13:21:48 -0400 Subject: [PATCH 4/6] [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 --- src/components/customizers.py | 4 +++- tests/test_components.py | 2 ++ tests/test_spec.py | 20 +++++++++++++------- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/components/customizers.py b/src/components/customizers.py index c05e326..cf4f4a6 100644 --- a/src/components/customizers.py +++ b/src/components/customizers.py @@ -14,6 +14,8 @@ ComponentType = OpenAPITool | OpenAPIResource | OpenAPIResourceTemplate +ACCESS_VALUES = ("read", "additive", "destructive") + def customize_components( route: HTTPRoute, component: ComponentType, @@ -117,7 +119,7 @@ def _tool_annotations(route: HTTPRoute) -> ToolAnnotations: access = route.extensions.get("x-cortex-mcp-access") if access is None: access = "read" if method == "GET" else "destructive" - elif access not in ("read", "additive", "destructive"): + elif access not in ACCESS_VALUES: logger.warning( f"Unknown x-cortex-mcp-access {access!r} on {method} {route.path}, " "treating it as destructive" diff --git a/tests/test_components.py b/tests/test_components.py index 65c4193..170be4a 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -1,4 +1,5 @@ """Tests for component customization.""" +import logging from unittest.mock import Mock import pytest @@ -149,6 +150,7 @@ def test_access_extension_marks_a_get_destructive(self, sample_route): def test_unknown_access_is_destructive(self, sample_route, caplog): """Test that an unknown x-cortex-mcp-access value warns and is destructive.""" + caplog.set_level(logging.WARNING, logger="src.components.customizers") sample_route.extensions = {"x-cortex-mcp-access": "Read"} annotations = self._tool(sample_route).annotations diff --git a/tests/test_spec.py b/tests/test_spec.py index 67e0193..511729f 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -2,27 +2,33 @@ import json from pathlib import Path +from src.components.customizers import ACCESS_VALUES + SPEC_PATH = Path(__file__).parent.parent / "swagger.json" class TestBundledSpec: """Test suite for the swagger.json the server ships with.""" - def test_mcp_writes_declare_access(self): - """Test that every MCP-enabled non-GET operation sets x-cortex-mcp-access. + def test_mcp_tools_declare_valid_access(self): + """Test that MCP-enabled operations set a valid x-cortex-mcp-access. - Without it the tool is published as destructive, even if it only reads. + Every operation other than a GET must set it. Without it, or with an + unknown value, the tool is published as destructive even if it only reads. """ spec = json.loads(SPEC_PATH.read_text()) - undeclared = [ + invalid = [ f"{method.upper()} {path}" for path, item in spec["paths"].items() for method, operation in item.items() if isinstance(operation, dict) - and method != "get" and operation.get("x-cortex-mcp-enabled") == "true" - and "x-cortex-mcp-access" not in operation + and (method != "get" or "x-cortex-mcp-access" in operation) + and operation.get("x-cortex-mcp-access") not in ACCESS_VALUES ] - assert undeclared == [] + assert not invalid, ( + f"Set x-cortex-mcp-access to one of {', '.join(ACCESS_VALUES)} on: " + f"{', '.join(invalid)}" + ) From ecf10509dca1e3210b5a04c1c0bacb7067c4c4a7 Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Tue, 27 Oct 2026 11:43:53 -0400 Subject: [PATCH 5/6] [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 --- src/components/customizers.py | 1 - tests/test_components.py | 11 +++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/components/customizers.py b/src/components/customizers.py index cf4f4a6..0867de3 100644 --- a/src/components/customizers.py +++ b/src/components/customizers.py @@ -130,7 +130,6 @@ def _tool_annotations(route: HTTPRoute) -> ToolAnnotations: return ToolAnnotations( read_only_hint=False, destructive_hint=access != "additive", - idempotent_hint=method == "PUT", ) diff --git a/tests/test_components.py b/tests/test_components.py index 170be4a..5531209 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -106,13 +106,8 @@ def test_get_is_read_only(self, sample_route): """Test that a GET without x-cortex-mcp-access is read-only.""" assert self._tool(sample_route).annotations.read_only_hint is True - @pytest.mark.parametrize( - ("method", "idempotent"), - [("POST", False), ("PUT", True), ("PATCH", False), ("DELETE", False)], - ) - def test_other_methods_default_to_destructive( - self, sample_route, method, idempotent - ): + @pytest.mark.parametrize("method", ["POST", "PUT", "PATCH", "DELETE"]) + def test_other_methods_default_to_destructive(self, sample_route, method): """Test that other methods without x-cortex-mcp-access are destructive.""" sample_route.method = method @@ -120,7 +115,7 @@ def test_other_methods_default_to_destructive( assert annotations.read_only_hint is False assert annotations.destructive_hint is True - assert annotations.idempotent_hint is idempotent + assert annotations.idempotent_hint is None def test_access_extension_marks_a_post_read_only(self, sample_route): """Test that x-cortex-mcp-access: read makes a POST read-only.""" From 0d7c482bcc39423e4b6eebffc113b28ea01c2243 Mon Sep 17 00:00:00 2001 From: Bruce Szudera Wienand Date: Tue, 27 Oct 2026 12:42:51 -0400 Subject: [PATCH 6/6] [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 --- src/components/customizers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/customizers.py b/src/components/customizers.py index 0867de3..31e6d1d 100644 --- a/src/components/customizers.py +++ b/src/components/customizers.py @@ -125,11 +125,12 @@ def _tool_annotations(route: HTTPRoute) -> ToolAnnotations: "treating it as destructive" ) access = "destructive" - if access == "read": - return ToolAnnotations(read_only_hint=True) + read_only = access == "read" + # MCP defines destructiveHint only for tools that aren't read-only. + destructive = None if read_only else access == "destructive" return ToolAnnotations( - read_only_hint=False, - destructive_hint=access != "additive", + read_only_hint=read_only, + destructive_hint=destructive, )