Apply PR #1 review: scopes-only auth, Zeitwerk autoloading, API v3 filtering, flat declarations

- Authorization is now purely scope-based: drop authorized_for_application?,
  the :applications introspection field, and the app-reach check in the
  authenticator + Tools::Base#with_authentication. Net effect unchanged for
  valid traffic (scope check alone still enforces <app>__<action>).
- Authority#introspection_payload emits scopes via Array(token.scopes) directly
  (token must always respond to #scopes; drop the respond_to? guard and the
  applications field).
- Set up Zeitwerk (Zeitwerk::Loader.for_gem) and remove all manual
  require/require_relative of gem files; version.rb stays eagerly required for
  the gemspec; MCPToolkit alias preserved.
- Port API v3 complex hash filtering into list_executor/resource via
  McpToolkit::Filtering: bare value = equality (comma => IN), { op:, value: }
  hash = operator filter, array of those = ANDed range; operators validated
  per column type; allowlist-safe (only filterable keys).
- Order by :id only when the primary key is numeric, else :created_at.
- Use flat (compact) leaf class declarations; GetExecutor uses attr_reader.
- Remove all provenance/'extracted from' comments.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Azdaroth

.rubocop.yml

@@ -18,6 +18,19 @@ Layout/LineLength:
 
 # --- Style ------------------------------------------------------------------
 
+# Prefer FLAT (compact) leaf declarations — `class McpToolkit::Auth::Authenticator`
+# — over the `module ... module ... class` pyramid. Files that genuinely define
+# several constants under one namespace (so the namespace must be opened as a
+# block) are excluded; there compact is not expressible.
+Style/ClassAndModuleChildren:
+  EnforcedStyle: compact
+  Exclude:
+    - "lib/mcp_toolkit.rb"
+    - "lib/mcp_toolkit/errors.rb"
+    - "lib/mcp_toolkit/filtering.rb"
+    - "lib/mcp_toolkit/serializer/base.rb"
+    - "lib/mcp_toolkit/tools/base.rb"
+
 # The gem (gemspec, Gemfile, lib, specs) is written with double-quoted strings;
 # match that convention instead of churning every literal.
 Style/StringLiterals:

lib/mcp_toolkit.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
-# ActiveSupport core extensions the extracted code relies on. Required up front
-# (not full Rails) so the gem works in any host: blank?/presence, deep_symbolize_keys,
+require "zeitwerk"
+
+# ActiveSupport core extensions the toolkit relies on. Required up front (not full
+# Rails) so the gem works in any host: blank?/presence, deep_symbolize_keys,
 # Array.wrap, compact_blank, iso8601 on Time/DateTime.
 require "active_support"
 require "active_support/core_ext/object/blank"
@@ -11,32 +13,15 @@
 require "active_support/core_ext/time/conversions"
 require "active_support/core_ext/date_time/conversions"
 
+# 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.
 require_relative "mcp_toolkit/version"
-require_relative "mcp_toolkit/errors"
-
-# Load order matters: Registry + Serializer::Base are referenced by Configuration
-# (the registry eagerly, the serializer base lazily).
-require_relative "mcp_toolkit/registry"
-require_relative "mcp_toolkit/resource"
-require_relative "mcp_toolkit/serializer/base"
-require_relative "mcp_toolkit/configuration"
-
-# Executors + schema (pure Ruby, no Rails needed to load).
-require_relative "mcp_toolkit/list_executor"
-require_relative "mcp_toolkit/get_executor"
-require_relative "mcp_toolkit/resource_schema"
-
-# Auth: satellite (introspection + authenticator) and authority.
-require_relative "mcp_toolkit/auth/introspection"
-require_relative "mcp_toolkit/auth/authenticator"
-require_relative "mcp_toolkit/auth/authority"
-
-# Server + tools wrap the official `mcp` gem.
-require_relative "mcp_toolkit/server"
 
-# Transport (Streamable-HTTP controller concern) + cache-backed session.
-require_relative "mcp_toolkit/session"
-require_relative "mcp_toolkit/transport/controller_methods"
+loader = Zeitwerk::Loader.for_gem
+# `version.rb` is loaded manually above; let Zeitwerk ignore it so it doesn't try
+# to manage the already-defined constant.
+loader.ignore("#{__dir__}/mcp_toolkit/version.rb")
+loader.setup
 
 # The toolkit for building account-scoped, read-only MCP servers on top of the
 # official `mcp` gem. See README.md for the satellite + authority quickstarts.

lib/mcp_toolkit/auth/authenticator.rb

@@ -1,98 +1,90 @@
 # frozen_string_literal: true
 
