From 565f6a9c9d23f984f8497d51d11bc1db0e79bb04 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 17:48:45 +0800 Subject: [PATCH] feat(mcp): give every MCP tool route an explicit operation_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fastapi-mcp uses each route's OpenAPI operationId as the MCP tool name exposed to LLM clients (fastapi_mcp/server.py:619-620). When the operation_id is omitted, FastAPI auto-generates a name like `prepare_submit_job_prepare_submit_job_post` (function name + path + method), which is verbose and ugly for the agent. This change adds an explicit `operation_id=` to all 17 MCP tool routes in spark_executor/server.py, named after the handler function (stripping the leading underscore). Concretely, the LLM client now sees tool names like `prepare_submit_job`, `confirm_submit_job`, `get_job_status`, etc. — same as the route paths. `/health` (the only non-MCP GET route) is left untouched; its auto-generated name (`health_check_health_get`) is never exposed. Also adds a regression test `test_every_mcp_route_has_explicit_clean_operation_id` in tests/integration/test_mcp_routes.py that: * fetches the live OpenAPI spec, * asserts every MCP tool route has an operationId, * rejects FastAPI's auto-generated format (e.g. operationId ending in `_post` / `_get`), * asserts no two routes share an operationId (fastapi-mcp keys its tool registry by name, so duplicates would shadow silently). All 226 tests pass. Co-Authored-By: Claude Fable 5 --- spark_executor/server.py | 17 ++++++++++ tests/integration/test_mcp_routes.py | 51 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/spark_executor/server.py b/spark_executor/server.py index e1bfe29..9554adf 100644 --- a/spark_executor/server.py +++ b/spark_executor/server.py @@ -83,6 +83,7 @@ def health_check(): @app.post( "/prepare_submit_job", + operation_id="prepare_submit_job", summary="Prepare a Spark job submission (no spark-submit yet)", description=( "Snapshot the named Connection's master / deploy_mode / spark_conf / " @@ -110,6 +111,7 @@ def _prepare_submit_job(req: PrepareSubmitJobRequest): @app.post( "/confirm_submit_job", + operation_id="confirm_submit_job", summary="Confirm and submit a previously-prepared job", description=( "Actually invoke spark-submit for the PendingSubmission identified " @@ -124,6 +126,7 @@ def _confirm_submit_job(req: PendingIdRequest): @app.post( "/list_pending_jobs", + operation_id="list_pending_jobs", summary="List all pending submissions", description="Return every PendingSubmission in any status (PENDING, SUBMITTED, CANCELLED, FAILED).", ) @@ -133,6 +136,7 @@ def _list_pending_jobs(_req: EmptyRequest = EmptyRequest()): @app.post( "/get_pending_job", + operation_id="get_pending_job", summary="Get a single pending submission", description="Return the PendingSubmission identified by pending_id, including its current status and outcome fields.", ) @@ -142,6 +146,7 @@ def _get_pending_job(req: PendingIdRequest): @app.post( "/update_pending_job", + operation_id="update_pending_job", summary="Update an unsubmitted pending submission", description=( "Modify parameters of a PENDING submission before confirm_submit_job. " @@ -155,6 +160,7 @@ def _update_pending_job(req: UpdatePendingJobRequest): @app.post( "/cancel_pending_job", + operation_id="cancel_pending_job", summary="Cancel a pending submission", description=( "Flip a PENDING (or already-CANCELLED) PendingSubmission to CANCELLED. " @@ -170,6 +176,7 @@ def _cancel_pending_job(req: PendingIdRequest): @app.post( "/get_job_status", + operation_id="get_job_status", summary="Query YARN for a job's current status", description=( "Return the YARN application state (RUNNING / SUCCEEDED / FAILED / " @@ -183,6 +190,7 @@ def _get_job_status(req: JobIdRequest): @app.post( "/get_job_result", + operation_id="get_job_result", summary="Query YARN for a job's terminal result view", description=( "Return a terminal-oriented view of a Spark job: final_status, " @@ -197,6 +205,7 @@ def _get_job_result(req: JobIdRequest): @app.post( "/get_job_logs", + operation_id="get_job_logs", summary="Fetch aggregated container logs for a job", description=( "Pull aggregated logs from the YARN ResourceManager. Returns the last " @@ -210,6 +219,7 @@ def _get_job_logs(req: GetJobLogsRequest): @app.post( "/kill_job", + operation_id="kill_job", summary="Kill a running job", description="PUT state=KILLED to YARN REST API for the job's application_id.", ) @@ -221,6 +231,7 @@ def _kill_job(req: JobIdRequest): @app.post( "/save_connection", + operation_id="save_connection", summary="Save or update a named Spark connection", description=( "Upsert a Connection record (master URL, deploy mode, optional YARN RM URL, " @@ -235,6 +246,7 @@ def _save_connection(req: SaveConnectionRequest): @app.post( "/list_connections", + operation_id="list_connections", summary="List all saved Spark connections", description="Return every Connection in the registry (model_dump form).", ) @@ -244,6 +256,7 @@ def _list_connections(_req: EmptyRequest = EmptyRequest()): @app.post( "/get_connection", + operation_id="get_connection", summary="Get a single connection by name", description="Return the Connection record, or 404 if not found.", ) @@ -253,6 +266,7 @@ def _get_connection(req: ConnectionNameRequest): @app.post( "/delete_connection", + operation_id="delete_connection", summary="Delete a saved connection", description="Remove a Connection by name. 404 if not found.", ) @@ -264,6 +278,7 @@ def _delete_connection(req: ConnectionNameRequest): @app.post( "/generate_job_file", + operation_id="generate_job_file", summary="Write LLM-generated PySpark code to disk", description=( "Takes a PySpark code string and writes it to a timestamped file under " @@ -279,6 +294,7 @@ def _generate_job_file(req: GenerateJobFileRequest): @app.post( "/read_job_file", + operation_id="read_job_file", summary="Read the contents of an existing PySpark script", description=( "Returns the text content of an existing script file at the given " @@ -294,6 +310,7 @@ def _read_job_file(req: ReadJobFileRequest): @app.post( "/update_job_file", + operation_id="update_job_file", summary="Overwrite an existing PySpark script with new content", description=( "Replaces the entire content of an existing script file. Path must " diff --git a/tests/integration/test_mcp_routes.py b/tests/integration/test_mcp_routes.py index 1d13527..0fcf0b9 100644 --- a/tests/integration/test_mcp_routes.py +++ b/tests/integration/test_mcp_routes.py @@ -64,6 +64,57 @@ def test_seventeen_tool_routes_registered(): assert "/update_pending_job" in paths +# --- operation_id: pin clean MCP tool names (no auto-generated suffixes) --- +# +# fastapi-mcp uses each route's OpenAPI `operationId` as the MCP tool name +# exposed to LLM clients (fastapi_mcp/server.py:619-620). If we omit +# `operation_id=`, FastAPI auto-generates a name like +# `prepare_submit_job_prepare_submit_job_post` (function_name + path + +# method), which is ugly for the agent. This test asserts every MCP tool +# route has an explicit, clean operation_id matching its handler. + +# Set of routes that are NOT MCP tools (no operation_id expected). +_NON_MCP_PATHS = {"/health", "/openapi.json", "/docs", "/docs/oauth2-redirect", "/redoc"} + + +def test_every_mcp_route_has_explicit_clean_operation_id(): + c = TestClient(app) + openapi = c.get("/openapi.json").json() + paths = openapi["paths"] + + seen: list[tuple[str, str, str]] = [] # (path, method, operation_id) + for path, methods in paths.items(): + if path in _NON_MCP_PATHS: + continue + for method, op in methods.items(): + if method.upper() not in {"GET", "POST", "PUT", "DELETE", "PATCH"}: + continue + op_id = op.get("operationId") + assert op_id, ( + f"route {method.upper()} {path} has no operationId in OpenAPI — " + f"fastapi-mcp will fall back to an auto-generated name like " + f"`{path.strip('/').replace('/', '_')}_{method}_...` and expose " + f"that to LLM clients. Add an explicit `operation_id=` to the decorator." + ) + # Reject FastAPI's auto-generated format: it always embeds the + # HTTP method as a `_post`/`_get` suffix, and repeats the path. + assert not op_id.endswith(f"_{method}"), ( + f"route {method.upper()} {path} has auto-generated operationId " + f"{op_id!r} (ends with `_{method}`). Add an explicit " + f"`operation_id=` to the decorator." + ) + seen.append((path, method.upper(), op_id)) + + # Sanity: we expect at least the 16 MCP tool routes from the project. + assert len(seen) >= 16, f"only found {len(seen)} MCP tool routes, expected >=16: {seen}" + + # Every operation_id must be unique — fastapi-mcp's tool registry keys + # by name, so duplicates would silently shadow one another. + ids = [op_id for _, _, op_id in seen] + dupes = [x for x in set(ids) if ids.count(x) > 1] + assert not dupes, f"duplicate operation_ids: {dupes}" + + # --- End-to-end body-based calls (the gap the route-registration test missed) --- def test_save_connection_accepts_dict_spark_conf_in_body():