From bd2db38a1f7d748a80ac511c452e066a3ee77d54 Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 3 Jul 2026 10:29:34 +0200 Subject: [PATCH 1/3] Resource discovery DX: did-you-mean, relationship targets, session-not-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) --- CHANGELOG.md | 25 +++++ lib/mcp_toolkit.rb | 3 + lib/mcp_toolkit/registry.rb | 83 +++++++++++++- lib/mcp_toolkit/resource_schema.rb | 92 +++++++++++++-- lib/mcp_toolkit/tools/resource_schema.rb | 6 +- .../transport/controller_methods.rb | 26 +++++ lib/mcp_toolkit/version.rb | 2 +- .../registry_and_executors_spec.rb | 105 ++++++++++++++++++ .../transport/controller_methods_spec.rb | 77 +++++++++++++ 9 files changed, 405 insertions(+), 14 deletions(-) create mode 100644 spec/mcp_toolkit/transport/controller_methods_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index e525ecf..b8de0f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,28 @@ +## [0.3.0] - 2026-07-03 + +### Added + +- **Resource discovery DX** improvements, closing gaps hit when an MCP agent had to guess a + related resource's name rather than discover it: + - The `resource_schema` tool now names, for each relationship, the `target_resource` it + resolves to (callable via `list` / `get`) plus, when known, a `target_name_attribute` hint of + that resource's human-readable field. A `scheduled_notifications.notification` link is thus + discoverably the `notifications` resource instead of a name to guess. Additive and backward + compatible: both fields are omitted when the target can't be resolved (e.g. a polymorphic + link), so existing relationship consumers are unaffected. + - An unknown resource name now raises a "did you mean" message — the closest registered + name(s) via Ruby's stdlib `DidYouMean::SpellChecker` (with a dependency-free edit-distance + fallback), plus the full list when the catalog is short. It flows unchanged to the caller as + a clean `InvalidParams` tool error via the existing `resolve_descriptor` path. + +### Changed + +- The transport controller now logs a WARN when a POST arrives with no matching session + (`mcp_render_session_not_found`): a greppable, id/token-free line recording only whether a + session-id header was present, so a non-shared `cache_store` misconfiguration surfaces in a + satellite's own logs instead of only as a client-side 404. The logger defaults to + `Rails.logger` and is overridable via the new `mcp_logger` controller hook. + ## [0.2.0] - 2026-07-02 ### Added diff --git a/lib/mcp_toolkit.rb b/lib/mcp_toolkit.rb index f34cce7..5e9528d 100644 --- a/lib/mcp_toolkit.rb +++ b/lib/mcp_toolkit.rb @@ -38,6 +38,8 @@ # time/conversions - Time#iso8601 (Serializer::Base timestamps; # Time.zone.parse in ListExecutor) # date_time/conversions - DateTime#iso8601 (token expires_at may be a DateTime) +# string/inflections - String#pluralize (ResourceSchema resolves a link's +# singular name to the plural registered resource name) require "active_support" require "active_support/core_ext/object/blank" require "active_support/core_ext/hash/keys" @@ -45,6 +47,7 @@ require "active_support/core_ext/enumerable" require "active_support/core_ext/time/conversions" require "active_support/core_ext/date_time/conversions" +require "active_support/core_ext/string/inflections" # The version constant is needed eagerly by the gemspec (before the loader is set # up), so it stays an explicit require rather than an autoload. diff --git a/lib/mcp_toolkit/registry.rb b/lib/mcp_toolkit/registry.rb index 6e56725..410407a 100644 --- a/lib/mcp_toolkit/registry.rb +++ b/lib/mcp_toolkit/registry.rb @@ -1,5 +1,15 @@ # frozen_string_literal: true +# `did_you_mean` is a Ruby default gem (loaded at interpreter startup on stock +# builds), used to suggest the closest registered resource name on a typo. A +# trimmed runtime without it falls back to the small edit-distance matcher below, +# so the require is guarded rather than assumed. +begin + require "did_you_mean" +rescue LoadError + nil +end + # Central registry of read-only resources exposed via the MCP server. Resources # are registered at boot (in a `to_prepare` initializer) and consumed by the # generic `resources` / `resource_schema` / `get` / `list` tools. @@ -10,6 +20,13 @@ class McpToolkit::Registry class UnknownResource < StandardError; end + # When the catalog has at most this many resources, the UnknownResource message + # lists all of them (cheap for a caller to scan); above it, only the closest + # suggestions are offered so the message stays short. + FULL_LIST_MAX = 10 + # Cap on the number of "did you mean" suggestions offered for a near-miss name. + MAX_SUGGESTIONS = 3 + def initialize @resources = {} @default_required_permissions_scope = nil @@ -43,7 +60,7 @@ def required_scope_for(resource) end def fetch(name) - find(name) or raise(UnknownResource, "unknown resource: #{name.inspect}") + find(name) or raise(UnknownResource, unknown_resource_message(name)) end def find(name) @@ -64,4 +81,68 @@ def resource_names def reset! @resources = {} end + + private + + # Builds the UnknownResource message: states the bad name, then (a) suggests the + # 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) + message = "unknown resource: #{name.inspect}" + names = resource_names + return message if names.empty? + + suggestions = suggestions_for(name.to_s, names) + message += ". Did you mean #{quote_join(suggestions)}?" if suggestions.any? + message += " Registered resources: #{quote_join(names.sort)}." if names.size <= FULL_LIST_MAX + message + end + + # Closest registered names to a near-miss. Uses Ruby's stdlib DidYouMean spell + # checker when available (typo/transposition aware), else the prefix/substring + + # edit-distance fallback below. + def suggestions_for(name, names) + if defined?(DidYouMean::SpellChecker) + Array(DidYouMean::SpellChecker.new(dictionary: names).correct(name)).first(MAX_SUGGESTIONS) + else + fallback_suggestions(name, names) + end + end + + # Fallback matcher for a runtime without `did_you_mean`: keep names that share a + # prefix/substring with the target or are within a small edit distance, closest + # first. + def fallback_suggestions(name, names) + target = name.downcase + matches = names.select { |candidate| near_miss?(candidate.downcase, target) } + matches.sort_by { |candidate| levenshtein(candidate.downcase, target) }.first(MAX_SUGGESTIONS) + end + + def near_miss?(candidate, target) + candidate.start_with?(target) || target.start_with?(candidate) || + candidate.include?(target) || levenshtein(candidate, target) <= 2 + end + + # Iterative Levenshtein edit distance (two-row DP) — small dictionaries, so an + # exact distance is cheap and keeps the fallback dependency-free. + def levenshtein(source, target) + return target.length if source.empty? + return source.length if target.empty? + + previous = (0..target.length).to_a + source.each_char.with_index do |source_char, row| + current = [row + 1] + target.each_char.with_index do |target_char, col| + cost = source_char == target_char ? 0 : 1 + current << [previous[col + 1] + 1, current[col] + 1, previous[col] + cost].min + end + previous = current + end + previous.last + end + + def quote_join(names) + names.map { |name| "\"#{name}\"" }.join(", ") + end end diff --git a/lib/mcp_toolkit/resource_schema.rb b/lib/mcp_toolkit/resource_schema.rb index b7b6d51..a4a5ac4 100644 --- a/lib/mcp_toolkit/resource_schema.rb +++ b/lib/mcp_toolkit/resource_schema.rb @@ -20,14 +20,23 @@ class McpToolkit::ResourceSchema }.freeze COMPUTED_TYPE = "computed" STANDARD_FILTERS = %w[ids updated_since limit offset].freeze + # Attribute names, in priority order, treated as a target resource's + # human-readable label. The first one a target actually serializes is surfaced + # as a `target_name_attribute` hint on the relationship (best-effort; omitted if + # the target declares none of them). + NAME_ATTRIBUTE_CANDIDATES = %i[name title label subject display_name].freeze - def self.call(resource) - new(resource).call + # `registry` is used to resolve each relationship to the registered resource it + # points at (see #relationships). Defaults to the process-wide registry; the + # `resource_schema` tool passes the active config's registry explicitly. + def self.call(resource, registry: McpToolkit.registry) + new(resource, registry:).call end - def initialize(resource) + def initialize(resource, registry: McpToolkit.registry) @resource = resource @model = resource.model + @registry = registry end def call @@ -43,7 +52,7 @@ def call private - attr_reader :resource, :model + attr_reader :resource, :model, :registry def attributes resource.attribute_names.map { |name| attribute_schema(name) } @@ -85,13 +94,76 @@ def filterable_column_for(attribute_name) end def relationships - resource.association_descriptors.map do |association| - { - name: association.links_key, - kind: association.type.to_s, - polymorphic: association.polymorphic || false - } + resource.association_descriptors.map { |association| relationship_schema(association) } + end + + # One relationship entry. Beyond the link key/kind/polymorphic flag it now also + # names the `target_resource` — the registered resource this link resolves to, + # callable via `list`/`get` — so e.g. a `scheduled_notifications.notification` + # link is discoverably the `notifications` resource rather than a name to guess. + # `target_name_attribute` is a best-effort hint at that resource's human-readable + # field. Both are omitted (additive/backward-compatible) when unresolved. + def relationship_schema(association) + target = target_resource_for(association) + { + name: association.links_key, + kind: association.type.to_s, + polymorphic: association.polymorphic || false, + target_resource: target&.name, + target_name_attribute: name_attribute_for(target) + }.compact + end + + # The registered resource an association points at, or nil. Polymorphic links + # have no single target, so they are left unresolved (the `polymorphic` flag and + # the record's `{id:, type:}` link value already carry the target type). Prefers + # an explicit target serializer's model, then falls back to matching the link's + # (pluralized) name against registered resource names. + def target_resource_for(association) + return nil if association.polymorphic + + target_from_serializer(association) || target_from_name(association) + end + + def target_from_serializer(association) + serializer = association.serializer + return nil unless serializer.respond_to?(:model_class) + + target_model = safe_model_class(serializer) + return nil unless target_model + + registry.resources.find { |candidate| candidate.model == target_model } + end + + def target_from_name(association) + candidate_names(association).each do |candidate| + match = registry.find(candidate) + return match if match end + nil + end + + # Names to try against the registry for a link, closest spelling first: the link + # key and association name as declared, then their pluralizations (resources are + # registered under plural names, so a singular `notification` link resolves to + # the `notifications` resource). + def candidate_names(association) + [association.links_key, association.name.to_s].flat_map do |base| + [base, base.pluralize] + end.uniq + end + + def safe_model_class(serializer) + serializer.model_class + rescue StandardError + nil + end + + def name_attribute_for(target) + return nil unless target + + names = target.attribute_names + NAME_ATTRIBUTE_CANDIDATES.find { |candidate| names.include?(candidate) }&.to_s end # Reads an attribute's DB column type, tolerating models that don't expose diff --git a/lib/mcp_toolkit/tools/resource_schema.rb b/lib/mcp_toolkit/tools/resource_schema.rb index 6350620..ea59576 100644 --- a/lib/mcp_toolkit/tools/resource_schema.rb +++ b/lib/mcp_toolkit/tools/resource_schema.rb @@ -8,7 +8,9 @@ class McpToolkit::Tools::ResourceSchema < McpToolkit::Tools::Base Describe a single read-only resource in detail. Pass the resource name as `resource` (use the `resources` tool to discover names). Returns: - attributes: every field in the response, each with its `type` and a value `format` hint - - relationships: associated resources emitted in the record's `links` + - relationships: associated resources emitted in the record's `links`; each names the + `target_resource` it resolves to (callable via `list`/`get`) plus, when known, a + `target_name_attribute` hint of that resource's human-readable field - standard_filters: ids, updated_since, limit, offset (accepted by the `list` tool) - filters: the per-attribute equality filter keys the `list` tool accepts The `attributes` and `relationships` names are also the valid values for the `fields` sparse @@ -31,7 +33,7 @@ def self.call(server_context:, resource: nil, **_args) # of THIS resource's shape (and an unknown resource is a clean tool error). descriptor = resolve_descriptor(resource, config) with_authentication(server_context, required_scope: config.registry.required_scope_for(descriptor)) do - McpToolkit::ResourceSchema.call(descriptor) + McpToolkit::ResourceSchema.call(descriptor, registry: config.registry) end rescue McpToolkit::Errors::InvalidParams => e error_response("Invalid request: #{e.message}") diff --git a/lib/mcp_toolkit/transport/controller_methods.rb b/lib/mcp_toolkit/transport/controller_methods.rb index 3fdb958..68df788 100644 --- a/lib/mcp_toolkit/transport/controller_methods.rb +++ b/lib/mcp_toolkit/transport/controller_methods.rb @@ -102,6 +102,14 @@ def mcp_extra_tools [] end + # 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) + + nil + end + # ---- per-request context -------------------------------------------- # Per-request context threaded to the tools. The gem merges the request's @@ -190,10 +198,28 @@ def mcp_render_unauthorized(message) end def mcp_render_session_not_found + mcp_log_session_not_found render json: { jsonrpc: "2.0", id: nil, error: { code: -32_001, message: "Session not found or expired" } }, status: :not_found end + + # Warns (greppable, no id/token) when a POST arrives with no matching session. + # The common cause is a session created on one process but looked up on another + # because `cache_store` isn't a shared store — invisible otherwise, since the + # caller just sees a 404. Records only whether a session-id header was PRESENT so + # a header-missing client bug is distinguishable from a cache misconfiguration. + def mcp_log_session_not_found + logger = mcp_logger + return unless logger + + header_present = request.headers[SESSION_HEADER].present? + logger.warn( + "[McpToolkit] MCP session not found or expired " \ + "(#{SESSION_HEADER} header present: #{header_present}). If sessions are created but not " \ + "found, cache_store is likely not shared across processes (set it to a shared store, e.g. Rails.cache)." + ) + end end diff --git a/lib/mcp_toolkit/version.rb b/lib/mcp_toolkit/version.rb index e713196..7bb52ea 100644 --- a/lib/mcp_toolkit/version.rb +++ b/lib/mcp_toolkit/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module McpToolkit - VERSION = "0.2.0" + VERSION = "0.3.0" end diff --git a/spec/mcp_toolkit/registry_and_executors_spec.rb b/spec/mcp_toolkit/registry_and_executors_spec.rb index cbcea67..d8e8d3f 100644 --- a/spec/mcp_toolkit/registry_and_executors_spec.rb +++ b/spec/mcp_toolkit/registry_and_executors_spec.rb @@ -331,6 +331,90 @@ def self.serialize_collection(records, scope: nil, total_count: nil, limit: nil, ] ) end + + # The DX gap this closes: a `scheduled_notifications.notification` link is + # discoverably the `notifications` resource (callable via list/get) rather than + # a name the caller has to guess. + context "relationship target resource resolution" do + let(:notification_model) do + Class.new do + def self.columns_hash + { "id" => FakeRelation::Column.new(:integer), "name" => FakeRelation::Column.new(:string) } + end + def self.primary_key = "id" + def self.model_name = FakeModelName.new("notifications") + end + end + + let(:notification_serializer) do + model = notification_model + Class.new(McpToolkit::Serializer::Base) do + attributes :id, :name + self.model_class = model + def self.name = "NotificationSerializer" + end + end + + let(:scheduled_notification_serializer) do + Class.new(McpToolkit::Serializer::Base) do + attributes :id + has_one :notification # singular link -> plural `notifications` resource + has_many :orphans # no registered target + has_one :subject_record, polymorphic: true + self.model_class = Object + def self.name = "ScheduledNotificationSerializer" + end + end + + before do + n_model = notification_model + n_serializer = notification_serializer + sn_serializer = scheduled_notification_serializer + McpToolkit.configure do |c| + c.registry.register(:notifications) do + model n_model + serializer n_serializer + description "Notifications." + scope { |_root| [] } + end + c.registry.register(:scheduled_notifications) do + model Object + serializer sn_serializer + description "Scheduled notifications." + scope { |_root| [] } + end + end + end + + subject(:schema) do + described_class.call(McpToolkit.registry.fetch("scheduled_notifications"), registry: McpToolkit.registry) + end + + it "names the target resource a singular link resolves to, with a name-attribute hint" do + relationship = schema[:relationships].find { |r| r[:name] == "notification" } + + expect(relationship).to include( + name: "notification", + kind: "has_one", + polymorphic: false, + target_resource: "notifications", + target_name_attribute: "name" + ) + end + + it "omits target fields (additive/backward-compatible) when no resource matches" do + relationship = schema[:relationships].find { |r| r[:name] == "orphans" } + + expect(relationship.keys).to contain_exactly(:name, :kind, :polymorphic) + end + + it "leaves a polymorphic link unresolved (no single target resource)" do + relationship = schema[:relationships].find { |r| r[:name] == "subject_record" } + + expect(relationship).to include(polymorphic: true) + expect(relationship).not_to have_key(:target_resource) + end + end end describe McpToolkit::Registry do @@ -338,6 +422,27 @@ def self.serialize_collection(records, scope: nil, total_count: nil, limit: nil, expect { McpToolkit.registry.fetch("nope") }.to raise_error(McpToolkit::Registry::UnknownResource) end + it "suggests the closest registered name on a near-miss (did you mean)" do + expect { McpToolkit.registry.fetch("widget") }.to raise_error( + McpToolkit::Registry::UnknownResource, /Did you mean "widgets"\?/ + ) + end + + it "lists the registered resources when the catalog is short" do + expect { McpToolkit.registry.fetch("zzz") }.to raise_error( + McpToolkit::Registry::UnknownResource, /Registered resources: "widgets"\./ + ) + end + + it "keeps its edit-distance fallback working without did_you_mean" do + registry = McpToolkit::Registry.new + registry.register(:notifications) { description "n" } + registry.register(:scheduled_notifications) { description "s" } + + expect(registry.send(:fallback_suggestions, "notification", registry.resource_names)) + .to include("notifications") + end + describe "required scope resolution" do subject(:registry) { McpToolkit::Registry.new } diff --git a/spec/mcp_toolkit/transport/controller_methods_spec.rb b/spec/mcp_toolkit/transport/controller_methods_spec.rb new file mode 100644 index 0000000..d5b890e --- /dev/null +++ b/spec/mcp_toolkit/transport/controller_methods_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require "spec_helper" +require "logger" +require "stringio" + +# The transport concern runs WITHOUT Rails in the gem's suite, so this exercises +# the session-not-found path against a minimal host class that provides only the +# class hooks the concern's `included do` block invokes (`before_action`; CSRF is +# guarded behind respond_to? and thus skipped). `request` / `response` / `render` +# are stubbed to the slice the method touches. +RSpec.describe McpToolkit::Transport::ControllerMethods do + let(:log_output) { StringIO.new } + let(:logger) { Logger.new(log_output) } + + let(:controller_class) do + Class.new do + def self.before_action(*); end + include McpToolkit::Transport::ControllerMethods + + attr_accessor :session_header_value + attr_reader :rendered + + def request + headers = { McpToolkit::Transport::ControllerMethods::SESSION_HEADER => session_header_value } + Struct.new(:headers).new(headers) + end + + def render(*args) + @rendered = args + end + end + end + + let(:controller) { controller_class.new } + + describe "#mcp_render_session_not_found logging (P3)" do + it "warns with a greppable, id-free message when a session is not found" do + controller.session_header_value = "sess-secret-123" + allow(controller).to receive(:mcp_logger).and_return(logger) + + controller.send(:mcp_render_session_not_found) + + expect(log_output.string).to include("[McpToolkit] MCP session not found or expired") + expect(log_output.string).to include("header present: true") + expect(log_output.string).to include("cache_store") + expect(log_output.string).not_to include("sess-secret-123") + end + + it "reports header present: false when no session-id header was sent" do + controller.session_header_value = nil + allow(controller).to receive(:mcp_logger).and_return(logger) + + controller.send(:mcp_render_session_not_found) + + expect(log_output.string).to include("header present: false") + end + + it "still renders the -32001 not-found JSON-RPC error after logging" do + controller.session_header_value = "sess-123" + allow(controller).to receive(:mcp_logger).and_return(logger) + + controller.send(:mcp_render_session_not_found) + + payload = controller.rendered.first + expect(payload).to include(status: :not_found) + expect(payload[:json][:error][:code]).to eq(-32_001) + end + + it "is a no-op (does not raise) when no logger is available, e.g. Rails absent" do + controller.session_header_value = "sess-123" + + expect { controller.send(:mcp_render_session_not_found) }.not_to raise_error + expect(controller.rendered.first).to include(status: :not_found) + end + end +end From 6af8e7f37622a217c41f39e26a4a4fd820be51ed Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 3 Jul 2026 11:13:27 +0200 Subject: [PATCH 2/3] Address PR #3 review: extract UnknownResourceMessage, simplify logger, 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 --- CHANGELOG.md | 7 +- lib/mcp_toolkit/registry.rb | 83 +------------------ lib/mcp_toolkit/resource_schema.rb | 19 +---- lib/mcp_toolkit/tools/resource_schema.rb | 3 +- .../transport/controller_methods.rb | 15 ++-- lib/mcp_toolkit/unknown_resource_message.rb | 75 +++++++++++++++++ .../registry_and_executors_spec.rb | 15 ++-- .../transport/controller_methods_spec.rb | 9 +- 8 files changed, 104 insertions(+), 122 deletions(-) create mode 100644 lib/mcp_toolkit/unknown_resource_message.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b8de0f7..1cea5c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,9 @@ - **Resource discovery DX** improvements, closing gaps hit when an MCP agent had to guess a related resource's name rather than discover it: - The `resource_schema` tool now names, for each relationship, the `target_resource` it - resolves to (callable via `list` / `get`) plus, when known, a `target_name_attribute` hint of - that resource's human-readable field. A `scheduled_notifications.notification` link is thus - discoverably the `notifications` resource instead of a name to guess. Additive and backward - compatible: both fields are omitted when the target can't be resolved (e.g. a polymorphic + resolves to (callable via `list` / `get`). A `scheduled_notifications.notification` link is + thus discoverably the `notifications` resource instead of a name to guess. Additive and + backward compatible: it is omitted when the target can't be resolved (e.g. a polymorphic link), so existing relationship consumers are unaffected. - An unknown resource name now raises a "did you mean" message — the closest registered name(s) via Ruby's stdlib `DidYouMean::SpellChecker` (with a dependency-free edit-distance diff --git a/lib/mcp_toolkit/registry.rb b/lib/mcp_toolkit/registry.rb index 410407a..3a5c301 100644 --- a/lib/mcp_toolkit/registry.rb +++ b/lib/mcp_toolkit/registry.rb @@ -1,15 +1,5 @@ # frozen_string_literal: true -# `did_you_mean` is a Ruby default gem (loaded at interpreter startup on stock -# builds), used to suggest the closest registered resource name on a typo. A -# trimmed runtime without it falls back to the small edit-distance matcher below, -# so the require is guarded rather than assumed. -begin - require "did_you_mean" -rescue LoadError - nil -end - # Central registry of read-only resources exposed via the MCP server. Resources # are registered at boot (in a `to_prepare` initializer) and consumed by the # generic `resources` / `resource_schema` / `get` / `list` tools. @@ -20,13 +10,6 @@ class McpToolkit::Registry class UnknownResource < StandardError; end - # When the catalog has at most this many resources, the UnknownResource message - # lists all of them (cheap for a caller to scan); above it, only the closest - # suggestions are offered so the message stays short. - FULL_LIST_MAX = 10 - # Cap on the number of "did you mean" suggestions offered for a near-miss name. - MAX_SUGGESTIONS = 3 - def initialize @resources = {} @default_required_permissions_scope = nil @@ -60,7 +43,7 @@ def required_scope_for(resource) end def fetch(name) - find(name) or raise(UnknownResource, unknown_resource_message(name)) + find(name) or raise(UnknownResource, McpToolkit::UnknownResourceMessage.new(name, resource_names).build) end def find(name) @@ -81,68 +64,4 @@ def resource_names def reset! @resources = {} end - - private - - # Builds the UnknownResource message: states the bad name, then (a) suggests the - # 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) - message = "unknown resource: #{name.inspect}" - names = resource_names - return message if names.empty? - - suggestions = suggestions_for(name.to_s, names) - message += ". Did you mean #{quote_join(suggestions)}?" if suggestions.any? - message += " Registered resources: #{quote_join(names.sort)}." if names.size <= FULL_LIST_MAX - message - end - - # Closest registered names to a near-miss. Uses Ruby's stdlib DidYouMean spell - # checker when available (typo/transposition aware), else the prefix/substring + - # edit-distance fallback below. - def suggestions_for(name, names) - if defined?(DidYouMean::SpellChecker) - Array(DidYouMean::SpellChecker.new(dictionary: names).correct(name)).first(MAX_SUGGESTIONS) - else - fallback_suggestions(name, names) - end - end - - # Fallback matcher for a runtime without `did_you_mean`: keep names that share a - # prefix/substring with the target or are within a small edit distance, closest - # first. - def fallback_suggestions(name, names) - target = name.downcase - matches = names.select { |candidate| near_miss?(candidate.downcase, target) } - matches.sort_by { |candidate| levenshtein(candidate.downcase, target) }.first(MAX_SUGGESTIONS) - end - - def near_miss?(candidate, target) - candidate.start_with?(target) || target.start_with?(candidate) || - candidate.include?(target) || levenshtein(candidate, target) <= 2 - end - - # Iterative Levenshtein edit distance (two-row DP) — small dictionaries, so an - # exact distance is cheap and keeps the fallback dependency-free. - def levenshtein(source, target) - return target.length if source.empty? - return source.length if target.empty? - - previous = (0..target.length).to_a - source.each_char.with_index do |source_char, row| - current = [row + 1] - target.each_char.with_index do |target_char, col| - cost = source_char == target_char ? 0 : 1 - current << [previous[col + 1] + 1, current[col] + 1, previous[col] + cost].min - end - previous = current - end - previous.last - end - - def quote_join(names) - names.map { |name| "\"#{name}\"" }.join(", ") - end end diff --git a/lib/mcp_toolkit/resource_schema.rb b/lib/mcp_toolkit/resource_schema.rb index a4a5ac4..8eb880e 100644 --- a/lib/mcp_toolkit/resource_schema.rb +++ b/lib/mcp_toolkit/resource_schema.rb @@ -20,11 +20,6 @@ class McpToolkit::ResourceSchema }.freeze COMPUTED_TYPE = "computed" STANDARD_FILTERS = %w[ids updated_since limit offset].freeze - # Attribute names, in priority order, treated as a target resource's - # human-readable label. The first one a target actually serializes is surfaced - # as a `target_name_attribute` hint on the relationship (best-effort; omitted if - # the target declares none of them). - NAME_ATTRIBUTE_CANDIDATES = %i[name title label subject display_name].freeze # `registry` is used to resolve each relationship to the registered resource it # points at (see #relationships). Defaults to the process-wide registry; the @@ -101,16 +96,15 @@ def relationships # names the `target_resource` — the registered resource this link resolves to, # callable via `list`/`get` — so e.g. a `scheduled_notifications.notification` # link is discoverably the `notifications` resource rather than a name to guess. - # `target_name_attribute` is a best-effort hint at that resource's human-readable - # field. Both are omitted (additive/backward-compatible) when unresolved. + # It is omitted (additive/backward-compatible) when the target can't be resolved + # (e.g. a polymorphic link). def relationship_schema(association) target = target_resource_for(association) { name: association.links_key, kind: association.type.to_s, polymorphic: association.polymorphic || false, - target_resource: target&.name, - target_name_attribute: name_attribute_for(target) + target_resource: target&.name }.compact end @@ -159,13 +153,6 @@ def safe_model_class(serializer) nil end - def name_attribute_for(target) - return nil unless target - - names = target.attribute_names - NAME_ATTRIBUTE_CANDIDATES.find { |candidate| names.include?(candidate) }&.to_s - end - # Reads an attribute's DB column type, tolerating models that don't expose # `columns_hash` (returns nil => the attribute is reported as "computed"). def column_type(name) diff --git a/lib/mcp_toolkit/tools/resource_schema.rb b/lib/mcp_toolkit/tools/resource_schema.rb index ea59576..cdd43c6 100644 --- a/lib/mcp_toolkit/tools/resource_schema.rb +++ b/lib/mcp_toolkit/tools/resource_schema.rb @@ -9,8 +9,7 @@ class McpToolkit::Tools::ResourceSchema < McpToolkit::Tools::Base the `resources` tool to discover names). Returns: - attributes: every field in the response, each with its `type` and a value `format` hint - relationships: associated resources emitted in the record's `links`; each names the - `target_resource` it resolves to (callable via `list`/`get`) plus, when known, a - `target_name_attribute` hint of that resource's human-readable field + `target_resource` it resolves to (callable via `list`/`get`) - standard_filters: ids, updated_since, limit, offset (accepted by the `list` tool) - filters: the per-attribute equality filter keys the `list` tool accepts The `attributes` and `relationships` names are also the valid values for the `fields` sparse diff --git a/lib/mcp_toolkit/transport/controller_methods.rb b/lib/mcp_toolkit/transport/controller_methods.rb index 68df788..f5e004f 100644 --- a/lib/mcp_toolkit/transport/controller_methods.rb +++ b/lib/mcp_toolkit/transport/controller_methods.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "logger" + # The MCP Streamable-HTTP transport, provided as an includable concern. An # app's controller includes this to get the full transport with no per-app code: # @@ -103,11 +105,13 @@ def mcp_extra_tools end # Logger for transport-level diagnostics. Defaults to Rails.logger when running - # inside Rails (nil outside it); overridable so a host can inject its own. + # inside Rails, and to a $stdout logger otherwise (e.g. the gem's own unit suite) + # so a diagnostic is never silently dropped. Overridable so a host can inject its + # own; never returns nil. def mcp_logger - return Rails.logger if defined?(Rails) && Rails.respond_to?(:logger) + return Rails.logger if defined?(Rails) - nil + @mcp_logger ||= Logger.new($stdout) end # ---- per-request context -------------------------------------------- @@ -212,11 +216,8 @@ def mcp_render_session_not_found # caller just sees a 404. Records only whether a session-id header was PRESENT so # a header-missing client bug is distinguishable from a cache misconfiguration. def mcp_log_session_not_found - logger = mcp_logger - return unless logger - header_present = request.headers[SESSION_HEADER].present? - logger.warn( + mcp_logger.warn( "[McpToolkit] MCP session not found or expired " \ "(#{SESSION_HEADER} header present: #{header_present}). If sessions are created but not " \ "found, cache_store is likely not shared across processes (set it to a shared store, e.g. Rails.cache)." diff --git a/lib/mcp_toolkit/unknown_resource_message.rb b/lib/mcp_toolkit/unknown_resource_message.rb new file mode 100644 index 0000000..1805682 --- /dev/null +++ b/lib/mcp_toolkit/unknown_resource_message.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +begin + require "did_you_mean" +rescue LoadError + nil +end + +# Builds the McpToolkit::Registry::UnknownResource message from a bad name and the +# registered resource names: it states the bad name, then (a) suggests the nearest +# registered name(s) for a near-miss — via Ruby's stdlib DidYouMean spell checker +# when available, else a dependency-free prefix/substring + edit-distance fallback — +# 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 + FULL_LIST_MAX = 10 + MAX_SUGGESTIONS = 3 + + def initialize(name, resource_names) + @name = name + @resource_names = resource_names + end + + def build + message = "unknown resource: #{@name.inspect}" + return message if @resource_names.empty? + + suggestions = suggestions_for(@name.to_s, @resource_names) + message += ". Did you mean #{quote_join(suggestions)}?" if suggestions.any? + message += " Registered resources: #{quote_join(@resource_names.sort)}." if @resource_names.size <= FULL_LIST_MAX + message + end + + private + + def suggestions_for(name, names) + if defined?(DidYouMean::SpellChecker) + Array(DidYouMean::SpellChecker.new(dictionary: names).correct(name)).first(MAX_SUGGESTIONS) + else + fallback_suggestions(name, names) + end + end + + def fallback_suggestions(name, names) + target = name.downcase + matches = names.select { |candidate| near_miss?(candidate.downcase, target) } + matches.sort_by { |candidate| levenshtein(candidate.downcase, target) }.first(MAX_SUGGESTIONS) + end + + def near_miss?(candidate, target) + candidate.start_with?(target) || target.start_with?(candidate) || + candidate.include?(target) || levenshtein(candidate, target) <= 2 + end + + def levenshtein(source, target) + return target.length if source.empty? + return source.length if target.empty? + + previous = (0..target.length).to_a + source.each_char.with_index do |source_char, row| + current = [row + 1] + target.each_char.with_index do |target_char, col| + cost = source_char == target_char ? 0 : 1 + current << [previous[col + 1] + 1, current[col] + 1, previous[col] + cost].min + end + previous = current + end + previous.last + end + + def quote_join(names) + names.map { |name| "\"#{name}\"" }.join(", ") + end +end diff --git a/spec/mcp_toolkit/registry_and_executors_spec.rb b/spec/mcp_toolkit/registry_and_executors_spec.rb index d8e8d3f..5be4894 100644 --- a/spec/mcp_toolkit/registry_and_executors_spec.rb +++ b/spec/mcp_toolkit/registry_and_executors_spec.rb @@ -390,15 +390,14 @@ def self.name = "ScheduledNotificationSerializer" described_class.call(McpToolkit.registry.fetch("scheduled_notifications"), registry: McpToolkit.registry) end - it "names the target resource a singular link resolves to, with a name-attribute hint" do + it "names the target resource a singular link resolves to" do relationship = schema[:relationships].find { |r| r[:name] == "notification" } expect(relationship).to include( name: "notification", kind: "has_one", polymorphic: false, - target_resource: "notifications", - target_name_attribute: "name" + target_resource: "notifications" ) end @@ -434,13 +433,11 @@ def self.name = "ScheduledNotificationSerializer" ) end - it "keeps its edit-distance fallback working without did_you_mean" do - registry = McpToolkit::Registry.new - registry.register(:notifications) { description "n" } - registry.register(:scheduled_notifications) { description "s" } + it "suggests a near-miss via UnknownResourceMessage's edit-distance fallback (no did_you_mean)" do + resource_names = %w[notifications scheduled_notifications] + message = McpToolkit::UnknownResourceMessage.new("notification", resource_names) - expect(registry.send(:fallback_suggestions, "notification", registry.resource_names)) - .to include("notifications") + expect(message.send(:fallback_suggestions, "notification", resource_names)).to include("notifications") end describe "required scope resolution" do diff --git a/spec/mcp_toolkit/transport/controller_methods_spec.rb b/spec/mcp_toolkit/transport/controller_methods_spec.rb index d5b890e..c001df9 100644 --- a/spec/mcp_toolkit/transport/controller_methods_spec.rb +++ b/spec/mcp_toolkit/transport/controller_methods_spec.rb @@ -67,10 +67,15 @@ def render(*args) expect(payload[:json][:error][:code]).to eq(-32_001) end - it "is a no-op (does not raise) when no logger is available, e.g. Rails absent" do + it "falls back to a $stdout logger (never nil) when Rails is absent, and still renders" do + # A sibling spec loads `rails/version` (partial Rails, no `.logger`) into this + # process, so hide the constant to deterministically exercise the Rails-absent + # branch regardless of load order. + hide_const("Rails") controller.session_header_value = "sess-123" - expect { controller.send(:mcp_render_session_not_found) }.not_to raise_error + expect { controller.send(:mcp_render_session_not_found) } + .to output(/\[McpToolkit\] MCP session not found or expired/).to_stdout expect(controller.rendered.first).to include(status: :not_found) end end From e1dcbba9edf4325e5c29bfbd6483ce275886a4c1 Mon Sep 17 00:00:00 2001 From: Karol Galanciak Date: Fri, 3 Jul 2026 11:34:45 +0200 Subject: [PATCH 3/3] Add spec coverage for McpToolkit::UnknownResourceMessage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../unknown_resource_message_spec.rb | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 spec/mcp_toolkit/unknown_resource_message_spec.rb diff --git a/spec/mcp_toolkit/unknown_resource_message_spec.rb b/spec/mcp_toolkit/unknown_resource_message_spec.rb new file mode 100644 index 0000000..08b2e5c --- /dev/null +++ b/spec/mcp_toolkit/unknown_resource_message_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe McpToolkit::UnknownResourceMessage do + def build(name, resource_names) + described_class.new(name, resource_names).build + end + + it "states the bad name alone when the registry is empty" do + expect(build("widgets", [])).to eq('unknown resource: "widgets"') + end + + it "suggests the nearest registered name on a near-miss" do + expect(build("widget", %w[widgets gadgets])).to include('Did you mean "widgets"?') + end + + it "lists all registered names, sorted, when the catalog is short" do + expect(build("zzz", %w[widgets gadgets])).to include('Registered resources: "gadgets", "widgets".') + end + + it "neither suggests nor lists for a large catalog with no near-miss" do + resource_names = (1..12).map { |n| "resource_#{n}" } + + message = build("zzzzz", resource_names) + + expect(message).to eq('unknown resource: "zzzzz"') + expect(message).not_to include("Registered resources:") + expect(message).not_to include("Did you mean") + end + + it "finds a near-miss via the dependency-free edit-distance fallback" do + resource_names = %w[notifications scheduled_notifications] + message = described_class.new("notification", resource_names) + + expect(message.send(:fallback_suggestions, "notification", resource_names)).to include("notifications") + end +end