Skip to content

Fix to make Symfony Finder component optional#267

Open
bigdevlarry wants to merge 1 commit intomodelcontextprotocol:mainfrom
bigdevlarry:Make-Symfony-Finder-component-optional
Open

Fix to make Symfony Finder component optional#267
bigdevlarry wants to merge 1 commit intomodelcontextprotocol:mainfrom
bigdevlarry:Make-Symfony-Finder-component-optional

Conversation

@bigdevlarry
Copy link
Contributor

@bigdevlarry bigdevlarry commented Mar 15, 2026

Motivation and Context

The changes make the Symfony Finder component optional, as referenced in issue #263

How Has This Been Tested?

Yes

Breaking Changes

Not a breaking change

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@bigdevlarry bigdevlarry force-pushed the Make-Symfony-Finder-component-optional branch from 68bd923 to bc19684 Compare March 15, 2026 02:36
@bigdevlarry bigdevlarry force-pushed the Make-Symfony-Finder-component-optional branch from eb1a5f9 to abf1b30 Compare March 23, 2026 13:06
@bigdevlarry bigdevlarry force-pushed the Make-Symfony-Finder-component-optional branch from dd9bfef to 6e8e705 Compare March 23, 2026 13:14
@soyuka
Copy link
Contributor

soyuka commented Mar 24, 2026

Can you also add a change to https://github.com/modelcontextprotocol/php-sdk/blob/main/src/Server/Builder.php#L526 ? Avoid calling the createDiscoverer when Finder isn't accessible + log? This makes php-sdk not dependent from the Finder.

@bigdevlarry
Copy link
Contributor Author

@soyuka, this means the discoverer will never be called if there's no finder, and we won't be able to raise the exception to prompt the installation of the finder itself. By your suggestion, adding a null check silences the error in the logs, and the program continues as expected. Is this the ideal behaviour, or should we halt the program with the exception as we have it, so users go back to install? Let me know what you think as well @chr-hertel

@soyuka
Copy link
Contributor

soyuka commented Mar 25, 2026

Mhh from this PR I thought you wanted to move the Finder to a dev-requirement making this a soft dependency. From my point of view if that's the case the code should work after installing with composer and I'd have thought that it'd make sense to skip classes dependent from the Finder.

If we don't want to move this requirement, then I'm not sure I understand how this code path is going to happen as its required in our dependencies.

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.

3 participants