Build mcp_toolkit: extract shared MCP-server framework from the two S… by Azdaroth · Pull Request #1 · BookingSync/mcp_toolkit

Azdaroth · 2026-06-18T17:09:58Z

…mily apps

Wraps the official mcp gem as the JSON-RPC core. Provides a config object, registry + Resource DSL + generic list/get/resources/resource_schema tools over an injectable serializer, dual-role central-app auth (satellite introspection + authority responder), cache-backed sessions, and a Streamable-HTTP controller concern. 45 specs green; gem builds.

…mily apps Wraps the official mcp gem as the JSON-RPC core. Provides a config object, registry + Resource DSL + generic list/get/resources/resource_schema tools over an injectable serializer, dual-role central-app auth (satellite introspection + authority responder), cache-backed sessions, and a Streamable-HTTP controller concern. 45 specs green; gem builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace CircleCI with a GitHub Actions workflow (rspec matrix on Ruby 3.2/3.3/3.4, plus rubocop and brakeman jobs). Add a self-contained .rubocop.yml and the rubocop/rubocop-rspec/rubocop-performance/brakeman dev deps. Style-only autocorrects across lib (no behavior change); 45 specs, rubocop (0 offenses), and brakeman (0 warnings) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jXubCJm5nU87UaCsmQLpf

