feat(mcp): give every MCP tool route an explicit operation_id
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 "
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user