-module McpToolkit
-  module Auth
-    # SATELLITE side. Resolves the authenticated, scoped context for a tool call:
-    #
-    #   1. Introspect the bearer token against the central app (cached).
-    #   2. Reject if invalid / expired / not scoped to `required_application`.
-    #   3. Resolve the active central account id, enforcing tenancy:
-    #        - accounts_user token  => its single bound `account_id`. A supplied
-    #          selector, if present, MUST match it.
-    #        - user (superuser) token => the selector is REQUIRED and MUST be one of
-    #          the token's `account_ids`.
-    #   4. Map that central account id to the LOCAL scope root via
-    #      `config.account_resolver` (e.g. Account.find_by(synced_id:)) and return
-    #      it as the tools' `scope_root`.
-    #
-    # The account selector mirrors what a gateway forwards: the resolved account id
-    # arrives as `_meta[config.account_meta_key]`. We also accept an `account_id`
-    # tool argument and the `config.account_id_header` header as fallbacks.
-    #
-    # Extracted from bsa-notifications' `McpServer::Authenticator`, made
-    # config-driven (account mapping + meta/header keys come from McpToolkit.config).
-    class Authenticator
-      Context = Struct.new(:scope_root, :introspection, keyword_init: true)
+# SATELLITE side. Resolves the authenticated, scoped context for a tool call:
+#
+#   1. Introspect the bearer token against the central app (cached).
+#   2. Reject if invalid / expired.
+#   3. Resolve the active central account id, enforcing tenancy:
+#        - accounts_user token  => its single bound `account_id`. A supplied
+#          selector, if present, MUST match it.
+#        - user (superuser) token => the selector is REQUIRED and MUST be one of
+#          the token's `account_ids`.
+#   4. Map that central account id to the LOCAL scope root via
+#      `config.account_resolver` (e.g. Account.find_by(synced_id:)) and return
+#      it as the tools' `scope_root`.
+#
+# The exact `<app>__<action>` scope is enforced separately by Tools::Base
+# (#with_account / #with_authentication) via `authorized_for_scope?`; the
+# authenticator only validates the token and resolves the tenant.
+#
+# The account selector mirrors what a gateway forwards: the resolved account id
+# arrives as `_meta[config.account_meta_key]`. We also accept an `account_id`
+# tool argument and the `config.account_id_header` header as fallbacks.
+class McpToolkit::Auth::Authenticator
+  Context = Struct.new(:scope_root, :introspection, keyword_init: true)
 
-      # @param token [String] the plaintext bearer the central app forwarded
-      # @param meta [Hash] the JSON-RPC `_meta` (string or symbol keys)
-      # @param arguments [Hash] the tool-call arguments (may carry `account_id`)
-      # @param header_account_id [Integer,String,nil] the account-id header value
-      # @param config [McpToolkit::Configuration]
-      def self.call(token:, meta: {}, arguments: {}, header_account_id: nil, config: McpToolkit.config)
-        new(token:, meta:, arguments:, header_account_id:, config:).call
-      end
-
-      def initialize(token:, meta:, arguments:, header_account_id:, config: McpToolkit.config)
-        @token = token
-        @meta = (meta || {}).transform_keys(&:to_s)
-        @arguments = (arguments || {}).transform_keys(&:to_s)
-        @header_account_id = header_account_id
-        @config = config
-      end
+  # @param token [String] the plaintext bearer the central app forwarded
+  # @param meta [Hash] the JSON-RPC `_meta` (string or symbol keys)
+  # @param arguments [Hash] the tool-call arguments (may carry `account_id`)
+  # @param header_account_id [Integer,String,nil] the account-id header value
+  # @param config [McpToolkit::Configuration]
+  def self.call(token:, meta: {}, arguments: {}, header_account_id: nil, config: McpToolkit.config)
+    new(token:, meta:, arguments:, header_account_id:, config:).call
+  end
 
-      def call
-        introspection = McpToolkit::Auth::Introspection.call(@token, config: @config)
-        raise McpToolkit::Errors::Unauthorized, "invalid or expired token" unless introspection.valid?
+  def initialize(token:, meta:, arguments:, header_account_id:, config: McpToolkit.config)
+    @token = token
+    @meta = (meta || {}).transform_keys(&:to_s)
+    @arguments = (arguments || {}).transform_keys(&:to_s)
+    @header_account_id = header_account_id
+    @config = config
+  end
 
-        unless introspection.authorized_for_application?(@config.required_application)
-          raise McpToolkit::Errors::Unauthorized,
-                "token is not authorized for the #{@config.required_application.inspect} application"
-        end
+  def call
+    introspection = McpToolkit::Auth::Introspection.call(@token, config: @config)
+    raise McpToolkit::Errors::Unauthorized, "invalid or expired token" unless introspection.valid?
 
