Repository navigation
[Server][Capability] Split RegistryInterface by role and defer loading into any registry - #686
chr-hertel wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Capability detection can trigger a supplied registry’s loader during build() despite lazy loading, potentially freezing a cold source empty or failing the build.
1 open issue
What changed in this PR
This PR advances #495 by splitting registry responsibilities and adding opt-in deferred loading for caller-supplied registries, while preserving their default eager loading.
Changes:
- Introduces catalog, resolver and writer interfaces, with request handlers depending on the role they use.
- Adds a lazy registry decorator and a load-lifecycle interface for custom registries.
- Adds regression tests and documents the loading behavior and compatibility change.
| File | Description |
|---|---|
| tests/Unit/Server/BuilderTest.php | Covers supplied-registry loading and capability detection. |
| tests/Unit/Capability/Registry/LazyRegistryTest.php | Tests deferred loading, retries and event suppression. |
| src/Server/Stateless/StandardHeaderValidator.php | Depends on the resolver interface. |
| src/Server/Handler/Request/ResourceUnsubscribeHandler.php | Depends on the resolver interface. |
| src/Server/Handler/Request/ResourceSubscribeHandler.php | Depends on the resolver interface. |
| src/Server/Handler/Request/ReadResourceHandler.php | Depends on the resolver interface. |
| src/Server/Handler/Request/ListToolsHandler.php | Depends on the catalog interface. |
| src/Server/Handler/Request/ListResourceTemplatesHandler.php | Depends on the catalog interface. |
| src/Server/Handler/Request/ListResourcesHandler.php | Depends on the catalog interface. |
| src/Server/Handler/Request/ListPromptsHandler.php | Depends on the catalog interface. |
| src/Server/Handler/Request/GetPromptHandler.php | Depends on the resolver interface. |
| src/Server/Handler/Request/CompletionCompleteHandler.php | Depends on the resolver interface. |
| src/Server/Handler/Request/CallToolHandler.php | Depends on the resolver interface. |
| src/Server/Builder.php | Wraps explicitly deferred custom registries and adjusts capability detection. |
| src/Capability/RegistryInterface.php | Extends the three role interfaces. |
| src/Capability/Registry/LazyRegistry.php | Adds the deferred-loading decorator. |
| src/Capability/Registry.php | Implements the load-lifecycle interface. |
| src/Capability/ElementWriterInterface.php | Defines registration and removal operations. |
| src/Capability/ElementResolverInterface.php | Defines individual element lookups. |
| src/Capability/ElementCatalogInterface.php | Defines paginated element listings. |
| src/Capability/DeferredRegistryInterface.php | Defines loading and event-suppression contracts. |
| docs/run/server-builder.md | Documents custom-registry loading options. |
| CHANGELOG.md | Records the new interfaces and compatibility change. |
Review effort: Balanced
7105cab to
f9cf392
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved registry-loading defects can prevent retries after a failed concurrent load and trigger premature capability detection for third-party lazy registries.
2 open issues
1 resolved since last review
Review effort: Balanced
fa5a2d2 to
a567732
Compare
a567732 to
efe0366
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues remain: capability-loading exceptions can escape request handling and terminate the stdio loop, while concurrent loading can incorrectly mark the decorator loaded and permanently lose registrations.
2 open issues
2 resolved since last review
Review effort: Balanced
efe0366 to
176b86a
Compare


Step 4 of #495:
RegistryInterfacenow extends a catalog, a resolver and a writer role, and request handlers depend on the role they use. Existing implementations keep working.setLazyLoading(true)now also defers loading into a registry fromsetRegistry(). The builder wraps it in an internal decorator instead of handing the registry a loader, so noRegistryhas to adopt one after construction - that was the objection on #494, happy to hear if this shape still has the same problem.Capabilities are now detected when a request asks for them, since nothing reads them earlier - so the builder doesn't need to know what is loaded and a server advertises what it actually holds.
[BC Break] with that explicit call, a registry you read directly stays empty until the server first reads it. Without the call it still loads at build.
[BC Break]
initialize,server/discoverandsubscriptions/listenrun a pending load, so the handshake is no longer free of discovery.Configuration::$capabilitiesbecomescapabilities().Compared to #495,
DeferredRegistryInterfacehasloadFrom()andisLoaded()instead ofisEmpty().