Skip to content

Fail loudly when discovery is configured but symfony/finder is missing - #464

Open
Faneraiy14 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Faneraiy14:fix/discovery-finder-missing-fails-loudly
Open

Fail loudly when discovery is configured but symfony/finder is missing#464
Faneraiy14 wants to merge 1 commit into
modelcontextprotocol:mainfrom
Faneraiy14:fix/discovery-finder-missing-fails-loudly

Conversation

@Faneraiy14

Copy link
Copy Markdown

Summary

Builder::build() routed around Discoverer::__construct()'s own class_exists(Finder::class) guard: instead of throwing, it logged a single warning and silently skipped the DiscoveryLoader. The result is the worst failure mode available — the server builds successfully, initialize succeeds, and tools/list quietly returns an empty array, with no actionable signal beyond one log line the operator may never see.

setDiscovery() is an explicit request for file-based discovery; if that request can't be honored, build() should fail immediately with the same RuntimeException and message Discoverer already throws for the identical condition, rather than downgrading a configured-but-impossible feature to a whisper.

Fixes #398

Test plan

  • vendor/bin/phpunit --testsuite=unit — 1513 tests, 3919 assertions, all passing
  • vendor/bin/phpunit --testsuite=integration — 64 tests, 145 assertions, all passing
  • vendor/bin/phpstan analyse (full repo, level 6 per phpstan.dist.neon) — no errors

Builder::build() previously routed around Discoverer::__construct()'s
own class_exists(Finder::class) guard: instead of throwing, it logged
a single warning and silently skipped the DiscoveryLoader. The result
is the worst failure mode available - the server builds successfully,
initialize succeeds, and tools/list quietly returns an empty array,
with no actionable signal beyond one log line the operator may never
see.

setDiscovery() is an explicit request for file-based discovery; if
that request can't be honored, build() should fail immediately with
the same RuntimeException and message Discoverer already throws for
the identical condition, rather than downgrading a configured-but-
impossible feature to a whisper.

Fixes modelcontextprotocol#398
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.

Builder::build() silently skips configured file-based discovery when symfony/finder is missing — should fail loudly

1 participant