Resource discovery DX: did-you-mean, relationship targets, session-no…#3
Merged
Conversation
…t-found log Closes three gaps an MCP agent hit when it had to guess a related resource's name instead of discovering it: - Registry#fetch now raises a "did you mean" message for an unknown resource: closest registered name(s) via stdlib DidYouMean::SpellChecker (dependency-free edit-distance fallback) + the full list when the catalog is short. Flows unchanged to the caller as a clean InvalidParams tool error via resolve_descriptor. - resource_schema now names each relationship's target_resource (callable via list/get) plus a best-effort target_name_attribute hint, resolved through the registry. So scheduled_notifications.notification is discoverably the `notifications` resource. Additive/backward-compatible: both fields are omitted when unresolved (e.g. polymorphic links). - The transport controller logs a WARN on session-not-found: greppable, id/token-free, records only whether a session-id header was present, so a non-shared cache_store misconfiguration shows in a satellite's own logs. Logger defaults to Rails.logger, overridable via mcp_logger. Bumps to 0.3.0. Full suite + rubocop + brakeman green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Azdaroth
commented
Jul 3, 2026
| # Logger for transport-level diagnostics. Defaults to Rails.logger when running | ||
| # inside Rails (nil outside it); overridable so a host can inject its own. | ||
| def mcp_logger | ||
| return Rails.logger if defined?(Rails) && Rails.respond_to?(:logger) |
Member
Author
There was a problem hiding this comment.
there is no way Rails won't respond to logger, hat's excessive. Also don't fallback to nil, instead fallback maybe to STDOUT
| # a header-missing client bug is distinguishable from a cache misconfiguration. | ||
| def mcp_log_session_not_found | ||
| logger = mcp_logger | ||
| return unless logger |
Member
Author
There was a problem hiding this comment.
woith a better fallback, that will be excessive
| # nearest registered name(s) for a near-miss and (b) — when the catalog is short — | ||
| # lists them all, so a caller (typically an MCP agent that guessed a name) can | ||
| # self-correct without another round-trip to the `resources` tool. | ||
| def unknown_resource_message(name) |
Member
Author
There was a problem hiding this comment.
move all of these related methods to a dedicated class, no need to clutter registry. Just delegate. Don't add methods comments, just a top-level class comment.
| kind: association.type.to_s, | ||
| polymorphic: association.polymorphic || false, | ||
| target_resource: target&.name, | ||
| target_name_attribute: name_attribute_for(target) |
Member
Author
There was a problem hiding this comment.
target attribute name is excessive, let's get rid of this concept here
…, drop target_name_attribute - Move did-you-mean/levenshtein logic out of Registry into a dedicated McpToolkit::UnknownResourceMessage (Registry just delegates). - mcp_logger: drop the redundant respond_to?(:logger) check; fall back to a $stdout Logger instead of nil, so mcp_log_session_not_found needs no guard. - Remove the target_name_attribute concept from resource_schema (keep target_resource): schema, tool description, CHANGELOG, and specs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Azdaroth
commented
Jul 3, 2026
| # and (b), when the catalog is short, lists them all, so a caller (typically an MCP | ||
| # agent that guessed a name) can self-correct without another round-trip to the | ||
| # `resources` tool. | ||
| class McpToolkit::UnknownResourceMessage |
Member
Author
There was a problem hiding this comment.
some basic test coverage would be useful
Addresses PR #3 review: basic tests for the extracted class — empty registry, near-miss suggestion, short-catalog listing, large-catalog no-op, and the dependency-free edit-distance fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
…t-found log
Closes three gaps an MCP agent hit when it had to guess a related resource's name instead of discovering it:
notificationsresource. Additive/backward-compatible: both fields are omitted when unresolved (e.g. polymorphic links).Bumps to 0.3.0. Full suite + rubocop + brakeman green.