feat(mcp): discover tool providers by registration event, not by scanning (#2077)#2083
Merged
Merged
Conversation
…ning Two mechanisms in this app did the same job and the older one was the more complex. MCP discovery probes every installed app's info.xml, builds candidate container aliases, resolves each through the container catching autoloader misses, and caches the resolution map with two invalidation mechanisms — an app-list hash, plus a clamped TTL because an app upgrade can add a provider without changing the app list. Roughly 86 lines of probing plus a cache, to find nine providers. All of it exists because it scans for something apps never announce. Apps do announce a listener. Nextcloud has two idioms and the split is the argument: when CORE owns a registry it adds a `registerXProvider()` to IRegistrationContext — thirty-odd of those, and only core can write them. When an APP owns a registry it dispatches a typed event, which is what workflowengine does for operations, checks and entities. OpenRegister is an app. That is already why the flow node registry uses an event (#2074), and core ships no MCP of its own, so there is nothing else upstream to conform to. The announced path is collected FIRST; the alias scan still runs after it for one release so the fleet is never broken mid-migration, and a provider whose appId is already present is skipped so a migrated app is never collected twice. Collection failure is logged, never fatal: an app with a broken listener should cost its own tools, not remove MCP from the instance. The scan and its cache come out in a follow-up, once the five apps that still register by alias have migrated. Removing them here would break every one of them.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ❌ | ✅ 174/174 | |||
| npm | ✅ | ✅ 555/555 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-07-24 16:32 UTC
Download the full PDF report from the workflow artifacts.
…thods Inserting collectAnnouncedMcpProviders() ahead of collectPerAppMcpProviders() left the alias-scan's docblock sitting above the NEW method and the old one with none — phpcs caught both. Each method now carries its own again. 0 errors across 1052 files; 88 flow tests green.
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
1 participant
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.
Proposal: or-mcp-registration-event
Summary
Discover MCP tool providers the way Nextcloud discovers everything else — a
typed registration event — and retire the bespoke scan.
Why
Two mechanisms in one app did the same job, and the older one was the more
complex.
MCP discovery probes every installed app's
info.xml, builds candidatecontainer aliases (
OCA\OpenRegister\Mcp\IMcpToolProvider::<appId>), resolveseach through the container catching autoloader misses, and caches the
resolution map with two invalidation mechanisms — an app-list hash, plus a
clamped TTL because an app upgrade can add a provider without changing the app
list. Roughly 86 lines of probing plus the cache, to discover nine providers.
That complexity exists because it scans for something apps never announce.
Apps do announce a listener.
Nextcloud has two idioms, and the split is the argument
IRegistrationContext::registerXProvider(class).Thirty-odd of these exist. Only core can add them, because only core can add
methods to that interface.
workflowenginedoes exactly this for operations, checks and entities.
OpenRegister is an app, so the event is the idiomatic choice for anything it
owns. That is already why the flow node registry uses one (#2074). Core ships
no MCP at all, so there is nothing upstream to conform to beyond the idiom.
What Changes
RegisterMcpToolProvidersEvent, mirroringRegisterFlowNodesEvent.collectAnnouncedMcpProviders()runs FIRST; the alias scan still runs afterit for one release so the fleet is never broken mid-migration.
never collected twice.
its own tools, not everyone else's.
Migration
Five apps outside OpenRegister register by alias today: hermiq, openbuild,
opencatalogi, decidesk, and
nextcloud-app-template. Each needs a listener.The template goes first — it is what new apps copy.
The alias scan and its cache are removed in a follow-up, once the fleet has
migrated. Removing them in the same change would break every app that has not.
Out of scope
#[McpTool]attribute scanning andIMcpScannableServicessolve a differentproblem (which classes to reflect over) and are untouched.
Safe to merge before the apps migrate
Both paths run: announced first, alias scan after, with appId de-duplication between them. Nothing changes for an app until it adds its listener.
🤖 Generated with Claude Code