Uh oh!
There was an error while loading. Please reload this page.
fix(cli): capability resolver matches provider identities, not name fragments (#7652) - #7935
Merged
Merged
Conversation
…ragments (#7652) `os serve` auto-adds `mcp` to `requires`, then skips loading a provider when the app already supplies one. That check compared each provider's `nameMatch` fragments against loaded plugin names with `String.includes()` — and a plugin that CONSUMES a capability is conventionally named after what it consumes. So a consumer reliably satisfied its own provider's fragment and suppressed it. The stock showcase hit exactly that: it loads `com.objectstack.connector.mcp` (the outbound MCP *client* connector), `'mcp'` is a substring of that name, so `MCPServerPlugin` never loaded and `/api/v1/mcp` and `/api/v1/mcp/skill` answered 501 under a boot banner advertising the endpoint. Fix the class, not the collision. `Serve.providesCapability` now compares a plugin's `name` and constructor name to the declared identities by EQUALITY, and every registry entry declares the provider's real registered plugin id rather than a fragment of it. No exclusion list, no lengthened fragment, no load-order luck. Both directions were measured, not assumed. Reading the provider packages showed most name fragments were already dead — `service-cache` never matched `com.objectstack.service.cache` (dash vs dot), and 18 of 23 entries were carried entirely by their class name — so the entries now carry the ids those packages actually register. A drift test imports every provider package and asserts the name it registers is one the registry declares, so a rename cannot quietly return the resolver to double-loading. Acceptance is the card's own repro, not the resolver: a spawned `os serve` with the consumer plugin loaded answers `GET /api/v1/mcp/skill` 200 and returns real JSON-RPC results for `initialize` and `tools/list`. Reverse-verified — with the substring match restored, both go back to 501. Sweep of the remaining fragments for the same exposure: `mcp` was the only one with a realized in-repo collision (59 plugin names, 64 plugin classes scanned). `audit` was the only other single-word fragment, one consumer away from the same fate. Reported, not separately special-cased — the uniform fix covers both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BTDu3CXAxGiTc75pg9vT8
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 17 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also reference the affected code. These are read-only:
|
hotlong
marked this pull request as ready for review
August 12, 2026 06:07
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#7652
The class of bug, not the collision
os serveauto-addsmcptorequires, then skips loading a provider when the app already supplies one. That "already supplied?" check compared each provider'snameMatchfragments against loaded plugin names withString.includes():A plugin that consumes a capability is conventionally named after the capability it consumes. Substring matching cannot tell the two apart, so a consumer reliably satisfies its own provider's fragment and suppresses the provider it depends on.
The stock showcase hit exactly that: it loads
com.objectstack.connector.mcp— the outbound MCP client connector —'mcp'is a substring of that name, soMCPServerPluginnever loaded and/api/v1/mcpand/api/v1/mcp/skillanswered501 "MCP server is not available"under a boot banner advertising the endpoint.Premise verified on current
origin/main(3c9a67e). The card's line numbers had drifted (auto-add now:888,CAPABILITY_PROVIDERS.mcp:431,hasPluginMatching:2331) but the mechanism was exactly as described.The fix
Serve.providesCapabilitycompares a plugin'snameand its constructor name against the registry's declared identities by equality, and every entry declares the provider's real registered plugin id (com.objectstack.mcp) rather than a fragment of it.nameMatchis renamed toidentitiesso the semantic change can't be missed by a caller.No exclusion list for the connector, no lengthened fragment, no reordering so the provider wins by luck. A plugin either is the provider or it is not.
Both directions — measured, not assumed
A resolver that "fixes" this by matching nothing would make every capability load its default provider and look green in this exact repro. So the identities were read out of the provider packages rather than guessed. That measurement turned up something worth stating:
Most of the old name fragments were already dead.
service-cachenever matchedcom.objectstack.service.cache(dash vs dot),plugin-emailnever matchedcom.objectstack.service.email,trigger-record-changenever matchedcom.objectstack.trigger.record-change. 18 of 23 entries were carried entirely by their class name; onlyservice-automation,service-analytics,audit,plugin-webhook-outboxandmcphad a live name fragment. The registry now carries the ids those packages actually register, which is strictly more correct than before.Two dead aliases were dropped as part of that:
'mcp-server'(the plugin iscom.objectstack.mcp) and'SharingPlugin'(the class isSharingServicePlugin). Neither matched anything.A drift test imports every provider package, constructs the exported class, and asserts the
nameit registers is one the registry declares — so a provider renaming itself fails loudly instead of quietly returning the resolver to double-loading.Sweep for the same exposure elsewhere
Scanned all 26 fragment lists (including the three inline
hasPluginMatchingcall sites) against a corpus of 59 plugin names and 64 plugin classes in this repo, asking which fragments were satisfiable by a plugin that is not the provider:mcpcom.objectstack.connector.mcp— the realized bugmcpwas the only realized collision.auditis the only other single-word fragment and was one consumer away from the same fate — any plugin namedcom.*.audit-*or a class ending inAuditPluginwould have satisfied it. Class-name fragments were all exposed the same way (MyAuditPlugincontainsAuditPlugin); none collide in-repo today. Reported, not separately special-cased — the uniform equality fix covers all of them, and nothing here justifies widening the change further.Acceptance — the consequence, not the mechanism
packages/cli/test/serve-mcp-capability-collision.e2e.test.tsspawns the shippedbin/run.js, boots an app that loads the consumer plugin and never declaresmcp, then asks the card's own question:GET /api/v1/mcp/skillPOST /api/v1/mcpinitializeprotocolVersion+serverInfoPOST /api/v1/mcptools/list/mcpis authenticated, so the test mints a realosk_key through the product route against theserve --devadmin seed (same approach asserve-mcp-stdio-answers.e2e.test.ts).Reverse-verified: with the substring match restored in
hasPluginMatching, this file fails withexpected 501 to be 200on both routes — the card's exact symptom.The consumer is declared in the fixture rather than imported because
@objectstack/connector-mcpis not a dependency of@objectstack/cli, soturbo run test's^buildnever builds it and it would be absent in CI. The resolver reads exactly two fields off a loaded plugin, so a plugin declaring the connector's real identity reproduces the defect with full fidelity — andserve-capability-identity.test.tspins that identity against the connector's actual source, so the fixture cannot drift into testing a name nobody registers.Tests
packages/cli/test/serve-capability-identity.test.ts(new, 53 tests) — both directions: the consumer must not satisfymcp; every registered provider must still satisfy its own capability by name and by class; near-misses substring matching used to accept are rejected; connectors satisfy no capability they merely consume; plus the package drift guard.packages/cli/test/serve-mcp-capability-collision.e2e.test.ts(new, 3 tests) — the acceptance pin above.Gates run
packages/clisuite — 113 files, 1247 tests, all passnode scripts/pm/dispatch-gates.mjs <changed paths>→pnpm check:startup-registry-verdict✅,pnpm check:nul-bytes✅pnpm build(turbo, 71 tasks) ✅pnpm check:type-check-debt✅ — no•error line forpackages/cli; no ledger entry raisedpackages/cli's tsconfig excludestest/, so both new test files were type-checked standalone under the samestrictconfig — zero errorseslint --no-inline-configon all three changed files ✅Related, deliberately not folded in
#7645 (stdio transport deaf — merged) and #7915 (
os servebanner pollutes the stdio transport's stdout — filed). Nothing measured here changes how either should be read: both are transport-layer defects that occur afterMCPServerPluginloads, whereas this one prevented it from loading at all. Worth noting the three compound — on the stock showcase the plugin never loaded, so #7645's and #7915's surfaces were unreachable there regardless.Generated by Claude Code