-        central_account_id = resolve_account_id(introspection)
-        scope_root = @config.account_resolver.call(central_account_id)
-        unless scope_root
-          raise McpToolkit::Errors::Unauthorized, "no local scope found for account_id=#{central_account_id}"
-        end
+    central_account_id = resolve_account_id(introspection)
+    scope_root = @config.account_resolver.call(central_account_id)
+    unless scope_root
+      raise McpToolkit::Errors::Unauthorized, "no local scope found for account_id=#{central_account_id}"
+    end
 
-        Context.new(scope_root:, introspection:)
-      end
+    Context.new(scope_root:, introspection:)
+  end
 
-      private
+  private
 
-      def resolve_account_id(introspection)
-        candidate = candidate_account_id
+  def resolve_account_id(introspection)
+    candidate = candidate_account_id
 
-        if introspection.accounts_user?
-          bound = introspection.account_id
-          if candidate.present? && candidate.to_i != bound.to_i
-            raise McpToolkit::Errors::Unauthorized,
-                  "account_id #{candidate} does not match this token's bound account"
-          end
+    if introspection.accounts_user?
+      bound = introspection.account_id
+      if candidate.present? && candidate.to_i != bound.to_i
+        raise McpToolkit::Errors::Unauthorized,
+              "account_id #{candidate} does not match this token's bound account"
+      end
 
-          return bound
-        end
+      return bound
+    end
 
-        # superuser / multi-account: selection is mandatory and must be authorized.
-        if candidate.blank?
-          raise McpToolkit::Errors::Unauthorized,
-                "this token spans multiple accounts; an account must be selected " \
-                "via _meta[\"#{@config.account_meta_key}\"] (or the account_id argument)"
-        end
+    # superuser / multi-account: selection is mandatory and must be authorized.
+    if candidate.blank?
+      raise McpToolkit::Errors::Unauthorized,
+            "this token spans multiple accounts; an account must be selected " \
+            "via _meta[\"#{@config.account_meta_key}\"] (or the account_id argument)"
+    end
 
-        unless introspection.authorized_account_ids.include?(candidate.to_i)
-          raise McpToolkit::Errors::Unauthorized, "account_id #{candidate} is not authorized for this token"
-        end
+    unless introspection.authorized_account_ids.include?(candidate.to_i)
+      raise McpToolkit::Errors::Unauthorized, "account_id #{candidate} is not authorized for this token"
+    end
 
-        candidate.to_i
-      end
+    candidate.to_i
+  end
 
-      def candidate_account_id
-        @meta[@config.account_meta_key].presence ||
-          @arguments["account_id"].presence ||
-          @header_account_id.presence
-      end
-    end
+  def candidate_account_id
+    @meta[@config.account_meta_key].presence ||
+      @arguments["account_id"].presence ||
+      @header_account_id.presence
   end
 end

lib/mcp_toolkit/auth/authority.rb

@@ -1,84 +1,77 @@
 # frozen_string_literal: true
 
-module McpToolkit
-  module Auth
-    # AUTHORITY side. The helpers the central app uses to (a) authenticate a
-    # plaintext bearer token against its local token store and (b) answer the
-    # introspection request satellites send.
-    #
-    # Both are thin and config-driven: the actual token lookup is the app's
-    # `config.token_authenticator` callable (its `McpToken.authenticate`
-    # equivalent), and the introspection payload is derived from the duck-typed
-    # token object that callable returns.
-    #
-    # ## Token object contract
-    #
-    # `config.token_authenticator.call(plaintext)` must return nil (no/invalid
-    # token) or an object responding to:
-    #
-    #   #kind            -> :accounts_user | :user (or string equivalents)
-    #   #account_id      -> the single bound account id for an accounts_user token,
-    #                       else nil
-    #   #account_ids     -> Array of authorized account ids
-    #   #expires_at      -> a Time/DateTime responding to #iso8601, or nil
-    #   #application_keys -> Array of application keys the token is scoped to ([] = unrestricted)
-    #   #scopes          -> Array of OAuth-style `<app>__<action>` scopes ([] = unrestricted).
-    #                       Optional; absent => emitted as []. Replaces application_keys
-    #                       as the authorization source on the satellite side.
-    #
-    # Optionally `#touch_last_used!` (called after a successful authenticate if
-    # present). A typical app token model (e.g. `McpToken`) satisfies this.
-    module Authority
-      module_function
+# AUTHORITY side. The helpers the central app uses to (a) authenticate a
+# plaintext bearer token against its local token store and (b) answer the
+# introspection request satellites send.
+#
+# Both are thin and config-driven: the actual token lookup is the app's
+# `config.token_authenticator` callable (its `McpToken.authenticate`
+# equivalent), and the introspection payload is derived from the duck-typed
+# token object that callable returns.
+#
+# ## Token object contract
+#
+# `config.token_authenticator.call(plaintext)` must return nil (no/invalid
+# token) or an object responding to:
+#
+#   #kind            -> :accounts_user | :user (or string equivalents)
+#   #account_id      -> the single bound account id for an accounts_user token,
+#                       else nil
+#   #account_ids     -> Array of authorized account ids
+#   #expires_at      -> a Time/DateTime responding to #iso8601, or nil
+#   #scopes          -> Array of OAuth-style `<app>__<action>` scopes ([] = unrestricted).
+#                       The sole authorization source on the satellite side.
+#
+# Optionally `#touch_last_used!` (called after a successful authenticate if
+# present). A typical app token model (e.g. `McpToken`) satisfies this.
+module McpToolkit::Auth::Authority
+  module_function
 
