Skip to content

Conversation

@maxisbey
Copy link
Contributor

@maxisbey maxisbey commented Feb 2, 2026

Replace the decorator-based handler registration on the lowlevel Server with direct on_* keyword arguments on the constructor. Handlers are now raw callables with a uniform (ctx, params) -> result signature, dispatched by method string.

  • Delete func_inspection.py (no longer needed without decorator introspection)
  • Remove RequestT generic from Server (transport-specific, never bound at construction)
  • Decouple ExperimentalHandlers from Server internals via callbacks
  • Update MCPServer to pass on_* kwargs via _create_handler_kwargs()
  • Make RequestContext.request_id optional (notification handlers pass None)

Tests and examples not yet updated — they still use the old decorator API.

Supersedes #1968.

@maxisbey maxisbey changed the base branch from sketch/lowlevel-server-v2 to main February 3, 2026 14:06
@maxisbey maxisbey force-pushed the sketch/lowlevel-server-v2-kwargs branch from acea3d7 to 8e1d947 Compare February 3, 2026 15:27
@maxisbey maxisbey changed the title refactor: replace Handler objects with on_* constructor kwargs refactor: replace lowlevel Server decorators with on_* constructor kwargs Feb 3, 2026
@claude
Copy link

claude bot commented Feb 3, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

if handler is not None:
self._notification_handlers[method] = handler

def _add_request_handler(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not as type safe as if we did the Handler approach here.

I don't think we decided for sure whether we wanted to be able to register/remove endpoints at runtime. Thoughts @Kludex ?


# TODO(maxisbey): remove private access — completion needs post-construction
# handler registration, find a better pattern for this
self._lowlevel_server._add_request_handler( # pyright: ignore[reportPrivateUsage]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usecase makes sense to me for adding request handlers after the fact... Unless we change it so that the Server object inside MCPServer is only constructed when you actually run the server, not when you construct MCPServer?

Thoughts @Kludex ?

@maxisbey maxisbey marked this pull request as draft February 3, 2026 15:41
] = lifespan,
# Request handlers
on_list_tools: Callable[
[ServerRequestContext[LifespanResultT, Any], types.PaginatedRequestParams | None],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be RequestT, why did you drop it? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that it doesn't make sense to have RequestT be a generic parameter referencing the Transport on the Server class. This is because Server can be instantiated and then used for many different Transports, and you would only know the actual type depending on what you pass in to the run method.

I could be wrong, but it didn't make sense to me. You thinking something else?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the ServerRequestContext depends on it anyway, so it seems something is wrong here.

This Any shouldn't be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed RequestT in the context object to default to Any since it'll be refactored in #2021 anyway.

I've removed the Any, here and in a few other places.

@maxisbey maxisbey force-pushed the sketch/lowlevel-server-v2-kwargs branch from 28c7209 to 7221cca Compare February 6, 2026 16:48
…args

Replace the decorator-based handler registration on the lowlevel Server with
direct on_* keyword arguments on the constructor. Handlers are raw callables
with a uniform (ctx, params) -> result signature.

- Server constructor takes on_list_tools, on_call_tool, etc.
- String-keyed dispatch instead of type-keyed
- Remove RequestT generic from Server (transport-specific, not bound at construction)
- Delete handler.py and func_inspection.py (no longer needed)
- Update ExperimentalHandlers to use callback-based registration
- Update MCPServer to pass on_* kwargs via _create_handler_kwargs()
- Update migration docs and docstrings
- Fix all migration.md examples to use ServerRequestContext instead of RequestContext
- Fix all imports to use 'from mcp.server import Server, ServerRequestContext'
- Apply ruff formatting to code examples
- Add Server constructor calls to examples that were missing them
- Re-export ServerRequestContext from mcp.server.__init__
- Add type hints to TaskResultHandler docstring example
…erver

Move handler functions from a dict-returning method to private methods
on MCPServer, passed directly to the Server constructor by name. This
eliminates the **kwargs unpacking pattern and makes the handler
registration explicit.
@maxisbey maxisbey force-pushed the sketch/lowlevel-server-v2-kwargs branch from 7221cca to 7e9e53f Compare February 9, 2026 13:38
Allow users to override default task handlers by passing on_get_task,
on_task_result, on_list_tasks, and on_cancel_task to enable_tasks().
User-provided handlers are registered first; defaults fill in the rest.
_task_support=task_support,
),
)
await handler(ctx, getattr(notify, "params", None))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to use getattr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea it's not, removed.



async def _ping_handler(request: types.PingRequest) -> types.ServerResult:
async def _ping_handler(ctx: Any, params: Any) -> types.EmptyResult:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why Any again? Please check all Anys in this branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea definitely shouldn't be Any here. I've updated this Any as well as gone through all the other Anys added, I think it's in a better spot now.

The subscribe capability in ResourcesCapability was hardcoded to False,
even when on_subscribe_resource handler was provided. Now dynamically
checks whether a subscribe handler is registered.

Also applies ruff formatting to experimental.py.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants