Skip to content

Conversation

@Kehrlann
Copy link
Contributor

@Kehrlann Kehrlann commented Nov 4, 2025

Fixes #4670

This PR introduces two variants (sync and async) of a "registry" of handlers, ClientMcpSyncHandlersRegistry.

These registries have two main responsibilities:

  • Discovering client MCP-annotated beans
  • Dispatching method calls per MCP client name

This allows to break the dependency between MCP clients and MCP-annotated beans. This was a problem when doing sampling, where there was a cycle MCP Client -> MCP handlers -> ChatClient -> MCP Client.
With this PR, the MCP clients depend on the registry, but the registry does not depend on the MCP-handlers. Instead, it discovers them dynamically and makes handlers available by client name. See #4751 for the initial attempt at fixing the circular dependency.

The flow to capture annotations has two steps:

  1. An initial scanning phase, with a BeanFactoryPostProcessor. Bean types are scanned for annotations, and the bean names found are recorded for later use. Additionally, @McpSampling and @McpElicitation annotations are tracked per client name. This allows to bootstrap MCP clients in the McpClientAutoConfiguration: by the time the clients are created, we know their capabilities, even if the annotated beans have not been instantiated yet.
  2. After all singletons have been instantiated, using SmartInitializingSingleton, actual bean instances are retrieved, and from the bean instances and annotated methods, MCP handlers are populated in the registry, stored by client name.

Notes:

  • There can only be one sampling and one elicitation annotation per client, but other annotations support multiple handlers.
  • The async version chains handlers sequentially, similar to the original Java SDK implementation (source)

@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch 5 times, most recently from cfce918 to 4835ccc Compare November 5, 2025 09:51
@Kehrlann Kehrlann changed the title [DRAFT] Add McpClientHandlersRegistry Add McpClientHandlersRegistry Nov 5, 2025
@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch from 4835ccc to 2b1f49b Compare November 5, 2025 12:57
@Kehrlann Kehrlann marked this pull request as ready for review November 5, 2025 13:13
@Kehrlann Kehrlann marked this pull request as draft November 5, 2025 16:40
@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch 3 times, most recently from 52676cd to 18e986a Compare November 6, 2025 09:20
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
- In febf86c, we broke a dependency cycle ChatClient -> McpClient
- With the introduction of ClientMcpSyncHandlersRegistry and the async
  variant, there is no dependency McpClient -> MCP handlers anymore,
  breaking the cycle in a simpler way.
- Here, we revert most of the changes of febf86c, but keep the tests.

Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch 3 times, most recently from ad8d987 to 4ab6503 Compare November 6, 2025 10:00
@Kehrlann Kehrlann marked this pull request as ready for review November 6, 2025 11:24
@Kehrlann Kehrlann requested a review from tzolov November 6, 2025 11:24
@tzolov tzolov added this to the 1.1.0.RC1 milestone Nov 6, 2025
@tzolov tzolov self-assigned this Nov 6, 2025
@tzolov tzolov added the MCP label Nov 6, 2025
Copy link
Contributor

@tzolov tzolov left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @Kehrlann
There are some small formatting issues I'll resolve while merging

if (!foundAnnotations.isEmpty()) {
this.allAnnotatedBeans.add(beanName);
}
for (var foundAnnotation : foundAnnotations) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this block can be nested in side the if (!foundAnnotations.isEmpty()) { above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already pretty nested ; I'm reluctant to nest it more.
The code is functionally equivalent anyway.

But that's not a strong opinion.

- Remove custom class resolution method and use AutoProxyUtils instead

Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch from 0bdeea7 to fa6a1f4 Compare November 7, 2025 09:05
Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
@Kehrlann Kehrlann force-pushed the dgarnier/mcp-client-handlers-registry branch from 5baface to 3929649 Compare November 7, 2025 15:45
tzolov pushed a commit that referenced this pull request Nov 7, 2025
…es (#4802)

Replace specification factory pattern with centralized handler registries
that scan and register MCP client handlers (sampling, elicitation, logging,
progress, and list-changed notifications). This simplifies the
auto-configuration by:

- Introducing ClientMcpSyncHandlersRegistry and ClientMcpAsyncHandlersRegistry
  that scan beans once for annotations and expose handlers by client name
- Removing intermediate specification factory beans and customizers
- Directly configuring MCP client specs from registries during client creation
- Eliminating need for separate specification classes per handler type
- Simplifying ToolCallingAutoConfiguration by removing
  BeanDefinitionRegistryPostProcessor complexity
- In febf86c, we broke a dependency cycle ChatClient -> McpClient
- With the introduction of ClientMcpSyncHandlersRegistry and the async
  variant, there is no dependency McpClient -> MCP handlers anymore,
  breaking the cycle in a simpler way.
- Here, we revert most of the changes of febf86c, but keep the tests.
- Remove unused MCP annotated beans auto-configuration
- Introduce AbstractClientMcpHandlerRegistry
- Find MCP Client annotations on @component beans
- AbstractClientMcpHandlerRegistry also discovers proxied beans
- Remove custom class resolution method and use AutoProxyUtils instead
- Add logging to MCP handlers registry
- Throw MCP Error on missing sampling and elicitation handlers in a client
- Fix missing auto-configurations McpClientAutoConfigurationIT

Signed-off-by: Daniel Garnier-Moiroux <[email protected]>
@tzolov
Copy link
Contributor

tzolov commented Nov 7, 2025

Rebased, squashed and merged at 2a2f155

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP Client Initialization Timing and Tool Callback Resolution Issues

2 participants