-      # Authenticate a plaintext bearer locally. Returns the token object or nil.
-      # Calls `touch_last_used!` on the token if it responds to it (throttled
-      # last-used tracking is the token model's concern, not ours).
-      def authenticate(plaintext, config: McpToolkit.config)
-        authenticator = config.token_authenticator
-        if authenticator.nil?
-          raise McpToolkit::Errors::ConfigurationError,
-                "token_authenticator is not configured; required for the :authority role"
-        end
+  # Authenticate a plaintext bearer locally. Returns the token object or nil.
+  # Calls `touch_last_used!` on the token if it responds to it (throttled
+  # last-used tracking is the token model's concern, not ours).
+  def authenticate(plaintext, config: McpToolkit.config)
+    authenticator = config.token_authenticator
+    if authenticator.nil?
+      raise McpToolkit::Errors::ConfigurationError,
+            "token_authenticator is not configured; required for the :authority role"
+    end
 
-        token = authenticator.call(plaintext)
-        return nil unless token
+    token = authenticator.call(plaintext)
+    return nil unless token
 
-        token.touch_last_used! if token.respond_to?(:touch_last_used!)
-        token
-      end
+    token.touch_last_used! if token.respond_to?(:touch_last_used!)
+    token
+  end
 
-      # Build the introspection response payload for a token object. This is the
-      # JSON the authority's `/mcp/tokens/introspect` endpoint renders, and the
-      # exact contract Auth::Introspection (the satellite) parses.
-      #
-      # @param token [#kind, #account_id, #account_ids, #expires_at, #application_keys]
-      # @return [Hash]
-      def introspection_payload(token)
-        account_ids = Array(token.account_ids)
-        {
-          valid: true,
-          kind: token.kind.to_s,
-          account_id: account_id_for(token, account_ids),
-          account_ids:,
-          expires_at: token.expires_at&.iso8601,
-          applications: Array(token.application_keys),
-          scopes: (token.respond_to?(:scopes) ? Array(token.scopes) : [])
-        }
-      end
+  # Build the introspection response payload for a token object. This is the
+  # JSON the authority's `/mcp/tokens/introspect` endpoint renders, and the
+  # exact contract Auth::Introspection (the satellite) parses.
+  #
+  # @param token [#kind, #account_id, #account_ids, #expires_at, #scopes]
+  # @return [Hash]
+  def introspection_payload(token)
+    account_ids = Array(token.account_ids)
+    {
+      valid: true,
+      kind: token.kind.to_s,
+      account_id: account_id_for(token, account_ids),
+      account_ids:,
+      expires_at: token.expires_at&.iso8601,
+      scopes: Array(token.scopes)
+    }
+  end
 
-      # The payload returned for a missing/invalid token. Render with HTTP 401.
-      def invalid_payload
-        { valid: false }
-      end
+  # The payload returned for a missing/invalid token. Render with HTTP 401.
+  def invalid_payload
+    { valid: false }
+  end
 
-      # account_id is the single bound account for an accounts_user token, else nil
-      # (a superuser/multi-account token advertises its set via account_ids).
-      def account_id_for(token, account_ids)
-        return token.account_id if token.respond_to?(:account_id) && token.account_id
+  # account_id is the single bound account for an accounts_user token, else nil
+  # (a superuser/multi-account token advertises its set via account_ids).
+  def account_id_for(token, account_ids)
+    return token.account_id if token.respond_to?(:account_id) && token.account_id
 
-        token.kind.to_s == "accounts_user" ? account_ids.first : nil
-      end
-    end
+    token.kind.to_s == "accounts_user" ? account_ids.first : nil
   end
 end