Skip to content

Conversation

@WebMamba
Copy link

@WebMamba WebMamba commented Nov 9, 2025

Q A
Bug fix? yes
New feature? no
Docs? no
Issues Fix #830
License MIT

As said on the issue, when monolog is not installed, the container can't compile the services.
The solution here is to register the service only if the bundle is installed

@carsonbot carsonbot added Bug Something isn't working Status: Needs Review labels Nov 9, 2025
@carsonbot carsonbot changed the title Fix services when monolog is not installed Fix services when monolog is not installed Nov 9, 2025
@WebMamba
Copy link
Author

WebMamba commented Nov 9, 2025

@chr-hertel tell me what you think about this solution, but while working on it I was thinking that just require monolog could be even better. I don't see any good reason to not use it.

@chr-hertel chr-hertel added the MCP Bundle Issues & PRs about the MCP SDK integration bundle label Nov 11, 2025
@carsonbot carsonbot changed the title Fix services when monolog is not installed [MCP Bundle] Fix services when monolog is not installed Nov 11, 2025
@chr-hertel
Copy link
Member

I think that solution works for me - is this the only place where we'd need that tho?

I also wonder about the implementation in the McpBundle class:

if ($transports['stdio']) {
$container->register('mcp.server.command', McpCommand::class)
->setArguments([
new Reference('mcp.server'),
new Reference('logger'),
])
->addTag('console.command');
}
if ($transports['http']) {
$container->register('mcp.server.controller', McpController::class)
->setArguments([
new Reference('mcp.server'),
new Reference('mcp.psr_http_factory'),
new Reference('mcp.http_foundation_factory'),
new Reference('mcp.psr17_factory'),
new Reference('mcp.psr17_factory'),
new Reference('monolog.logger.mcp', ContainerInterface::NULL_ON_INVALID_REFERENCE),
])
->setPublic(true)
->addTag('controller.service_arguments');
}

@camilleislasse do you recall why it's Reference('logger') with McpCommand, but McpController is with Reference('monolog.logger.mcp')?

return static function (ContainerConfigurator $container): void {
$container->services()
->set('monolog.logger.mcp')
if (class_exists(\Symfony\Bundle\MonologBundle\MonologBundle::class)) {
Copy link
Member

Choose a reason for hiding this comment

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

please use in import here instead the FCQN:

Suggested change
if (class_exists(\Symfony\Bundle\MonologBundle\MonologBundle::class)) {
if (class_exists(MonologBundle::class)) {

camilleislasse added a commit to camilleislasse/ai that referenced this pull request Nov 13, 2025
Fixes symfony#830

- Wrap monolog.logger.mcp service creation in class_exists() check
- Add ->nullOnInvalid() to setLogger() call
- Uniformize logger references in McpCommand and McpController
- Update test to skip when MonologBundle is not installed

Builds on symfony#849 by @WebMamba
@camilleislasse
Copy link

@camilleislasse do you recall why it's Reference('logger') with McpCommand, but McpController is with Reference('monolog.logger.mcp')?

@chr-hertel I don't recall why, probably a mistake on my part

@OskarStark
Copy link
Contributor

Closing in favor of #864

Thanks

@OskarStark OskarStark closed this Nov 13, 2025
@camilleislasse
Copy link

thank's @WebMamba

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

Labels

Bug Something isn't working MCP Bundle Issues & PRs about the MCP SDK integration bundle Status: Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MCP Bundle] missing requirement to monolog

5 participants