Repository navigation
[Capability][Server] Add element sources and isolate loading failures - #687
Draft
chr-hertel wants to merge 1 commit into
Draft
chr-hertel wants to merge 1 commit into
chr-hertel wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
An unavailable child causes every registry read to rerun healthy loaders, filesystem discovery, and schema reflection instead of retaining successful initialization work during retries.
1 open issue
What changed in this PR
Adds value-based element sources to the PHP SDK’s capability registry and isolates loading failures so healthy elements remain available while failed loads are retried.
Changes:
- Introduces element collections, declared element kinds, and
Builder::addSource(). - Moves discovery onto the source adapter and deprecates
DiscoveryLoader. - Adds partial-load handling, retry tests, and registration documentation.
| File | Description |
|---|---|
| tests/Unit/Server/BuilderTest.php | Tests source failures, capabilities, and discovery precedence. |
| tests/Unit/Capability/Registry/Source/ElementSourceTest.php | Tests source loading, retries, serialization, and discovery. |
| tests/Unit/Capability/Registry/Loader/ChainLoaderTest.php | Tests failure isolation and logging. |
| tests/Unit/Capability/Registry/LazyRegistryTest.php | Tests pending loads after partial failures. |
| src/Server/Builder.php | Integrates sources, capability declarations, and retryable loading. |
| src/Exception/PartialLoadException.php | Introduces the incomplete-load exception. |
| src/Capability/Registry/Source/ElementSourceInterface.php | Defines source provision and kind declarations. |
| src/Capability/Registry/Source/ElementKind.php | Defines the four element kinds. |
| src/Capability/Registry/Source/ElementCollection.php | Holds element references in a readonly collection. |
| src/Capability/Registry/Source/DiscoverySource.php | Adapts discovery results into collections. |
| src/Capability/Registry/Loader/SourceLoader.php | Registers source elements with optional gap-filling behavior. |
| src/Capability/Registry/Loader/DiscoveryLoader.php | Adds a deprecation annotation. |
| src/Capability/Registry/Loader/ChainLoader.php | Logs failures and continues remaining loaders. |
| src/Capability/Registry/LazyRegistry.php | Keeps partial loads pending and exposes load status. |
| src/Capability/Registry.php | Serves partial results while retaining retry state. |
| src/Capability/DeferredRegistryInterface.php | Adds load-status reporting. |
| docs/servers/registration.md | Documents sources, precedence, and retries. |
| docs/run/server-builder.md | Adds the source registration API reference. |
| CHANGELOG.md | Records source APIs, failure-handling changes, and deprecation. |
Review effort: Balanced
chr-hertel
force-pushed
the
issue-495-capability-element-sources
branch
from
November 8, 2026 15:02
7105cab to
f9cf392
Compare
chr-hertel
force-pushed
the
element-sources
branch
from
November 8, 2026 15:03
32bc5ae to
fd375b8
Compare
chr-hertel
force-pushed
the
issue-495-capability-element-sources
branch
from
November 8, 2026 15:39
f9cf392 to
fa5a2d2
Compare
chr-hertel
force-pushed
the
element-sources
branch
from
November 8, 2026 15:39
fd375b8 to
2fcac14
Compare
chr-hertel
force-pushed
the
element-sources
branch
from
November 8, 2026 15:57
2fcac14 to
8e69d74
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Registry loading can advertise incomplete capabilities, while retry handling can break last-write-wins ordering and change read-dependent custom loaders’ derived registrations during recovery.
3 open issues
1 resolved since last review
Review effort: Balanced
Comment on lines
+55
to
+56
| if (!$runRemaining && !isset($retry[$position])) { | ||
| continue; |
Comment on lines
+59
to
+60
| try { | ||
| $loader->load($registry); |
chr-hertel
force-pushed
the
issue-495-capability-element-sources
branch
from
November 8, 2026 17:09
fa5a2d2 to
a567732
Compare
chr-hertel
force-pushed
the
element-sources
branch
from
November 8, 2026 17:09
8e69d74 to
9f04e20
Compare
soyuka
reviewed
Nov 9, 2026
| /** | ||
| * @param callable(string): bool $exists | ||
| */ | ||
| private function mayWrite(string $kind, string $key, callable $exists): bool |
chr-hertel
force-pushed
the
element-sources
branch
from
November 10, 2026 00:19
9f04e20 to
d2764b5
Compare
chr-hertel
force-pushed
the
issue-495-capability-element-sources
branch
from
November 10, 2026 00:28
a567732 to
efe0366
Compare
chr-hertel
force-pushed
the
issue-495-capability-element-sources
branch
from
November 10, 2026 21:37
efe0366 to
176b86a
Compare
chr-hertel
force-pushed
the
element-sources
branch
from
November 10, 2026 21:37
d2764b5 to
b21c651
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #686, items 1, 3 and 4 of #495.
A source returns its elements as an immutable
ElementCollection, andBuilder::addSource()adds it to the load. Discovery becomes one of those sources, without the$ownedstate ofDiscoveryLoader, which is deprecated.A failing loader or source is now logged and skipped, the rest stays servable and the load stays pending, so the next read retries only what failed. While the load is incomplete the server advertises all kinds, otherwise a client would miss them for the whole session. [BC Break]
build()no longer throws for such a failure.Where this differs from #495:
kinds()on sources, since capabilities are detected at request time with [Server][Capability] SplitRegistryInterfaceby role and defer loading into any registry #686has*and the$loadingguard stay for nowNot in here: per-element isolation for reflected elements (#477), so one bad
addTool()still skips all manually added elements. Also no caching decorator and no visibility seam yet. A source that stays down is called and logged on every read - not sure yet if that needs a backoff, WDYT?