From 837f0c8da3c332ab21f58fbaa1e61d6c4cb3e7cf Mon Sep 17 00:00:00 2001 From: "druks-operator[bot]" <284423593+druks-operator[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 06:19:56 +0000 Subject: [PATCH 1/2] Derive extension-owned agent tool names in the MCP OpenAPI document The MCP layer now namespaces extension-owned agent operation ids itself: an unprefixed operation_id gains its owner's prefix during OpenAPI generation, while already-prefixed and platform-owned operations are untouched. Boot validation keeps requiring an explicit operation_id and a non-empty docstring, and drops the prefix demand. Co-Authored-By: Claude Opus 4.8 --- backend/druks/mcp/app.py | 59 ++++++++++++++++++++++------ backend/tests/test_mcp_endpoint.py | 63 +++++++++++++++++++++++++++++- docs/writing-an-extension.md | 14 ++++--- 3 files changed, 118 insertions(+), 18 deletions(-) diff --git a/backend/druks/mcp/app.py b/backend/druks/mcp/app.py index 18bde0f3..125d9651 100644 --- a/backend/druks/mcp/app.py +++ b/backend/druks/mcp/app.py @@ -77,25 +77,61 @@ def _validate_agent_tools(api: FastAPI) -> None: # The provider logs component-fn errors instead of raising, so derived tools # cannot refuse boot; validate the routes before derivation. Inclusion is # deferred (api.routes holds unresolved routers), so the contexts iterator - # is the one view with every route's merged tags — and the loader tags each - # extension route with its extension's name, so the tag names the owner. - extension_names = {extension.name for extension in iter_extensions()} - + # is the one view with every route's merged tags. Validation owns only the + # two demands the author owns — an explicit operation_id and a non-empty + # docstring; the extension prefix is the framework's to derive, not the + # author's to repeat (see _namespace_agent_operations). for route in iter_route_contexts(api.routes): if not isinstance(route.original_route, APIRoute) or "agent" not in route.tags: continue where = f"{'/'.join(sorted(route.methods or ()))} {route.path}" - operation_id = route.operation_id - if not operation_id: + if not route.operation_id: raise InvalidAgentToolError(where, "an explicit operation_id is required") if not inspect.getdoc(route.endpoint): raise InvalidAgentToolError(where, "a non-empty endpoint docstring is required") - extension = next((tag for tag in route.tags if tag in extension_names), None) - if extension and not operation_id.startswith(f"{extension}_"): - raise InvalidAgentToolError( - where, f"operation_id {operation_id!r} must start with {extension + '_'!r}" - ) + + +def _namespace_agent_operations(spec: dict, extension_names: set[str]) -> None: + # Derive each extension-owned agent operation's id to f"{extension}_{operation_id}", + # so the tool name the provider reads off the spec is namespaced without the + # author repeating the prefix. The loader tags every extension route with its + # extension's name, so among an agent operation's tags the one naming an + # installed extension is the owner; platform agent operations carry no such + # tag and keep their declared ids. An already-prefixed id passes through, so + # stable names like ship_start never double — and the namespace is what makes + # the merged document's operation ids globally unique. + for operations in spec.get("paths", {}).values(): + for operation in operations.values(): + if not isinstance(operation, dict) or "agent" not in operation.get("tags", []): + continue + operation_id = operation.get("operationId") + if not operation_id: + continue + extension = next((tag for tag in operation["tags"] if tag in extension_names), None) + if extension and not operation_id.startswith(f"{extension}_"): + operation["operationId"] = f"{extension}_{operation_id}" + + +def _install_agent_namespacing(api: FastAPI) -> None: + # The tool name comes from the spec's operation id, so the namespace must + # land on the document api.openapi() builds — not on FastAPI's cached, merged + # route contexts, which later generation silently discards. Wrap the app's + # own generator so the provider here and every later /openapi.json share one + # namespaced document: each fresh build (including after the openapi_schema + # reset below) re-derives the ids, and the pass-through check keeps a warm + # cache idempotent. + extension_names = {extension.name for extension in iter_extensions()} + generate = api.openapi + + def namespaced() -> dict: + if api.openapi_schema: + return api.openapi_schema + spec = generate() + _namespace_agent_operations(spec, extension_names) + return spec + + api.openapi = namespaced def _annotate(route: HTTPRoute, component: object) -> None: @@ -110,6 +146,7 @@ def _annotate(route: HTTPRoute, component: object) -> None: def create_mcp_app(api: FastAPI) -> StarletteWithLifespan: _validate_agent_tools(api) + _install_agent_namespacing(api) # Built directly rather than via from_fastapi, which owns the transport: # raise_app_exceptions=False makes an app crash reach the tool as the # app's sanitized 500, so no masking is needed and the taxonomy travels. diff --git a/backend/tests/test_mcp_endpoint.py b/backend/tests/test_mcp_endpoint.py index 59325322..525a2d3c 100644 --- a/backend/tests/test_mcp_endpoint.py +++ b/backend/tests/test_mcp_endpoint.py @@ -19,6 +19,7 @@ from fastapi.testclient import TestClient from fastmcp.client import Client from fastmcp.client.transports import StreamableHttpTransport +from starlette.routing import Route _IN_APP_ASK = { "presentation": "in_app", @@ -216,7 +217,6 @@ async def test_tools_list_pins_platform_and_extension_tools(app, pat_token): ("operation_id", "docstring", "message"), [ (None, "Start a review.", "explicit operation_id"), - ("start_review", "Start a review.", "must start with 'review_'"), ("review_start", None, "docstring"), ], ) @@ -241,6 +241,67 @@ async def endpoint(): create_mcp_app(api) +def _agent_route_app(operation_id: str) -> FastAPI: + # A synthetic agent route owned by the installed 'review' extension — the + # loader-stamped 'review' tag names the owner, exactly as a real router does. + # The endpoint mounts the same /mcp Route and mcp lifespan as the real app so + # tools/list resolves in-process. + held: dict[str, object] = {} + + @asynccontextmanager + async def lifespan(scope_app): + async with held["mcp"].lifespan(scope_app): + yield + + api = FastAPI(lifespan=lifespan) + router = APIRouter() + + async def endpoint(): + """Scan the review target.""" + + router.add_api_route( + "/scans", endpoint, methods=["POST"], operation_id=operation_id, tags=["agent"] + ) + api.include_router(router, prefix="/api/review", tags=["review"]) + + mcp = create_mcp_app(api) + held["mcp"] = mcp + api.router.routes.append( + Route("/mcp", mcp, methods=["POST", "DELETE"], include_in_schema=False) + ) + return api + + +def _served_operation_id(schema: dict, path: str) -> str: + return schema["paths"][path]["post"]["operationId"] + + +@pytest.mark.parametrize( + ("operation_id", "expected"), + [ + ("scan", "review_scan"), # an unprefixed id gains its owner's prefix + ("review_scan", "review_scan"), # an already-prefixed id passes through, never doubled + ], +) +async def test_extension_agent_route_derives_the_namespaced_tool( + druks_db, pat_token, operation_id, expected +): + api = _agent_route_app(operation_id) + + # The document the provider consumed and every later regeneration carry the + # derived id, not the bare one the author declared. + assert _served_operation_id(api.openapi(), "/api/review/scans") == expected + api.openapi_schema = None + assert _served_operation_id(api.openapi(), "/api/review/scans") == expected + + async with live(api), _client(api, pat_token) as client: + tools = {tool.name for tool in await client.list_tools()} + + assert expected in tools + if operation_id != expected: + assert operation_id not in tools + + async def test_claims_resolve_the_calling_account(app, druks_db): # get_usage must answer as the token's account — the forwarded bearer. mine = Account.get_or_create("op@example.com") diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 95996d20..e792f219 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -651,12 +651,14 @@ the prefix its own resource is called: router = APIRouter(prefix="/reviews") ``` -Tagging a route `agent` also derives it into an MCP tool: its explicit `operation_id`, -prefixed with the extension name, is the tool name and its docstring is the description. -`GET` derives read-only; a write declares `x-destructive: false` or `x-idempotent: true` -in `openapi_extra` only when that statement is genuinely true, otherwise the safe defaults -are destructive and non-idempotent. Boot refuses a missing or unprefixed tool name -and a missing docstring. +Tagging a route `agent` also derives it into an MCP tool: you give it an explicit +`operation_id`, and Druks derives the tool name by prefixing it with your extension +name — write `operation_id="add_peer"` in `peer_tracker` and the tool is +`peer_tracker_add_peer`. The docstring is the description. `GET` derives read-only; +a write declares `x-destructive: false` or `x-idempotent: true` in `openapi_extra` +only when that statement is genuinely true, otherwise the safe defaults are +destructive and non-idempotent. Boot refuses a missing `operation_id` or a missing +docstring. Two spellings run through druks, and which one a segment wears says who owns it: From 8dc9775ceed30037ea60fcec48ebf6ee4e854b46 Mon Sep 17 00:00:00 2001 From: "druks-operator[bot]" <284423593+druks-operator[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 05:56:10 +0000 Subject: [PATCH 2/2] Guard derived operation-id collisions and drop redundant namespacing cache check Co-Authored-By: Claude Opus 4.8 --- backend/druks/mcp/app.py | 30 +++++++++++++++++++++++------- backend/tests/test_mcp_endpoint.py | 23 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/backend/druks/mcp/app.py b/backend/druks/mcp/app.py index 125d9651..bac1d9de 100644 --- a/backend/druks/mcp/app.py +++ b/backend/druks/mcp/app.py @@ -100,8 +100,17 @@ def _namespace_agent_operations(spec: dict, extension_names: set[str]) -> None: # installed extension is the owner; platform agent operations carry no such # tag and keep their declared ids. An already-prefixed id passes through, so # stable names like ship_start never double — and the namespace is what makes - # the merged document's operation ids globally unique. - for operations in spec.get("paths", {}).values(): + # the merged document's operation ids globally unique. A derived id that would + # collide with another route's explicit id is rejected: before this derivation + # the clash was visible in the author's code, so the framework must surface it + # now that it owns the naming. + existing_ids = { + op.get("operationId") + for ops in spec.get("paths", {}).values() + for op in ops.values() + if isinstance(op, dict) and op.get("operationId") + } + for path, operations in spec.get("paths", {}).items(): for operation in operations.values(): if not isinstance(operation, dict) or "agent" not in operation.get("tags", []): continue @@ -110,7 +119,14 @@ def _namespace_agent_operations(spec: dict, extension_names: set[str]) -> None: continue extension = next((tag for tag in operation["tags"] if tag in extension_names), None) if extension and not operation_id.startswith(f"{extension}_"): - operation["operationId"] = f"{extension}_{operation_id}" + derived = f"{extension}_{operation_id}" + if derived in existing_ids: + raise InvalidAgentToolError( + path, + f"derived operation id {derived!r} collides with existing " + "operation id; rename the conflicting route", + ) + operation["operationId"] = derived def _install_agent_namespacing(api: FastAPI) -> None: @@ -119,14 +135,14 @@ def _install_agent_namespacing(api: FastAPI) -> None: # route contexts, which later generation silently discards. Wrap the app's # own generator so the provider here and every later /openapi.json share one # namespaced document: each fresh build (including after the openapi_schema - # reset below) re-derives the ids, and the pass-through check keeps a warm - # cache idempotent. + # reset below) re-derives the ids. generate() is the app's own api.openapi, + # which already returns the cached schema when it is warm, so namespaced() + # need not repeat that guard — and the derivation is idempotent, so running + # it on a warm cache leaves the already-prefixed ids untouched. extension_names = {extension.name for extension in iter_extensions()} generate = api.openapi def namespaced() -> dict: - if api.openapi_schema: - return api.openapi_schema spec = generate() _namespace_agent_operations(spec, extension_names) return spec diff --git a/backend/tests/test_mcp_endpoint.py b/backend/tests/test_mcp_endpoint.py index 525a2d3c..52b9bc38 100644 --- a/backend/tests/test_mcp_endpoint.py +++ b/backend/tests/test_mcp_endpoint.py @@ -241,6 +241,29 @@ async def endpoint(): create_mcp_app(api) +def test_derived_operation_id_collision_stops_boot(): + # The 'review' extension's unprefixed 'scan' derives to 'review_scan', which + # another route already claims explicitly — the framework must reject the + # clash rather than silently mint two operations sharing an id. + api = FastAPI() + router = APIRouter() + + async def scan(): + """Scan the review target.""" + + async def review_scan(): + """Re-run the review scan.""" + + router.add_api_route("/scans", scan, methods=["POST"], operation_id="scan", tags=["agent"]) + router.add_api_route( + "/rescans", review_scan, methods=["POST"], operation_id="review_scan", tags=["agent"] + ) + api.include_router(router, prefix="/api/review", tags=["review"]) + + with pytest.raises(InvalidAgentToolError, match="collides with existing operation id"): + create_mcp_app(api) + + def _agent_route_app(operation_id: str) -> FastAPI: # A synthetic agent route owned by the installed 'review' extension — the # loader-stamped 'review' tag names the owner, exactly as a real router does.