| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
`setLazyLoading()` defaults to true but never applied to a registry supplied through `setRegistry()`: the builder ran the chain loader itself and set `$eagerlyLoaded = true`, because it could not hand a loader to an instance it did not construct. Two consequences under a persistent runtime. The loaders run once at build, so a source that is not ready yet (a cold metadata cache) freezes the registry empty for the whole process and `tools/list` keeps returning `[]`. And `detectCapabilities()` reads `hasTools()` off that cold registry, so the server can advertise `tools: false` and a client that respects capabilities never calls `tools/list` at all. `Registry::deferLoadingFrom()` adopts a loader after construction, so the builder can defer instead of loading eagerly. It chains behind a loader the constructor took while that one is still owed its run, and replaces it once it has already run, since chaining would run it twice and discovery would rescan. Resetting `loaded` is what lets the adopted loader run on the next read: without it, a registry read before `build()` would keep the deferred loader forever unrun. Deferral is not conditional on the registry being empty. Gating it that way would mean a caller who hand-registers a single element before `setRegistry()` silently falls back to eager loading and gets the cold-source bug back. Instead `Registry::isEmpty()` — which reads the element arrays directly and so does not trigger the loader — tells `detectCapabilities()` that a deferred registry already holds elements, and it advertises them as one more opaque source. That over-advertises a registry holding only one kind, which is harmless per MCP semantics and already how custom loaders and discovery are treated. A foreign `RegistryInterface` cannot be deferred into and still loads eagerly. `RegistryInterface` is unchanged: both methods live on `Registry`, which is what the builder already type-checks for `loadFrom()`.
|
Closing this — I'd rather not push Registry in this direction. The underlying gap is real (a registry supplied through setRegistry() never participates in setLazyLoading(), and detectCapabilities() then reads a possibly-cold registry), but adopting a loader post-construction is not the right shape for it. Will come back with a different approach. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Follow-up to #389, which made loading lazy for the registry the builder constructs. This does the same for one the caller supplies.
The gap
setLazyLoading() defaults to true, but it never applied to a registry passed through setRegistry(). resolve() ran the chain loader itself and set $eagerlyLoaded = true, because the builder could not hand a loader to an instance it did not construct.
Two consequences under a persistent runtime (FrankenPHP worker mode, Laravel Octane):
Not hypothetical: symfony/mcp-bundle registers its registry as a Registry service and calls setRegistry() on it, and API Platform does the same on its Laravel side. Every consumer that supplies its own registry service is affected, and API Platform carried a custom tools/list handler purely to work around it.
The change
Registry::deferLoadingFrom() adopts a loader after construction, so the builder can defer instead of loading eagerly and report $eagerlyLoaded = false.
It chains behind a loader the constructor took while that one is still owed its run, and replaces it once it has already run — chaining there would run it twice and discovery would rescan. Resetting loaded is what lets the adopted loader run on the next read; without it a registry that was read before build() (a warmup, a debug command, a health check) would keep the deferred loader forever unrun, which is the same cold-source bug this PR exists to fix.
Deferral is deliberately not conditional on the registry being empty. Gating it that way would mean a caller who hand-registers a single element before setRegistry() silently falls back to eager loading and gets the bug back. Instead Registry::isEmpty() — which reads the element arrays directly and so does not trigger the loader while answering — tells detectCapabilities() that a deferred registry already holds elements, and it advertises them as one more opaque source.
That over-advertises a registry holding only one kind. Per the doctrine already stated in detectCapabilities()'s docblock, over-advertising is harmless under MCP semantics, and it is already how custom loaders and discovery are treated.
Two guards remain:
Notes
Tests
RegistryTest: deferLoadingFrom() does not run until the first read, runs exactly once across many reads, chains behind a constructor loader, and — the regression guard for the loaded reset — runs the adopted loader even when the constructor loader had already run. isEmpty() is true for a fresh registry without triggering a configured loader, and false after registerTool().
BuilderTest: an empty custom registry defers its loader past build() and runs it on the first read; a pre-populated one defers too; the deferred setup advertises tools: true while the loader is asserted never to run; setLazyLoading(false) still loads eagerly. testBuildAdvertisesToolsForPreloadedCustomRegistry passes unchanged.