Skip to content

Commit ecafdc7

Browse files
committed
Re-scope the codemod to run-on-v2 minimalism
The goal is that migrated v1 code runs on v2 on its legacy paths, not that it adopts v2 idioms. Applying that bar: - Leave e.error.code / .message / .data chains alone: v2's MCPError keeps a typed .error ErrorData, so the v1 spelling runs and type-checks unchanged. The except-binding tracking goes with it. - Rewrite one-argument McpError(...) calls to MCPError.from_error_data(...) instead of flattening the inline ErrorData: the user's expression is kept as written and the non-inline form no longer needs a marker. - Convert v1 positional arguments on the lowlevel Server constructor to keywords (v2 is keyword-only after name but kept v1's names and order), pinned against the installed signature by a new ratchet test. - Reword every marker message that pointed at replaced internals or at the successor of the removed experimental tasks API; state removals plainly instead of steering users onto new surfaces. - Teach the batch harness that a reportArgumentType error naming a detonating argument type (timedelta, AnyUrl) is a real break, never v2 strictness drift, and ignore stale work/ directories.
1 parent 801095a commit ecafdc7

7 files changed

Lines changed: 159 additions & 246 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
work/
2+
work/

scripts/codemod-batch-test/run.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@
4747
# failed to flag looks exactly like that.
4848
DRIFT_RULES = frozenset({"reportArgumentType", "reportOptionalSubscript", "reportOptionalMemberAccess"})
4949

50+
# Argument types that detonate at RUNTIME on v2 (`timedelta` where v2 takes float
51+
# seconds, `AnyUrl` where v2 takes `str`). A `reportArgumentType` error naming one
52+
# of these is a real break pyright happens to catch, never strictness drift.
53+
DETONATOR_TYPES = ("timedelta", "AnyUrl")
54+
5055
# The v1 environment lives OUTSIDE the SDK checkout: inside it, uv resolves the
5156
# SDK workspace itself no matter the cwd, and the env would silently hold v2.
5257
V1_ENV_DIR = Path.home() / ".cache" / "mcp-codemod-batch-test" / "v1env"
@@ -258,7 +263,12 @@ def _audit_repo(repo: Repo, *, v1_python: Path, fresh: bool) -> tuple[dict[str,
258263
# Uncovered errors are actionable unless everything says v2 strictness
259264
# drift: an untouched file, no mcp symbol in the message, and a rule
260265
# from the drift list. A silent codemod miss fails any one of these.
261-
if error.file not in rewritten_files and "mcp" not in error.message.lower() and error.rule in DRIFT_RULES:
266+
if (
267+
error.file not in rewritten_files
268+
and "mcp" not in error.message.lower()
269+
and error.rule in DRIFT_RULES
270+
and not any(f'of type "{detonator}"' in error.message for detonator in DETONATOR_TYPES)
271+
):
262272
drift.append(error)
263273
else:
264274
actionable.append(error)

src/mcp-codemod/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ manual fix-up.
2828
`streamablehttp_client` -> `streamable_http_client`), resolved through the
2929
file's imports so an aliased import or an unrelated symbol with the same name
3030
is never touched.
31-
- `McpError(ErrorData(code=..., message=...))` to the flat `MCPError(...)`
32-
constructor, and `e.error.code` / `e.error.message` / `e.error.data` to
33-
`e.code` / `e.message` / `e.data` inside an `except McpError as e:` block.
31+
- `McpError(...)` calls to `MCPError.from_error_data(...)`, which takes the
32+
same single `ErrorData` argument the v1 constructor did. (`e.error.code` and
33+
friends are deliberately left alone: they still work on v2.)
3434
- camelCase attribute reads on `mcp.types` models to their snake_case v2
3535
spellings (`.inputSchema` -> `.input_schema`), restricted to the field names
3636
the v1 types actually declared. Other camelCase APIs (`logging.getLogger`, a
@@ -54,8 +54,8 @@ The codemod never guesses at these; it leaves them exactly as written and adds a
5454

5555
- Removed APIs that have no drop-in replacement (`create_connected_server_and_client_session`,
5656
the WebSocket transport, `mcp.shared.progress`, `get_context()`), and imports
57-
of whole module namespaces v2 deleted (the experimental tasks API, which is
58-
first-class on v2). Together with the renames these account for every public
57+
of whole module namespaces v2 deleted (the removed experimental tasks
58+
API). Together with the renames these account for every public
5959
module v1 shipped, so an import is never left to fail unexplained.
6060
- The v1 `mcp.types` names with no v2 home (`Cursor`, the `TASK_*` constants, the
6161
type-machinery aliases). `mcp_types` is not a name-superset of v1's `mcp.types`,

src/mcp-codemod/mcp_codemod/_mappings.py

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"CAMEL_FIELDS",
1616
"ERRORDATA_QNAMES",
1717
"FASTMCP_QNAMES",
18+
"LOWLEVEL_CTOR_POSITIONAL_PARAMS",
1819
"LOWLEVEL_DECORATOR_METHODS",
1920
"LOWLEVEL_REMOVED_ATTRS",
2021
"LOWLEVEL_SERVER_QNAMES",
@@ -61,23 +62,15 @@
6162
# for every public module v1 shipped, which `tests/codemod/test_mappings.py`
6263
# pins against the frozen v1 module list and the installed v2 package.
6364
REMOVED_MODULES: dict[str, str] = {
64-
"mcp.client.experimental": (
65-
"removed: the experimental tasks API is first-class on v2; see the tasks section of the migration guide"
66-
),
67-
"mcp.server.experimental": (
68-
"removed: the experimental tasks API is first-class on v2; see the tasks section of the migration guide"
69-
),
70-
"mcp.server.lowlevel.experimental": (
71-
"removed: the experimental tasks API is first-class on v2; see the tasks section of the migration guide"
72-
),
73-
"mcp.shared.experimental": (
74-
"removed: the experimental tasks API is first-class on v2; see the tasks section of the migration guide"
75-
),
65+
"mcp.client.experimental": ("removed: the v1 experimental tasks API was deleted and has no replacement"),
66+
"mcp.server.experimental": ("removed: the v1 experimental tasks API was deleted and has no replacement"),
67+
"mcp.server.lowlevel.experimental": ("removed: the v1 experimental tasks API was deleted and has no replacement"),
68+
"mcp.shared.experimental": ("removed: the v1 experimental tasks API was deleted and has no replacement"),
7669
"mcp.client.websocket": "removed: the WebSocket transport was deleted",
7770
"mcp.server.websocket": "removed: the WebSocket transport was deleted",
7871
"mcp.server.lowlevel.func_inspection": "removed: it was an internal helper of the lowlevel server",
7972
"mcp.shared.progress": "removed: report progress with `ctx.report_progress()` inside a handler",
80-
"mcp.shared.response_router": "removed: superseded by `JSONRPCDispatcher`",
73+
"mcp.shared.response_router": "removed: it was internal session machinery; there is no public replacement",
8174
}
8275

8376
# Symbol renames, keyed by every v1 qualified name the symbol was reachable from.
@@ -102,7 +95,8 @@
10295
# `# mcp-codemod:` marker carrying the replacement guidance.
10396
REMOVED_APIS: dict[str, str] = {
10497
"mcp.shared.memory.create_connected_server_and_client_session": (
105-
"removed: connect an in-memory pair with `mcp.Client(server)` instead"
98+
"removed: pair `create_client_server_memory_streams()` with `Server.run()` and a `ClientSession` "
99+
"to keep the v1 test shape, or use `mcp.Client(server)`"
106100
),
107101
"mcp.shared.progress.progress": "removed: report progress with `ctx.report_progress()` inside a handler",
108102
"mcp.shared.progress.Progress": "removed: `mcp.shared.progress` was deleted",
@@ -113,12 +107,12 @@
113107
"split: use `mcp.server.context.ServerRequestContext` or `mcp.client.context.ClientRequestContext`"
114108
),
115109
"mcp.os.win32.utilities.terminate_windows_process": "removed",
116-
"mcp.shared.session.BaseSession": "removed: sessions now run on `JSONRPCDispatcher`",
110+
"mcp.shared.session.BaseSession": "removed: use `ClientSession` or `ServerSession` directly",
117111
"mcp.server.lowlevel.server.request_ctx": (
118112
"removed: the module-level ContextVar is gone; handlers now receive `ctx` explicitly"
119113
),
120114
# The v1 `mcp.types` names with no same-name home in `mcp_types`. The task
121-
# vocabulary collapsed into the literal strings on v2 and the rest were v1
115+
# vocabulary left with the experimental tasks API and the rest were v1
122116
# type-machinery aliases. Enumerating every one is what keeps the
123117
# `mcp.types` -> `mcp_types` rewrite honest: `tests/codemod/test_mappings.py`
124118
# checks that every other public v1 name resolves on `mcp_types`, so an
@@ -138,15 +132,15 @@
138132
"mcp.types.ServerRequestType": "removed: use the `ServerRequest` union",
139133
"mcp.types.ServerNotificationType": "removed: use the `ServerNotification` union",
140134
"mcp.types.ServerResultType": "removed: use the `ServerResult` union",
141-
"mcp.types.TaskExecutionMode": "removed: `ToolExecution.task_support` takes the literal string on v2",
142-
"mcp.types.TASK_REQUIRED": 'removed: use the literal string `"required"`',
143-
"mcp.types.TASK_OPTIONAL": 'removed: use the literal string `"optional"`',
144-
"mcp.types.TASK_FORBIDDEN": 'removed: use the literal string `"forbidden"`',
145-
"mcp.types.TASK_STATUS_WORKING": 'removed: use the literal string `"working"`',
146-
"mcp.types.TASK_STATUS_INPUT_REQUIRED": 'removed: use the literal string `"input_required"`',
147-
"mcp.types.TASK_STATUS_COMPLETED": 'removed: use the literal string `"completed"`',
148-
"mcp.types.TASK_STATUS_FAILED": 'removed: use the literal string `"failed"`',
149-
"mcp.types.TASK_STATUS_CANCELLED": 'removed: use the literal string `"cancelled"`',
135+
"mcp.types.TaskExecutionMode": "removed with the v1 experimental tasks API",
136+
"mcp.types.TASK_REQUIRED": "removed with the v1 experimental tasks API",
137+
"mcp.types.TASK_OPTIONAL": "removed with the v1 experimental tasks API",
138+
"mcp.types.TASK_FORBIDDEN": "removed with the v1 experimental tasks API",
139+
"mcp.types.TASK_STATUS_WORKING": "removed with the v1 experimental tasks API",
140+
"mcp.types.TASK_STATUS_INPUT_REQUIRED": "removed with the v1 experimental tasks API",
141+
"mcp.types.TASK_STATUS_COMPLETED": "removed with the v1 experimental tasks API",
142+
"mcp.types.TASK_STATUS_FAILED": "removed with the v1 experimental tasks API",
143+
"mcp.types.TASK_STATUS_CANCELLED": "removed with the v1 experimental tasks API",
150144
}
151145

152146
# Extras the v1 `mcp` distribution declared that v2 does not, with guidance.
@@ -285,6 +279,12 @@ def _to_snake(name: str) -> str:
285279
"mount_path": "removed: mount the app under a Starlette route instead",
286280
}
287281

282+
# The v1 lowlevel `Server.__init__` parameters after `name`, in positional order.
283+
# v2 makes everything after `name` keyword-only but keeps these names, so a v1
284+
# positional argument converts to the keyword at its position one for one.
285+
# Pinned against the installed v2 constructor by `tests/codemod/test_mappings.py`.
286+
LOWLEVEL_CTOR_POSITIONAL_PARAMS: tuple[str, ...] = ("version", "instructions", "website_url", "icons", "lifespan")
287+
288288
# Attributes removed from the lowlevel `Server` whose NAMES survive elsewhere on
289289
# v2 (`Context.request_context` is a live idiom), so unlike `REMOVED_ATTRS` they
290290
# are only matched against a receiver the pre-pass proved is a lowlevel server.

src/mcp-codemod/mcp_codemod/_transformer.py

Lines changed: 50 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@
3232
from libcst.helpers import get_full_name_for_node
3333
from libcst.metadata import (
3434
CodeRange,
35-
ExpressionContext,
36-
ExpressionContextProvider,
3735
MetadataWrapper,
3836
PositionProvider,
3937
QualifiedNameProvider,
@@ -44,6 +42,7 @@
4442
CAMEL_FIELDS,
4543
ERRORDATA_QNAMES,
4644
FASTMCP_QNAMES,
45+
LOWLEVEL_CTOR_POSITIONAL_PARAMS,
4746
LOWLEVEL_DECORATOR_METHODS,
4847
LOWLEVEL_REMOVED_ATTRS,
4948
LOWLEVEL_SERVER_QNAMES,
@@ -276,7 +275,7 @@ def visit_AnnAssign(self, node: cst.AnnAssign) -> None:
276275

277276

278277
class _V1ToV2(cst.CSTTransformer):
279-
METADATA_DEPENDENCIES = (QualifiedNameProvider, PositionProvider, ExpressionContextProvider)
278+
METADATA_DEPENDENCIES = (QualifiedNameProvider, PositionProvider)
280279

281280
def __init__(self, prepass: _PrePass, *, add_markers: bool) -> None:
282281
super().__init__()
@@ -302,7 +301,6 @@ def __init__(self, prepass: _PrePass, *, add_markers: bool) -> None:
302301
# and whether its type names `McpError`. An inner handler that re-binds a
303302
# name shadows the outer binding of that name; any other inner handler is
304303
# transparent to the lookup.
305-
self._except_bindings: list[tuple[str, bool]] = []
306304
# Calls that are a `with` item bound to a three-element tuple: the one form
307305
# whose result tuple `leave_WithItem` can rewrite rather than flag.
308306
self._narrowable_calls: set[int] = set()
@@ -409,37 +407,6 @@ def visit_Arg(self, node: cst.Arg) -> None:
409407
def visit_Param(self, node: cst.Param) -> None:
410408
self._not_a_reference.add(id(node.name))
411409

412-
def _is_mcperror_binding(self, name: str) -> bool:
413-
"""Whether the nearest enclosing handler that binds `name` catches `McpError`.
414-
415-
Handlers that bind some other name (or none) are transparent, so a nested
416-
`try`/`except` inside an `except McpError as e:` does not hide `e`; one
417-
that re-binds `e` itself shadows the outer binding.
418-
"""
419-
for bound, is_mcperror in reversed(self._except_bindings):
420-
if bound == name:
421-
return is_mcperror
422-
return False
423-
424-
def visit_ExceptHandler(self, node: cst.ExceptHandler) -> None:
425-
bound = ""
426-
if node.name is not None and isinstance(node.name.name, cst.Name):
427-
bound = node.name.name.value
428-
# `except (McpError, ValueError) as e:` catches a tuple of types.
429-
if isinstance(node.type, cst.Tuple):
430-
caught: list[cst.BaseExpression] = [element.value for element in node.type.elements]
431-
elif node.type is not None:
432-
caught = [node.type]
433-
else:
434-
caught = []
435-
self._except_bindings.append((bound, any(self._qualified(kind) & MCPERROR_QNAMES for kind in caught)))
436-
437-
def leave_ExceptHandler(
438-
self, original_node: cst.ExceptHandler, updated_node: cst.ExceptHandler
439-
) -> cst.ExceptHandler:
440-
self._except_bindings.pop()
441-
return updated_node
442-
443410
# ------------------------------------------------------------------ imports
444411

445412
def leave_ImportFrom(self, original_node: cst.ImportFrom, updated_node: cst.ImportFrom) -> cst.ImportFrom:
@@ -580,22 +547,10 @@ def leave_Name(self, original_node: cst.Name, updated_node: cst.Name) -> cst.Nam
580547
return updated_node
581548

582549
def leave_Attribute(self, original_node: cst.Attribute, updated_node: cst.Attribute) -> cst.BaseExpression:
583-
# A READ of `e.error.code` -> `e.code` when `e` is bound by `except McpError
584-
# as e:`. Only the full three-part chain in a load context is touched: a bare
585-
# `e.error` may be a whole `ErrorData` being passed somewhere, and an
586-
# ASSIGNMENT like `e.error.message = ...` must stay as written -- v2's
587-
# `MCPError.message` is a read-only property over the still-mutable `.error`,
588-
# so collapsing a write would break code that works on v2 today.
589-
if (
590-
original_node.attr.value in ("code", "message", "data")
591-
and isinstance(original_node.value, cst.Attribute)
592-
and original_node.value.attr.value == "error"
593-
and isinstance(original_node.value.value, cst.Name)
594-
and self._is_mcperror_binding(original_node.value.value.value)
595-
and self.get_metadata(ExpressionContextProvider, original_node, None) is ExpressionContext.LOAD
596-
):
597-
self.rewrites["mcperror_attr"] += 1
598-
return updated_node.with_changes(value=cst.ensure_type(updated_node.value, cst.Attribute).value)
550+
# `e.error.code` on a caught error is deliberately NOT collapsed to `e.code`:
551+
# v2's `MCPError` keeps a typed `.error` ErrorData, so the v1 spelling runs
552+
# and type-checks unchanged -- touching it would be modernization, not
553+
# migration.
599554

600555
# An attribute the lowlevel `Server` lost whose name survives elsewhere on
601556
# v2, matched only against a receiver the pre-pass proved is such a server
@@ -679,15 +634,22 @@ def leave_Attribute(self, original_node: cst.Attribute, updated_node: cst.Attrib
679634
def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> cst.Call:
680635
callee = self._qualified(original_node.func)
681636

682-
# `McpError(ErrorData(code=..., message=..., data=...))` flattened to
683-
# `MCPError(code=..., message=..., data=...)`; the name itself is renamed by
684-
# `leave_Name`, which has already run on the inner nodes. v1's constructor
685-
# took a single `ErrorData`; when that one argument is anything other than
686-
# an inline `ErrorData(...)` call there is nothing safe to unpack, so the
687-
# call is marked instead -- v2's signature is `(code, message, data=None)`.
637+
# v1's constructor took a single `ErrorData`; v2's classmethod
638+
# `MCPError.from_error_data(...)` takes exactly that argument, so any
639+
# one-argument call converts uniformly -- the user's expression is kept as
640+
# written, whatever it is. The name itself is renamed by `leave_Name`,
641+
# which has already run on the inner nodes.
642+
if callee & MCPERROR_QNAMES and len(original_node.args) == 1:
643+
self.rewrites["mcperror_ctor"] += 1
644+
return updated_node.with_changes(
645+
func=cst.Attribute(value=updated_node.func, attr=cst.Name("from_error_data"))
646+
)
647+
688648
# A subclass's `super().__init__(...)` is the same constructor spelled the
689-
# one way a qualified name cannot reach, so it gets the same treatment.
690-
if (callee & MCPERROR_QNAMES or self._is_mcperror_super_init(original_node)) and len(original_node.args) == 1:
649+
# one way a classmethod cannot replace, so the inline `ErrorData(...)` is
650+
# flattened into v2's `(code, message, data=None)` arguments; any other
651+
# single argument has nothing safe to unpack and is marked.
652+
if self._is_mcperror_super_init(original_node) and len(original_node.args) == 1:
691653
wrapped = original_node.args[0].value
692654
if isinstance(wrapped, cst.Call) and self._qualified(wrapped.func) & ERRORDATA_QNAMES:
693655
self.rewrites["mcperror_ctor"] += 1
@@ -700,12 +662,12 @@ def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> cst.Cal
700662
"unpack the `ErrorData` being passed here into those arguments",
701663
)
702664

703-
# camelCase keyword arguments still work on v2 (every model field also
704-
# accepts its camelCase alias by name), so unlike an attribute READ this
705-
# rename is cosmetic and cannot break the call -- which is why, unlike the
706-
# attribute form, the risky tier needs no review marker here. Every
707-
# hand-migrated example in the SDK converted them, so the codemod follows
708-
# suit, gated on the callee resolving into the SDK.
665+
# camelCase keyword arguments still work at RUNTIME on v2 (every model
666+
# field accepts its camelCase alias by name), but the synthesized
667+
# `__init__` signatures are snake_case, so leaving them fails the user's
668+
# own type-checking. The rename cannot break the call -- which is why,
669+
# unlike the attribute form, the risky tier needs no review marker here --
670+
# and is gated on the callee resolving into the SDK.
709671
if any(name == "mcp" or name.startswith(("mcp.", "mcp_types.")) for name in callee):
710672
arguments: list[cst.Arg] = []
711673
renamed_any = False
@@ -747,6 +709,29 @@ def leave_Call(self, original_node: cst.Call, updated_node: cst.Call) -> cst.Cal
747709
elif keyword in REMOVED_CTOR_PARAMS:
748710
self._diag(argument, "removed_ctor_param", "manual", f"`{keyword}=` {REMOVED_CTOR_PARAMS[keyword]}")
749711

712+
# The lowlevel `Server` constructor is keyword-only after `name` on v2, but
713+
# its parameters kept v1's names and order, so v1 positionals convert to
714+
# keywords one for one. A `*`-splat hides how many positions it fills, so a
715+
# call carrying one is left for v2 to reject loudly at construction.
716+
if (
717+
callee & LOWLEVEL_SERVER_QNAMES
718+
and 1 < len(original_node.args) <= 1 + len(LOWLEVEL_CTOR_POSITIONAL_PARAMS)
719+
and not any(argument.star for argument in original_node.args)
720+
):
721+
arguments = []
722+
for index, argument in enumerate(updated_node.args):
723+
if index > 0 and argument.keyword is None:
724+
self.rewrites["lowlevel_ctor_kwargs"] += 1
725+
argument = argument.with_changes(
726+
keyword=cst.Name(LOWLEVEL_CTOR_POSITIONAL_PARAMS[index - 1]),
727+
equal=cst.AssignEqual(
728+
whitespace_before=cst.SimpleWhitespace(""), whitespace_after=cst.SimpleWhitespace("")
729+
),
730+
)
731+
arguments.append(argument)
732+
if arguments != list(updated_node.args):
733+
updated_node = updated_node.with_changes(args=arguments)
734+
750735
# The streamable-HTTP client's keyword surface and yield shape both changed.
751736
# The keyword check lives here so that it fires however the call is used (an
752737
# `async with` item, `enter_async_context(...)`, an intermediate variable).

0 commit comments

Comments
 (0)