account_meta_key and account_id_header now default to neutral values (mcp-toolkit/account-id, X-MCP-Account-ID) instead of BookingSync-specific strings; they remain overridable per app. Scrubbed BookingSync references from code comments, README, and CHANGELOG (the gemspec homepage stays — it's the real repo URL). 45 specs, rubocop, and brakeman all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015jXubCJm5nU87UaCsmQLpf

Tools now require the exact "<required_application>_<scope_action>" scope on the introspected token. scope_action is a class-level DSL on Tools::Base defaulting to :read (inherited by subclasses; a write tool declares scope_action :write). Authorization derives from a new `scopes` field instead of `applications`: - Auth::Introspection::Result gains :scopes, #authorized_for_scope?, and #authorized_for_application? now derives app-reach from scopes. - Tools::Base enforces the exact scope in with_account and with_authentication. - Auth::Authority#introspection_payload emits scopes (from token.scopes when present, else []). `applications` retained for backward-compat, unused for auth. - NULL/empty token scopes remain unrestricted (backward-compat). Specs + CHANGELOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…on>) Distinguishes upstream/satellite-app scopes (e.g. notifications__read) from single-underscore core/bookingsync scopes (notifications_read), and matches the existing <app>__<tool> tool-namespacing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Azdaroth · 2026-06-24T13:50:26Z

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+module McpToolkit


do not use The
Nested
Module
Or
Class
Declarations

instead:

Make::The::Definition::Flat

Azdaroth · 2026-06-24T13:51:24Z

+    # 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


excessive comment that it was extracted from there

it applies to all files

Azdaroth · 2026-06-24T14:31:00Z

+          account_ids:,
+          expires_at: token.expires_at&.iso8601,
+          applications: Array(token.application_keys),
+          scopes: (token.respond_to?(:scopes) ? Array(token.scopes) : [])


it must always respond to scopes, respond_to? is not neded

Azdaroth · 2026-06-24T14:31:45Z

+          account_id: account_id_for(token, account_ids),
+          account_ids:,
+          expires_at: token.expires_at&.iso8601,
+          applications: Array(token.application_keys),


this should no longer be used, it's all about scopes for authorization

Azdaroth · 2026-06-24T14:32:13Z

+        # derived from the token's `scopes` of the form `<app>__<action>`: a token
+        # reaches an app if it carries any scope for that app. NULL/empty scopes =
+        # unrestricted (backward-compat).
+        def authorized_for_application?(required_application)


not needed anymore. it shoudd be sufficient to make a check just for the scopes.

Azdaroth · 2026-06-24T14:39:26Z

+module McpToolkit
+  # Runs a read-only, paginated "list" query for a registered resource, rooted on
+  # the scoped relation. Supports the standard `ids`, `updated_since`, `limit`,
+  # `offset` filters plus the resource's declared per-attribute equality filters,


only equality filters? What about complex filtering with sending a hash? was it backported?

Azdaroth · 2026-06-24T14:40:23Z

+      relation = apply_ids(relation)
+      relation = apply_updated_since(relation)
+      relation = apply_attribute_filters(relation)
+      relation.order(:id)


ordering by ID makes sense only if the primary key is numeric. if it's not, we should order by created_at

this needs to work overall like it does for API v3

Azdaroth · 2026-06-24T14:43:13Z

+module McpToolkit
+  # Descriptor for a single read-only resource exposed via the MCP server. Built
+  # via `McpToolkit.registry.register`; consumed by the List/Get executors and the
+  # resource_schema tool. Extracted from bsa-notifications' `McpServer::Resource`.


do not mention anywhere where it was extracted and how

Azdaroth · 2026-06-24T14:43:40Z

+# frozen_string_literal: true
+
+require "mcp"
+require_relative "tools/list"


do we need all these require statements? Use zeitwek properly

Azdaroth · 2026-06-24T14:44:13Z

+# (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"


almost certainly all these require statements can be removed by using Zeitwek properly

this stil lapplies, why do we need all these require statements here?

…ltering, 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>

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

…op compact-cop excludes Style/ClassAndModuleChildren compact was suppressed on 5 files via Exclude. Resolve each so the escape-hatch shrinks to just the gem entrypoint: - filtering.rb, serializer/base.rb, tools/base.rb: single leaf constant disguised as pyramid nesting -> rewritten compact (module/class McpToolkit::Foo::Bar). - errors.rb: genuinely 4 constants under one namespace -> split one-per-file under lib/mcp_toolkit/errors/ (base + invalid_params + unauthorized + configuration_error), each a compact subclass; the Errors namespace is now implicit from the directory. Subclasses rely on Zeitwerk autoloading the in-namespace Base (no requires added). - mcp_toolkit.rb: legitimately needs the module block (loader setup, top-level methods, MCPToolkit alias) -> stays the sole compact-cop exclude. No public API / behaviour / auth changes. Still zero internal requires; the split relies entirely on Zeitwerk autoloading. Add an eager_load spec to guard the flat + split layout against NameError regressions. Verified: rubocop 0 offenses, rspec 67 examples 0 failures, eager_load clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Azdaroth · 2026-06-25T11:07:03Z

+  # 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


do not use respond_to? as it doesn't make sense. if it's nil, just return early

Azdaroth · 2026-06-25T11:07:15Z

@@ -0,0 +1,173 @@
+# frozen_string_literal: true
+
+require "digest"


do we need these here if we are using Zeitwerk?

Azdaroth · 2026-06-25T11:08:27Z

+    public
+
+    def accounts_user?
+      kind.to_s == "accounts_user"


don't use hardcoded strings like this in any place. These constants should be in a single file and the used in collaborating objects . applies to "user" as well

Azdaroth · 2026-06-25T11:09:10Z

+
+  INVALID = Result.new(valid: false).freeze
+
+  class << self


don't use that style, use self.class_method_name

ideally, enforce by rubocop

Azdaroth · 2026-06-25T11:09:35Z

+    return cached if cached
+
+    result = fetch
+    cache.write(cache_key, result, expires_in: @config.introspection_cache_ttl)


don't use instance variable in any place like this provide proper attr_reader

Azdaroth · 2026-06-25T11:20:35Z

+    it "treats empty/absent token scopes as unrestricted (backward-compat)" do
+      stub_introspect(
+        token: "unrestricted",
+        body: { valid: true, kind: "user", account_id: nil, account_ids: [1], expires_at: nil, scopes: [] }


this could be wrong, but maybe it's a matter of a unit test

We need to make it scopes the following way:

if scopes is an empty array and the tool does not require any scopes - it's allowed

if scopes is an mpty array ad the tool requires some scope - it should not be allowed

Azdaroth · 2026-06-25T11:27:43Z

+        body: { valid: true, kind: "accounts_user", account_id: 42, account_ids: [42], scopes: ["notifications__read"] }
+      )
+
+      context = described_class.call(token: "au")


for all the specs in all the files in this gem - use named subject in every place.

Azdaroth · 2026-06-25T11:28:39Z

+# Integration-style specs (data path, server end-to-end, Zeitwerk eager loading)
+# intentionally describe a behaviour with a string rather than a single class.
+RSpec/DescribeClass:
+  Exclude:


exclude all specs

Azdaroth · 2026-06-25T11:28:59Z


+### Added
+
+- OAuth-style **scope** enforcement. Tokens now carry `scopes` of the form


add all notes under -0.1.0 version until it's released

Azdaroth · 2026-06-25T11:29:33Z

+
+# Convenience "everything" task for local runs. CI (GitHub Actions) keeps these
+# as separate jobs (test / rubocop / brakeman) so failures stay isolated.
+task default: %i[spec rubocop]


it's missing brakeman though

Cross-cutting style patterns: - Convert every `class << self` to `def self.x`; enforce via new Style/ClassMethodsDefinitions (def_self) RuboCop cop. - Convert `module_function` modules (Filtering, Auth::Authority, Server) to explicit `def self.x` methods. - Expose initialize-assigned ivars via private attr_reader and read through the reader (Introspection, Authenticator, ListExecutor, ResourceSchema, Resource non-DSL methods). - Extract token-kind / domain string literals into McpToolkit::TokenKinds. Object extractions: - Auth::AuthorityServerClient owns the Faraday connection + introspection POST; Introspection delegates to it (caching/parsing only). - Configuration#sql_sanitizer (default ActiveRecord-backed McpToolkit::SqlSanitizer, injectable) replaces the runtime `defined?(ActiveRecord::Base)` branch in Filtering. Behavior / correctness: - Scope authorization: a tool is allowed iff every required scope is present in the token's scopes. Empty token scopes are unrestricted ONLY for tools that require no scope; removed the empty-scopes-unrestricted backward-compat path. - account_id_for: drop respond_to?, guard on nil and read account_id directly. - get tool: accept a string/UUID record id (uncoerced through the finder); expose the entry point as `def self.call`. Requires / Zeitwerk: - Centralize stdlib/external requires in the entrypoint (faraday stays with its owner AuthorityServerClient); remove scattered per-file requires; audit and justify the active_support core_ext list. Doc / provenance cleanups: - Remove Prometheus and API-v3 provenance mentions; fix stale application_keys / IntrospectionPayload references. Config / tooling / docs: - .rubocop.yml: exclude spec/**/*, drop rubocop-rspec plugin + per-spec tuning, add the def_self cop. - Every spec file uses a named subject(:...); no bare unnamed subject. - CHANGELOG: fold [Unreleased] into a single [0.1.0] accumulation heading. - Rakefile: add a brakeman task; default = spec rubocop brakeman (CI parity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ression spec Reworks scope authorization and packages the satellite transport as a mountable Rails engine, bumping the unreleased gem to 0.2.0. Scope authorization is now declared EXPLICITLY per resource via `required_permissions_scope` (with a registry-wide `default_required_permissions_scope`), replacing the implicit `required_application` + `scope_action` derivation. A resource's effective scope is its own declaration, else the registry default, else none; the `list` / `get` / `resource_schema` tools resolve the resource first, then enforce its effective scope. Adds a mountable `McpToolkit::Engine` + gem-provided `McpToolkit::ServerController`, so a satellite writes `mount McpToolkit::Engine => "/mcp"` instead of four hand-declared routes and a controller. The controller's parent is configurable via `Configuration#parent_controller` (default `ActionController::Base`). The engine is additive — `Transport::ControllerMethods` stays a standalone concern — and both the engine and `app/controllers` load only when Rails is present, so non-Rails hosts and the unit suite never reference them. Engine fix: the four routes are drawn in the engine's `config/routes.rb`, NOT a class-body `routes.draw`. Rails' routes_reloader re-evaluates an engine's config/routes.rb on every route reload but never replays a class-body draw, so the old form was wiped on the first reload, leaving the engine route-less and every `/mcp` 404'ing (only real satellite boot caught this; the stubbed engine_spec did not). Adds a regression spec that boots a REAL minimal Rails::Application in an isolated subprocess, mounts the engine, forces `reload_routes!`, and asserts the engine route set still maps the four endpoints afterward. It fails on the old class-body form (empty route set) and passes on the config/routes.rb form — verified both ways. The subprocess keeps the real-Rails boot from contaminating the Rails-absent unit suite. `rails` is added as a test-only dev dependency (`require: false`); it is not a gem runtime dependency. Also requires `faraday` in auth_spec where `Faraday::ConnectionFailed` is referenced directly, fixing an order-dependent `uninitialized constant` flake (faraday is otherwise required lazily by Auth::AuthorityServerClient). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…engine reload spec: use Gem.ruby for the child interpreter to_i collapsed every non-numeric account id to 0, so a non-numeric selector could pass tenancy checks against an unrelated bound account, and a superuser selecting a string/UUID id resolved as 0 instead of the chosen id. Normalize all account-id comparisons (and the returned value) to strings; the resolver's find_by(synced_id:) coerces back per-column for integer and string ids alike. The engine route-reload spec booted its child via bare "bundle exec ruby", which under a Bundler-managed parent can resolve a system Ruby. Invoke the running interpreter directly via Gem.ruby and load the same bundle in the child (require "bundler/setup", BUNDLE_GEMFILE inherited). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Azdaroth · 2026-06-26T15:03:30Z

@@ -1,5 +1,79 @@
-## [Unreleased]
+## [0.2.0]


put everything under 0.1.0 versions, all the changes and change the release date to be 2026-06-28 there.

…0.1.0

Azdaroth

💪 🐉 Opa!

Azdaroth self-assigned this Jun 18, 2026

Azdaroth and others added 4 commits June 18, 2026 20:54

Azdaroth commented Jun 24, 2026

View reviewed changes

Azdaroth and others added 3 commits June 24, 2026 17:14

Enforce token expiry on the satellite (security C1)

26c003f

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

Azdaroth commented Jun 25, 2026

View reviewed changes

Azdaroth and others added 3 commits June 25, 2026 13:55

Azdaroth commented Jun 26, 2026

View reviewed changes

CHANGELOG: collapse changes under 0.1.0 (2026-06-28); set version to …

73b012e

…0.1.0

Azdaroth commented Jun 26, 2026

View reviewed changes

Azdaroth merged commit 5ad578a into master Jun 26, 2026
10 checks passed

		@@ -0,0 +1,98 @@
		# frozen_string_literal: true

		module McpToolkit

		@@ -0,0 +1,173 @@
		# frozen_string_literal: true

		require "digest"


		### Added

		- OAuth-style scope enforcement. Tokens now carry `scopes` of the form

Uh oh!

Conversation

Azdaroth commented Jun 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Azdaroth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant