modelcontextprotocol / modelcontextprotocol/php-sdk
Builder::build() silently skips configured file-based discovery when symfony/finder is missing — should fail loudly
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 1.6k
- Forks
- 173
- Avg merge
- 2d 49m
- Merged PRs (30d)
- 23
Description
Calling setDiscovery() is an explicit declaration that the server's tools come from attribute discovery. But since symfony/finder is only a suggested dependency, a host application can easily end up without it — and when that happens, build() does this:
if (null !== $this->discoveryBasePath) {
if (null !== $this->discoverer || class_exists(Finder::class)) {
// ... discovery runs
} else {
$logger->warning('File-based discovery requires symfony/finder...');
}
}
The result is the worst failure mode available: the server builds successfully, initialize succeeds, and tools/list returns an empty array. The operator sees a healthy server; the symptom surfaces far from the cause, as confused MCP clients with no tools. The only breadcrumb is a single warning log line.
This also contradicts the SDK's own Discoverer::__construct(), which already throws RuntimeException('File-based discovery requires symfony/finder. ...') for exactly this situation — the builder's class_exists pre-check just routes around that guard, downgrading a configured-but-impossible feature from an error to a whisper.
Proposal: when discoveryBasePath is set, no custom discoverer was supplied, and Finder is unavailable, build() should throw (the Discoverer's existing message is perfect) instead of warning-and-skipping. This costs nothing for explicit-registration users, client-only users, or anyone without setDiscovery() — it only converts a silent production mystery into an immediate, actionable boot error for people who asked for discovery and can't have it.
Observed on v0.7.0. Context: we hit this failure mode while integrating the SDK into a Symfony bundle (pimcore/data-hub-simple-rest#312) and worked around it by requiring symfony/finder in the bundle directly — which remains the right consumer-side fix, but doesn't help the next integrator who doesn't know about the silent path.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating Builder::build() and the setDiscovery() handling described in the issue, then compare its missing-Finder branch with Discoverer::__construct(). Add or update a focused test for a configured discovery path without symfony/finder. Done means build() throws the existing actionable RuntimeException instead of logging a warning and producing an empty tool list.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100