From 5ba952a69351d7b65df1188d56dc435f5f4f0ef6 Mon Sep 17 00:00:00 2001 From: Alexander Chueshev Date: Mon, 22 Sep 2025 19:20:41 +0200 Subject: [PATCH] Introduce MCP tool versioning This allow declare different versions of MCP tool. The tools/call and tools/list only serves the latest version for now. Co-authored-by: Tan Le --- app/services/mcp/tools/api_tool.rb | 6 +- app/services/mcp/tools/base_service.rb | 4 + app/services/mcp/tools/custom_service.rb | 95 ++- .../mcp/tools/get_server_version_service.rb | 32 +- app/services/mcp/tools/manager.rb | 103 ++- app/services/mcp/tools/version_helper.rb | 13 + ee/app/services/ee/mcp/tools/manager.rb | 41 +- .../mcp/tools/search_codebase_service.rb | 65 +- ee/spec/services/ee/mcp/tools/manager_spec.rb | 153 +++-- .../mcp/tools/search_codebase_service_spec.rb | 167 ++++- lib/api/mcp/base.rb | 5 + lib/api/mcp/handlers/call_tool.rb | 18 +- lib/api/mcp/handlers/list_tools.rb | 2 +- spec/lib/api/mcp/handlers/call_tool_spec.rb | 64 ++ .../api/mcp/handlers/call_tool_spec.rb | 2 +- .../api/mcp/handlers/list_tools_spec.rb | 25 +- spec/services/mcp/tools/api_tool_spec.rb | 14 +- spec/services/mcp/tools/base_service_spec.rb | 10 + .../services/mcp/tools/custom_service_spec.rb | 590 ++++++++++++------ .../tools/get_server_version_service_spec.rb | 139 ++++- .../services/mcp/tools/version_helper_spec.rb | 49 ++ 21 files changed, 1235 insertions(+), 362 deletions(-) create mode 100644 app/services/mcp/tools/version_helper.rb create mode 100644 spec/lib/api/mcp/handlers/call_tool_spec.rb create mode 100644 spec/services/mcp/tools/version_helper_spec.rb diff --git a/app/services/mcp/tools/api_tool.rb b/app/services/mcp/tools/api_tool.rb index 3f08a92ed9adc3..f4c034ec9bd37f 100644 --- a/app/services/mcp/tools/api_tool.rb +++ b/app/services/mcp/tools/api_tool.rb @@ -3,7 +3,7 @@ module Mcp module Tools class ApiTool - attr_reader :route, :settings + attr_reader :name, :route, :settings, :version # Grape types are represented as a string by calling `.to_s` on a type # The values are built based on the existing routes: @@ -18,9 +18,11 @@ class ApiTool '[String]' => 'string' }.freeze - def initialize(route) + def initialize(name:, route:) + @name = name @route = route @settings = route.app.route_setting(:mcp) + @version = @settings[:version] || "0.1.0" end def description diff --git a/app/services/mcp/tools/base_service.rb b/app/services/mcp/tools/base_service.rb index 11faf605fc6b92..8723689f7cffc5 100644 --- a/app/services/mcp/tools/base_service.rb +++ b/app/services/mcp/tools/base_service.rb @@ -80,6 +80,10 @@ def description def input_schema raise NoMethodError end + + def version + raise NoMethodError + end end end end diff --git a/app/services/mcp/tools/custom_service.rb b/app/services/mcp/tools/custom_service.rb index a99edbd493e504..a09b9cd542629c 100644 --- a/app/services/mcp/tools/custom_service.rb +++ b/app/services/mcp/tools/custom_service.rb @@ -4,7 +4,69 @@ module Mcp module Tools class CustomService < BaseService - extend Gitlab::Utils::Override + extend ::Gitlab::Utils::Override + + class << self + # Register a new version with specific metadata + def register_version(version, metadata = {}) + @versions = {} unless defined?(@versions) + @versions[version] = metadata.freeze + end + + # Get the latest registered version + def latest_version + return unless @versions + + @versions.keys.max_by { |v| Gem::Version.new(v) } + end + + # Get all available versions + def available_versions + return [] unless @versions + + @versions.keys.sort_by { |v| Gem::Version.new(v) } + end + + # Check if a version exists + def version_exists?(version) + return false unless @versions + + @versions.key?(version) + end + + # Get version metadata + def version_metadata(version) + return {} unless @versions + + @versions[version] || {} + end + end + + attr_reader :requested_version + + def initialize(name:, version: nil) + super(name: name) + @requested_version = version || self.class.latest_version + + raise ArgumentError, "No versions registered for #{self.class.name}" if self.class.available_versions.empty? + + return if self.class.version_exists?(@requested_version) + + raise ArgumentError, "Version #{@requested_version} not found. " \ + "Available: #{self.class.available_versions.join(', ')}" + end + + def version + @requested_version + end + + def description + version_metadata.fetch(:description) { raise NoMethodError, "Description not defined for version #{version}" } + end + + def input_schema + version_metadata.fetch(:input_schema) { raise NoMethodError, "Input schema not defined for version #{version}" } + end override :set_cred def set_cred(current_user: nil, access_token: nil) @@ -13,7 +75,7 @@ def set_cred(current_user: nil, access_token: nil) end def execute(request: nil, params: nil) - return Response.error("CustomService: current_user is not set") unless current_user.present? + return Response.error("#{self.class.name}: current_user is not set") unless current_user.present? authorize!(params) @@ -58,6 +120,35 @@ def find_project(project_id) project end # rubocop: enable CodeReuse/ActiveRecord + + protected + + override :perform + def perform(arguments = {}) + method_name = "perform_#{version_method_suffix}" + + if respond_to?(method_name, true) + send(method_name, arguments) # rubocop:disable GitlabSecurity/PublicSend -- To map version with corresponding method + else + # Fallback to default implementation if version-specific method doesn't exist + perform_default(arguments) + end + end + + # Default implementation - can be overridden in subclasses + def perform_default(_arguments = {}) + raise NoMethodError, "No implementation found for version #{version}" + end + + private + + def version_metadata + self.class.version_metadata(version) + end + + def version_method_suffix + version.tr('.', '_') + end end end end diff --git a/app/services/mcp/tools/get_server_version_service.rb b/app/services/mcp/tools/get_server_version_service.rb index ad0aa353c27f00..2e1bdcfe4f0145 100644 --- a/app/services/mcp/tools/get_server_version_service.rb +++ b/app/services/mcp/tools/get_server_version_service.rb @@ -5,33 +5,35 @@ module Tools class GetServerVersionService < CustomService extend ::Gitlab::Utils::Override - override :description - def description - 'Get the current version of MCP server.' - end - - override :authorize! - def authorize!(_params) - true - end - - override :input_schema - def input_schema - { + # Register version 0.1.0 + register_version '0.1.0', { + description: 'Get the current version of MCP server.', + input_schema: { type: 'object', properties: {}, required: [] } + } + + override :authorize! + def authorize!(_params) + true end protected - override :perform - def perform(_arguments = {}) + # Version 0.1.0 implementation + def perform_0_1_0(_arguments = {}) data = { version: Gitlab::VERSION, revision: Gitlab.revision } formatted_content = [{ type: 'text', text: data[:version] }] ::Mcp::Tools::Response.success(formatted_content, data) end + + # Fallback to 0.1.0 behavior for any unimplemented versions + override :perform_default + def perform_default(arguments = {}) + perform_0_1_0(arguments) + end end end end diff --git a/app/services/mcp/tools/manager.rb b/app/services/mcp/tools/manager.rb index 6e16d22b824666..6716cb919ac17f 100644 --- a/app/services/mcp/tools/manager.rb +++ b/app/services/mcp/tools/manager.rb @@ -3,8 +3,41 @@ module Mcp module Tools class Manager + include VersionHelper + + class ToolNotFoundError < StandardError + attr_reader :tool_name + + def initialize(tool_name) + @tool_name = tool_name + super("Tool '#{tool_name}' not found.") + end + end + + class VersionNotFoundError < StandardError + attr_reader :tool_name, :requested_version, :available_versions + + def initialize(tool_name, requested_version, available_versions) + @tool_name = tool_name + @requested_version = requested_version + @available_versions = available_versions + super("Tool '#{tool_name}' version '#{requested_version}' not found. " \ + "Available versions: #{available_versions.join(', ')}") + end + end + + class InvalidVersionFormatError < StandardError + attr_reader :version + + def initialize(version) + @version = version + super("Invalid semantic version format: #{version}.") + end + end + + # Registry of all custom tools mapped to their service classes CUSTOM_TOOLS = { - 'get_mcp_server_version' => ::Mcp::Tools::GetServerVersionService.new(name: 'get_mcp_server_version') + 'get_mcp_server_version' => ::Mcp::Tools::GetServerVersionService }.freeze attr_reader :tools @@ -13,18 +46,74 @@ def initialize @tools = build_tools end + def list_tools + tools + end + + def get_tool(name:, version: nil) + raise InvalidVersionFormatError, version if version && !validate_semantic_version(version) + + return get_custom_tool(name, version) if CUSTOM_TOOLS.key?(name) + + return get_api_tool(name, version) if discover_api_tools.key?(name) + + raise ToolNotFoundError, name + end + private + def get_custom_tool(name, version) + tool_class = CUSTOM_TOOLS[name] + + unless version.nil? || tool_class.version_exists?(version) + available_versions = tool_class.available_versions + raise VersionNotFoundError.new(name, version, available_versions) + end + + tool_class.new(name: name, version: version) + end + + def get_api_tool(name, version) + api_tools = discover_api_tools + tool = api_tools[name] + api_tool_version = tool.version + + raise VersionNotFoundError.new(name, version, [api_tool_version]) if version && version != api_tool_version + + tool + end + def build_tools - api_tools = {} - ::API::API.routes.each do |route| - settings = route.app.route_setting(:mcp) - next if settings.blank? + tools = {} + + # Build custom tools using their latest versions + CUSTOM_TOOLS.each do |name, tool_class| + tools[name] = tool_class.new(name: name) + end - api_tools[settings[:tool_name].to_s] = Mcp::Tools::ApiTool.new(route) + # Include API tools (discovered from route_setting :mcp) + api_tools = discover_api_tools + api_tools.each do |name, tool| + tools[name] = tool end - { **api_tools, **CUSTOM_TOOLS } + tools + end + + def discover_api_tools + @api_tools ||= begin + api_tools = {} + ::API::API.routes.each do |route| + settings = route.app.route_setting(:mcp) + next if settings.blank? + + name = settings[:tool_name].to_s + + api_tools[name] = Mcp::Tools::ApiTool.new(name: name, route: route) + end + + api_tools.freeze + end end end end diff --git a/app/services/mcp/tools/version_helper.rb b/app/services/mcp/tools/version_helper.rb new file mode 100644 index 00000000000000..bc33c58978e616 --- /dev/null +++ b/app/services/mcp/tools/version_helper.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Mcp + module Tools + module VersionHelper + def validate_semantic_version(version) + return false if version.nil? || version.empty? + + ::Gitlab::Regex.semver_regex.match?(version.to_s) + end + end + end +end diff --git a/ee/app/services/ee/mcp/tools/manager.rb b/ee/app/services/ee/mcp/tools/manager.rb index fe0a35f4e1de95..e858fa34e1733d 100644 --- a/ee/app/services/ee/mcp/tools/manager.rb +++ b/ee/app/services/ee/mcp/tools/manager.rb @@ -6,14 +6,51 @@ module Tools module Manager extend ::Gitlab::Utils::Override + # Registry of all EE custom tools mapped to their service classes EE_CUSTOM_TOOLS = { - 'semantic_code_search' => ::Mcp::Tools::SearchCodebaseService.new(name: 'semantic_code_search') + 'semantic_code_search' => ::Mcp::Tools::SearchCodebaseService }.freeze + override :get_tool + def get_tool(name:, version: nil) + if version && !validate_semantic_version(version) + raise ::Mcp::Tools::Manager::InvalidVersionFormatError, version + end + + return get_ee_custom_tool(name, version) if EE_CUSTOM_TOOLS.key?(name) + + super + end + override :build_tools def build_tools ce_tools = super - { **ce_tools, **EE_CUSTOM_TOOLS } + ee_tools = build_ee_tools + { **ce_tools, **ee_tools } + end + + private + + def get_ee_custom_tool(name, version) + tool_class = EE_CUSTOM_TOOLS[name] + + unless version.nil? || tool_class.version_exists?(version) + available_versions = tool_class.available_versions + raise ::Mcp::Tools::Manager::VersionNotFoundError.new(name, version, available_versions) + end + + tool_class.new(name: name, version: version) + end + + def build_ee_tools + tools = {} + + # Build EE custom tools using their latest versions + EE_CUSTOM_TOOLS.each do |name, tool_class| + tools[name] = tool_class.new(name: name) + end + + tools end end end diff --git a/ee/app/services/mcp/tools/search_codebase_service.rb b/ee/app/services/mcp/tools/search_codebase_service.rb index 7e0b6b0416fb95..278c7609d7460f 100644 --- a/ee/app/services/mcp/tools/search_codebase_service.rb +++ b/ee/app/services/mcp/tools/search_codebase_service.rb @@ -9,39 +9,16 @@ class SearchCodebaseService < CustomService ACTIVE_CONTEXT_QUERY = ::Ai::ActiveContext::Queries REQUIRED_ABILITY = :read_code - override :ability - def auth_ability - REQUIRED_ABILITY - end - - override :auth_target - def auth_target(params) - project_id = params.dig(:arguments, :project_id) - - raise ArgumentError, "#{name}: project not found, the params received: #{params.inspect}" if project_id.nil? - - find_project(project_id) - end - - override :description - def description - desc = <<~DESC - Performs semantic code search across project files using vector similarity. - - Returns ranked code snippets with file paths and content matches based on natural language queries. - - Use this tool for questions about a project's codebase. - For example: "how something works" or "code that does X", or finding specific implementations. - - This tool supports directory scoping and configurable result limits for targeted code discovery and analysis. - DESC - desc.strip - end - - override :input_schema - def input_schema - { + # Register version 0.1.0 + register_version '0.1.0', { + description: "Performs semantic code search across project files using vector similarity.\n\n" \ + "Returns ranked code snippets with file paths and content matches based on natural language queries.\n\n" \ + "Use this tool for questions about a project's codebase.\n" \ + "For example: \"how something works\" or \"code that does X\", or finding specific implementations.\n\n" \ + "This tool supports directory scoping and configurable result limits for targeted code discovery and " \ + "analysis.", + input_schema: { type: 'object', properties: { semantic_query: { @@ -80,12 +57,26 @@ def input_schema required: %w[semantic_query project_id], additionalProperties: false } + } + + override :ability + def auth_ability + REQUIRED_ABILITY + end + + override :auth_target + def auth_target(params) + project_id = params.dig(:arguments, :project_id) + + raise ArgumentError, "#{name}: project not found, the params received: #{params.inspect}" if project_id.nil? + + find_project(project_id) end protected - override :perform - def perform(arguments = {}, query = {}) # rubocop:disable Lint/UnusedMethodArgument -- `query` is required by the contract + # Version 0.1.0 implementation + def perform_0_1_0(arguments = {}) limit = arguments[:limit] || 20 knn = arguments[:knn] || 64 semantic_query = arguments[:semantic_query] @@ -117,6 +108,12 @@ def perform(arguments = {}, query = {}) # rubocop:disable Lint/UnusedMethodArgum ::Mcp::Tools::Response.success(formatted_content, result.to_a) end + # Fallback to 0.1.0 behavior for any unimplemented versions + override :perform_default + def perform_default(arguments = {}) + perform_0_1_0(arguments) + end + private def failure_response(result, project_id) diff --git a/ee/spec/services/ee/mcp/tools/manager_spec.rb b/ee/spec/services/ee/mcp/tools/manager_spec.rb index e36f5b091d8e01..d298f25c3d273a 100644 --- a/ee/spec/services/ee/mcp/tools/manager_spec.rb +++ b/ee/spec/services/ee/mcp/tools/manager_spec.rb @@ -3,16 +3,23 @@ require 'spec_helper' RSpec.describe Mcp::Tools::Manager, feature_category: :ai_agents do - let(:get_mcp_server_version_service) { Mcp::Tools::GetServerVersionService.new(name: 'get_mcp_server_version') } - let(:semantic_code_search_service) { Mcp::Tools::SearchCodebaseService.new(name: 'semantic_code_search') } + let(:api_double) { class_double(API::API) } before do - stub_const("#{described_class}::CUSTOM_TOOLS", { 'get_mcp_server_version' => get_mcp_server_version_service }) - stub_const("EE::#{described_class}::EE_CUSTOM_TOOLS", { 'semantic_code_search' => semantic_code_search_service }) + # Stub the CE CUSTOM_TOOLS with only CE tools + ce_custom_tools = { + 'get_mcp_server_version' => Mcp::Tools::GetServerVersionService + } + stub_const("#{described_class}::CUSTOM_TOOLS", ce_custom_tools) + + # Stub the EE_CUSTOM_TOOLS with EE tools + ee_custom_tools = { + 'semantic_code_search' => Mcp::Tools::SearchCodebaseService + } + stub_const("EE::#{described_class}::EE_CUSTOM_TOOLS", ee_custom_tools) end describe '#initialize' do - let(:api_double) { class_double(API::API) } let(:routes) { [] } before do @@ -24,7 +31,6 @@ it 'initializes with only custom tools' do manager = described_class.new - expect(manager.tools).to eq(described_class::CUSTOM_TOOLS.merge(described_class::EE_CUSTOM_TOOLS)) expect(manager.tools.keys).to contain_exactly('get_mcp_server_version', 'semantic_code_search') end end @@ -35,16 +41,16 @@ let(:route1) { instance_double(Grape::Router::Route, app: app1) } let(:route2) { instance_double(Grape::Router::Route, app: app2) } let(:routes) { [route1, route2] } - let(:mcp_settings1) { { tool_name: :create_user, params: [:name, :email] } } - let(:mcp_settings2) { { tool_name: :delete_user, params: [:id] } } + let(:mcp_settings1) { { tool_name: :create_user, params: [:name, :email], version: '1.0.0' } } + let(:mcp_settings2) { { tool_name: :delete_user, params: [:id], version: '1.1.0' } } let(:api_tool1) { instance_double(Mcp::Tools::ApiTool) } let(:api_tool2) { instance_double(Mcp::Tools::ApiTool) } before do allow(app1).to receive(:route_setting).with(:mcp).and_return(mcp_settings1) allow(app2).to receive(:route_setting).with(:mcp).and_return(mcp_settings2) - allow(Mcp::Tools::ApiTool).to receive(:new).with(route1).and_return(api_tool1) - allow(Mcp::Tools::ApiTool).to receive(:new).with(route2).and_return(api_tool2) + allow(Mcp::Tools::ApiTool).to receive(:new).with(name: 'create_user', route: route1).and_return(api_tool1) + allow(Mcp::Tools::ApiTool).to receive(:new).with(name: 'delete_user', route: route2).and_return(api_tool2) end it 'creates ApiTool instances for routes with MCP settings' do @@ -53,8 +59,8 @@ expect(manager.tools).to include( 'create_user' => api_tool1, 'delete_user' => api_tool2, - 'get_mcp_server_version' => get_mcp_server_version_service, - 'semantic_code_search' => semantic_code_search_service + 'get_mcp_server_version' => be_a(Mcp::Tools::GetServerVersionService), + 'semantic_code_search' => be_a(Mcp::Tools::SearchCodebaseService) ) expect(manager.tools.size).to eq(4) end @@ -82,7 +88,7 @@ allow(app1).to receive(:route_setting).with(:mcp).and_return(mcp_settings1) allow(app2).to receive(:route_setting).with(:mcp).and_return(nil) allow(app3).to receive(:route_setting).with(:mcp).and_return({}) - allow(Mcp::Tools::ApiTool).to receive(:new).with(route1).and_return(api_tool1) + allow(Mcp::Tools::ApiTool).to receive(:new).with(name: 'valid_tool', route: route1).and_return(api_tool1) end it 'skips routes with blank MCP settings' do @@ -90,46 +96,105 @@ expect(manager.tools).to include( 'valid_tool' => api_tool1, - 'get_mcp_server_version' => get_mcp_server_version_service, - 'semantic_code_search' => semantic_code_search_service + 'get_mcp_server_version' => be_a(Mcp::Tools::GetServerVersionService), + 'semantic_code_search' => be_a(Mcp::Tools::SearchCodebaseService) ) expect(manager.tools.size).to eq(3) - expect(Mcp::Tools::ApiTool).to have_received(:new).once.with(route1) - expect(Mcp::Tools::ApiTool).not_to have_received(:new).with(route2) - expect(Mcp::Tools::ApiTool).not_to have_received(:new).with(route3) + expect(Mcp::Tools::ApiTool).to have_received(:new).once.with(name: 'valid_tool', route: route1) + expect(Mcp::Tools::ApiTool).not_to have_received(:new).with('route2', route2) + expect(Mcp::Tools::ApiTool).not_to have_received(:new).with('route3', route3) end end + end - context 'with mixed mcp and non-mcp routes' do - let(:app1) { instance_double(Grape::Endpoint) } - let(:app2) { instance_double(Grape::Endpoint) } - let(:app3) { instance_double(Grape::Endpoint) } - let(:route1) { instance_double(Grape::Router::Route, app: app1) } - let(:route2) { instance_double(Grape::Router::Route, app: app2) } - let(:route3) { instance_double(Grape::Router::Route, app: app3) } - let(:routes) { [route1, route2, route3] } - let(:mcp_settings1) { { tool_name: :first_tool, params: [:param1] } } - let(:mcp_settings3) { { tool_name: :third_tool, params: [:param3] } } - let(:api_tool1) { instance_double(Mcp::Tools::ApiTool) } - let(:api_tool3) { instance_double(Mcp::Tools::ApiTool) } + describe '#list_tools' do + it 'returns the tools hash' do + manager = described_class.new - before do - allow(app1).to receive(:route_setting).with(:mcp).and_return(mcp_settings1) - allow(app2).to receive(:route_setting).with(:mcp).and_return(nil) - allow(app3).to receive(:route_setting).with(:mcp).and_return(mcp_settings3) - allow(Mcp::Tools::ApiTool).to receive(:new).with(route1).and_return(api_tool1) - allow(Mcp::Tools::ApiTool).to receive(:new).with(route3).and_return(api_tool3) + expect(manager.list_tools).to eq(manager.tools) + end + end + + describe '#get_tool' do + let(:manager) { described_class.new } + + context 'with custom tool' do + context 'when requesting specific version' do + it 'returns the correct version' do + tool = manager.get_tool(name: 'get_mcp_server_version', version: '0.1.0') + + expect(tool).to be_a(Mcp::Tools::GetServerVersionService) + expect(tool.version).to eq('0.1.0') + end end - it 'only processes valid routes and merges with custom tools' do - manager = described_class.new + context 'when requesting latest version' do + it 'returns the latest version' do + tool = manager.get_tool(name: 'get_mcp_server_version') - expect(manager.tools).to eq( - 'first_tool' => api_tool1, - 'third_tool' => api_tool3, - 'get_mcp_server_version' => get_mcp_server_version_service, - 'semantic_code_search' => semantic_code_search_service - ) + expect(tool).to be_a(Mcp::Tools::GetServerVersionService) + expect(tool.version).to eq('0.1.0') + end + end + + context 'when requesting non-existent version' do + it 'raises VersionNotFoundError' do + expect { manager.get_tool(name: 'get_mcp_server_version', version: '99.99.99') } + .to raise_error(described_class::VersionNotFoundError) do |error| + expect(error.tool_name).to eq('get_mcp_server_version') + expect(error.requested_version).to eq('99.99.99') + expect(error.available_versions).to eq(['0.1.0']) + end + end + end + end + + context 'with EE custom tool' do + context 'when requesting specific version' do + it 'returns the correct version' do + tool = manager.get_tool(name: 'semantic_code_search', version: '0.1.0') + + expect(tool).to be_a(Mcp::Tools::SearchCodebaseService) + expect(tool.version).to eq('0.1.0') + end + end + + context 'when requesting latest version' do + it 'returns the latest version' do + tool = manager.get_tool(name: 'semantic_code_search') + + expect(tool).to be_a(Mcp::Tools::SearchCodebaseService) + expect(tool.version).to eq('0.1.0') + end + end + + context 'when requesting non-existent version' do + it 'raises VersionNotFoundError' do + expect { manager.get_tool(name: 'semantic_code_search', version: '99.99.99') } + .to raise_error(::Mcp::Tools::Manager::VersionNotFoundError) do |error| + expect(error.tool_name).to eq('semantic_code_search') + expect(error.requested_version).to eq('99.99.99') + expect(error.available_versions).to eq(['0.1.0']) + end + end + end + end + + context 'with non-existent tool' do + it 'raises ToolNotFoundError' do + expect { manager.get_tool(name: 'non_existent_tool') } + .to raise_error(described_class::ToolNotFoundError) do |error| + expect(error.tool_name).to eq('non_existent_tool') + end + end + end + + context 'with invalid version format' do + it 'raises InvalidVersionFormatError' do + expect { manager.get_tool(name: 'get_mcp_server_version', version: 'invalid-version') } + .to raise_error(described_class::InvalidVersionFormatError) do |error| + expect(error.version).to eq('invalid-version') + end end end end diff --git a/ee/spec/services/mcp/tools/search_codebase_service_spec.rb b/ee/spec/services/mcp/tools/search_codebase_service_spec.rb index 91ac80e523c851..ebef0ada087649 100644 --- a/ee/spec/services/mcp/tools/search_codebase_service_spec.rb +++ b/ee/spec/services/mcp/tools/search_codebase_service_spec.rb @@ -3,14 +3,115 @@ require 'spec_helper' RSpec.describe Mcp::Tools::SearchCodebaseService, feature_category: :mcp_server do - let(:service) { described_class.new(name: 'get_code_context') } + let(:service_name) { 'get_code_context' } let(:current_user) { create(:user) } let(:project) { create :project, :repository } let_it_be(:oauth_token) { 'test_token_123' } + let(:service) { described_class.new(name: service_name, version: '0.1.0') } + + describe 'version registration' do + it 'registers version 0.1.0' do + expect(described_class.version_exists?('0.1.0')).to be true + end + + it 'has 0.1.0 as the latest version' do + expect(described_class.latest_version).to eq('0.1.0') + end + + it 'returns available versions in order' do + expect(described_class.available_versions).to eq(['0.1.0']) + end + end + + describe 'version metadata' do + describe 'version 0.1.0' do + let(:metadata) { described_class.version_metadata('0.1.0') } + + it 'has correct description' do + expect(metadata[:description]).to eq <<~DESC.strip + Performs semantic code search across project files using vector similarity. + + Returns ranked code snippets with file paths and content matches based on natural language queries. + + Use this tool for questions about a project's codebase. + For example: "how something works" or "code that does X", or finding specific implementations. + + This tool supports directory scoping and configurable result limits for targeted code discovery and analysis. + DESC + end + + it 'has correct input schema' do + expect(metadata[:input_schema]).to eq({ + type: 'object', + properties: { + semantic_query: { + type: 'string', + minLength: 1, + maxLength: 1000, + description: "A brief natural language query about the code you want to find in the project " \ + "(e.g.: 'authentication middleware', 'database connection logic', or 'API error handling')." + }, + project_id: { + type: 'string', + description: 'Either a project id or project path.' + }, + directory_path: { + type: 'string', + minLength: 1, + maxLength: 100, + description: 'Optional directory path to scope the search (e.g., "app/services/").' + }, + knn: { + type: 'integer', + default: 64, + minimum: 1, + maximum: 100, + description: 'Number of nearest neighbors used internally. ' \ + "This controls search precision vs. speed - higher values find more diverse results but take longer." + }, + limit: { + type: 'integer', + default: 20, + minimum: 1, + maximum: 100, + description: 'Maximum number of results to return.' + } + }, + required: %w[semantic_query project_id], + additionalProperties: false + }) + end + end + end + + describe 'initialization' do + context 'when no version is specified' do + it 'uses the latest version' do + service = described_class.new(name: service_name) + expect(service.version).to eq('0.1.0') + end + end + + context 'when version 0.1.0 is specified' do + it 'uses version 0.1.0' do + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.version).to eq('0.1.0') + end + end + + context 'when invalid version is specified' do + it 'raises ArgumentError' do + expect { described_class.new(name: service_name, version: '1.0.0') } + .to raise_error(ArgumentError, 'Version 1.0.0 not found. Available: 0.1.0') + end + end + end + describe '#description' do it 'returns the correct description' do - expected_description = <<~DESC + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.description).to eq <<~DESC.strip Performs semantic code search across project files using vector similarity. Returns ranked code snippets with file paths and content matches based on natural language queries. @@ -20,33 +121,51 @@ This tool supports directory scoping and configurable result limits for targeted code discovery and analysis. DESC - - expect(service.description).to eq(expected_description.strip) end end describe '#input_schema' do it 'returns the expected JSON schema' do - schema = service.input_schema - - expect(schema[:type]).to eq('object') - expect(schema[:required]).to match_array(%w[semantic_query project_id]) - expect(schema[:additionalProperties]).to be false - - expect(schema[:properties][:semantic_query][:type]).to eq('string') - expect(schema[:properties][:semantic_query][:minLength]).to eq(1) - - expect(schema[:properties][:project_id][:type]).to eq('string') - - expect(schema[:properties][:directory_path][:type]).to eq('string') - - expect(schema[:properties][:knn][:type]).to eq('integer') - expect(schema[:properties][:knn][:default]).to eq(64) - expect(schema[:properties][:knn][:minimum]).to eq(1) - - expect(schema[:properties][:limit][:type]).to eq('integer') - expect(schema[:properties][:limit][:default]).to eq(20) - expect(schema[:properties][:limit][:minimum]).to eq(1) + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.input_schema).to eq({ + type: 'object', + properties: { + semantic_query: { + type: 'string', + minLength: 1, + maxLength: 1000, + description: "A brief natural language query about the code you want to find in the project " \ + "(e.g.: 'authentication middleware', 'database connection logic', or 'API error handling')." + }, + project_id: { + type: 'string', + description: 'Either a project id or project path.' + }, + directory_path: { + type: 'string', + minLength: 1, + maxLength: 100, + description: 'Optional directory path to scope the search (e.g., "app/services/").' + }, + knn: { + type: 'integer', + default: 64, + minimum: 1, + maximum: 100, + description: 'Number of nearest neighbors used internally. ' \ + "This controls search precision vs. speed - higher values find more diverse results but take longer." + }, + limit: { + type: 'integer', + default: 20, + minimum: 1, + maximum: 100, + description: 'Maximum number of results to return.' + } + }, + required: %w[semantic_query project_id], + additionalProperties: false + }) end end diff --git a/lib/api/mcp/base.rb b/lib/api/mcp/base.rb index d92be7eb9a6ad7..cf91d5394f7a7f 100644 --- a/lib/api/mcp/base.rb +++ b/lib/api/mcp/base.rb @@ -5,6 +5,7 @@ module Mcp class Base < ::API::Base include ::API::Helpers::HeadersHelpers include APIGuard + include ::Mcp::Tools::VersionHelper # JSON-RPC Specification # See: https://www.jsonrpc.org/specification @@ -24,6 +25,10 @@ class Base < ::API::Base invalid_params: { code: -32602, message: 'Invalid params' + }, + version_mismatch: { + code: -32001, + message: 'Version not supported' } # NOTE: Parse error code -32700 is unsupported due to 400 Bad Request returned by Workhorse }.freeze diff --git a/lib/api/mcp/handlers/call_tool.rb b/lib/api/mcp/handlers/call_tool.rb index 1e6dd93e87c3a3..ae8f1da922b18f 100644 --- a/lib/api/mcp/handlers/call_tool.rb +++ b/lib/api/mcp/handlers/call_tool.rb @@ -10,10 +10,15 @@ def initialize(manager) end def invoke(request, params, current_user = nil) - tool = find_tool!(params[:name]) + name = params[:name] - # only custom_service needs the current_user injected - tool.set_cred(current_user: current_user) if tool.is_a? ::Mcp::Tools::CustomService + begin + tool = manager.get_tool(name: name) + rescue ::Mcp::Tools::Manager::ToolNotFoundError => e + raise ArgumentError, e.message + end + + tool.set_cred(current_user: current_user) if tool.is_a?(::Mcp::Tools::CustomService) tool.execute(request: request, params: params) end @@ -21,13 +26,6 @@ def invoke(request, params, current_user = nil) private attr_reader :manager - - def find_tool!(name) - tool = manager.tools[name] - raise ArgumentError, 'name is unsupported' unless tool - - tool - end end end end diff --git a/lib/api/mcp/handlers/list_tools.rb b/lib/api/mcp/handlers/list_tools.rb index d3596c5d4a4e73..ea4246b836d9aa 100644 --- a/lib/api/mcp/handlers/list_tools.rb +++ b/lib/api/mcp/handlers/list_tools.rb @@ -10,7 +10,7 @@ def initialize(manager) end def invoke(current_user) - tools_hash = manager.tools + tools_hash = manager.list_tools tools = tools_hash.filter_map do |name, tool| next nil if exclude_tool?(name, current_user) diff --git a/spec/lib/api/mcp/handlers/call_tool_spec.rb b/spec/lib/api/mcp/handlers/call_tool_spec.rb new file mode 100644 index 00000000000000..88055bb2d8f17d --- /dev/null +++ b/spec/lib/api/mcp/handlers/call_tool_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Mcp::Handlers::CallTool, feature_category: :mcp_server do + let(:manager) { instance_double(Mcp::Tools::Manager) } + let(:request) { instance_double(Rack::Request) } + let(:current_user) { instance_double(User) } + + subject(:handler) { described_class.new(manager) } + + describe '#invoke' do + let(:tool_name) { 'test_tool' } + let(:params) { { name: tool_name, arguments: { param: 'value' } } } + let(:tool) { instance_double(Mcp::Tools::BaseService) } + + context 'when tool is found and version matches' do + before do + allow(manager).to receive(:get_tool).with(name: tool_name).and_return(tool) + allow(tool).to receive(:is_a?).with(Mcp::Tools::CustomService).and_return(false) + allow(tool).to receive(:execute).and_return({ content: [{ type: 'text', text: 'Success' }] }) + end + + it 'executes the tool successfully' do + result = handler.invoke(request, params, current_user) + + expect(manager).to have_received(:get_tool).with(name: tool_name) + expect(tool).to have_received(:execute).with(request: request, params: params) + expect(result).to eq({ content: [{ type: 'text', text: 'Success' }] }) + end + end + + context 'when tool is a CustomService' do + let(:custom_tool) { instance_double(Mcp::Tools::CustomService) } + + before do + allow(manager).to receive(:get_tool).with(name: tool_name).and_return(custom_tool) + allow(custom_tool).to receive(:is_a?).with(Mcp::Tools::CustomService).and_return(true) + allow(custom_tool).to receive(:set_cred) + allow(custom_tool).to receive(:execute).and_return({ content: [{ type: 'text', text: 'Success' }] }) + end + + it 'sets credentials before executing' do + result = handler.invoke(request, params, current_user) + + expect(custom_tool).to have_received(:set_cred).with(current_user: current_user) + expect(custom_tool).to have_received(:execute).with(request: request, params: params) + expect(result).to eq({ content: [{ type: 'text', text: 'Success' }] }) + end + end + + context 'when tool is not found' do + before do + allow(manager).to receive(:get_tool).with(name: tool_name) + .and_raise(Mcp::Tools::Manager::ToolNotFoundError.new(tool_name)) + end + + it 'raises ArgumentError' do + expect { handler.invoke(request, params, current_user) } + .to raise_error(ArgumentError, "Tool '#{tool_name}' not found.") + end + end + end +end diff --git a/spec/requests/api/mcp/handlers/call_tool_spec.rb b/spec/requests/api/mcp/handlers/call_tool_spec.rb index 581f7d1d40891f..2dc92259c69d6b 100644 --- a/spec/requests/api/mcp/handlers/call_tool_spec.rb +++ b/spec/requests/api/mcp/handlers/call_tool_spec.rb @@ -107,7 +107,7 @@ it 'returns invalid params error' do expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']['code']).to eq(-32602) - expect(json_response['error']['data']['params']).to eq('name is unsupported') + expect(json_response['error']['data']['params']).to include("Tool 'unknown_tool' not found") end end end diff --git a/spec/requests/api/mcp/handlers/list_tools_spec.rb b/spec/requests/api/mcp/handlers/list_tools_spec.rb index ae55fdb4ac98c3..06491e1a885822 100644 --- a/spec/requests/api/mcp/handlers/list_tools_spec.rb +++ b/spec/requests/api/mcp/handlers/list_tools_spec.rb @@ -31,18 +31,19 @@ tools = json_response['result']['tools'] tool_names = tools.pluck('name') - expect(tool_names).to include( - 'get_pipeline_jobs', - 'gitlab_search', - 'get_issue', - 'create_issue', - 'create_merge_request', - 'get_merge_request', - 'get_merge_request_commits', - 'get_merge_request_diffs', - 'get_merge_request_pipelines', - 'get_mcp_server_version' - ) + expect(tool_names).to match_array(%w[ + get_pipeline_jobs + gitlab_search + get_issue + create_issue + create_merge_request + get_merge_request + get_merge_request_commits + get_merge_request_diffs + get_merge_request_pipelines + get_mcp_server_version + semantic_code_search + ]) end context 'when running with EE features', if: Gitlab.ee? do diff --git a/spec/services/mcp/tools/api_tool_spec.rb b/spec/services/mcp/tools/api_tool_spec.rb index 3e1cc05628cacd..9b2f528daf468d 100644 --- a/spec/services/mcp/tools/api_tool_spec.rb +++ b/spec/services/mcp/tools/api_tool_spec.rb @@ -26,12 +26,22 @@ allow(app).to receive(:route_setting).with(:mcp).and_return(mcp_settings) end - subject(:api_tool) { described_class.new(route) } + subject(:api_tool) { described_class.new(name: 'test_tool', route: route) } describe '#initialize' do - it 'sets the route and settings' do + it 'sets the name, route and settings' do + expect(api_tool.name).to eq('test_tool') expect(api_tool.route).to eq(route) expect(api_tool.settings).to eq(mcp_settings) + expect(api_tool.version).to eq('0.1.0') + end + + context 'when version is specified in settings' do + let(:mcp_settings) { { params: [:param1, :param2], version: '2.0.0' } } + + it 'uses the specified version' do + expect(api_tool.version).to eq('2.0.0') + end end end diff --git a/spec/services/mcp/tools/base_service_spec.rb b/spec/services/mcp/tools/base_service_spec.rb index 2cb671ad793b54..43506c9d868211 100644 --- a/spec/services/mcp/tools/base_service_spec.rb +++ b/spec/services/mcp/tools/base_service_spec.rb @@ -23,6 +23,10 @@ def input_schema } end + def version + '1.0.0' + end + protected def perform(arguments, _query = {}) @@ -48,6 +52,12 @@ def perform(arguments, _query = {}) end end + describe '#version' do + it 'raises NoMethodError' do + expect { service.version }.to raise_error(NoMethodError) + end + end + describe '#perform' do it 'raises NoMethodError' do expect { service.send(:perform, {}, {}) }.to raise_error(NoMethodError) diff --git a/spec/services/mcp/tools/custom_service_spec.rb b/spec/services/mcp/tools/custom_service_spec.rb index 230b22f2584011..2b85a7661e6326 100644 --- a/spec/services/mcp/tools/custom_service_spec.rb +++ b/spec/services/mcp/tools/custom_service_spec.rb @@ -2,292 +2,498 @@ require 'spec_helper' -RSpec.describe Mcp::Tools::CustomService, feature_category: :mcp_server do - let(:service_name) { 'test_api_tool' } +RSpec.describe Mcp::Tools::CustomService, :aggregate_failures, feature_category: :mcp_server do + let(:service_name) { 'test_custom_tool' } let(:current_user) { create(:user) } let(:project) { create :project, :repository } - describe '#format_response_content' do - let(:service) { described_class.new(name: service_name) } + # Create a test service class that inherits from CustomService + let(:test_service_class) do + Class.new(described_class) do + # Register version 1.0.0 + register_version '1.0.0', { + description: 'First version of test tool', + input_schema: { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + } + } + + # Register version 1.1.0 + register_version '1.1.0', { + description: 'Enhanced version of test tool', + input_schema: { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'integer' } + }, + required: ['name'] + } + } + + # Register version 2.0.0 + register_version '2.0.0', { + description: 'Major version with breaking changes', + input_schema: { + type: 'object', + properties: { + full_name: { type: 'string' }, + metadata: { type: 'object' } + }, + required: ['full_name'] + } + } + + def auth_ability + :read_code + end + + def auth_target(params) + project_id = params.dig(:arguments, :project_id) + find_project(project_id) + end + + protected + + def perform_1_0_0(arguments = {}) + ::Mcp::Tools::Response.success( + [{ type: 'text', text: "Hello #{arguments[:name]} (v1.0.0)" }], + { version: '1.0.0', name: arguments[:name] } + ) + end + + def perform_1_1_0(arguments = {}) + text = "Hello #{arguments[:name]}" + text += ", age #{arguments[:age]}" if arguments[:age] + text += " (v1.1.0)" - it 'raises NoMethodError' do - expect { service.send(:format_response_content, {}) }.to raise_error(NoMethodError) + ::Mcp::Tools::Response.success( + [{ type: 'text', text: text }], + { version: '1.1.0', name: arguments[:name], age: arguments[:age] } + ) + end + + def perform_2_0_0(arguments = {}) + ::Mcp::Tools::Response.success( + [{ type: 'text', text: "Hello #{arguments[:full_name]} (v2.0.0)" }], + { version: '2.0.0', full_name: arguments[:full_name], metadata: arguments[:metadata] } + ) + end + + def perform_default(arguments = {}) + ::Mcp::Tools::Response.success( + [{ type: 'text', text: "Default implementation with #{arguments}" }], + { version: 'default' } + ) + end end end - context 'when custom tool perform without error' do - before do - service.set_cred(access_token: nil, current_user: current_user) - project.add_developer(current_user) + describe '.register_version' do + it 'registers version metadata' do + expect(test_service_class.version_metadata('1.0.0')).to eq({ + description: 'First version of test tool', + input_schema: { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + } + }) end - let(:arguments) { { arguments: { project_id: project.id.to_s } } } - let(:service) { test_get_service_class.new(name: service_name) } - let(:test_get_service_class) do - Class.new(described_class) do - def description - 'Test custom tool' - end + it 'freezes the metadata' do + metadata = test_service_class.version_metadata('1.0.0') + expect(metadata).to be_frozen + end + end - def auth_ability - :read_code - end + describe '.latest_version' do + it 'returns the highest semantic version' do + expect(test_service_class.latest_version).to eq('2.0.0') + end - def auth_target(params) - project_id = params.dig(:arguments, :project_id) - find_project(project_id) - end + context 'when no versions are registered' do + let(:empty_service_class) { Class.new(described_class) } - def input_schema - { - type: 'object', - properties: {}, - required: [] - } - end + it 'returns nil' do + expect(empty_service_class.latest_version).to be_nil + end + end + end - protected + describe '.available_versions' do + it 'returns all versions sorted by semantic version' do + expect(test_service_class.available_versions).to eq(['1.0.0', '1.1.0', '2.0.0']) + end - def perform(_arguments = {}) - data = { version: "18.5.0-pre", revision: "2b34553de07" } - formatted_content = [{ type: 'text', text: data[:version] }] - ::Mcp::Tools::Response.success(formatted_content, data) - end + context 'when no versions are registered' do + let(:empty_service_class) { Class.new(described_class) } + + it 'returns empty array' do + expect(empty_service_class.available_versions).to eq([]) end end + end - let(:success) { true } - let(:response_code) { 200 } + describe '.version_exists?' do + it 'returns true for existing versions' do + expect(test_service_class.version_exists?('1.0.0')).to be true + expect(test_service_class.version_exists?('1.1.0')).to be true + expect(test_service_class.version_exists?('2.0.0')).to be true + end - it 'returns success response' do - result = service.execute(request: nil, params: arguments) + it 'returns false for non-existing versions' do + expect(test_service_class.version_exists?('0.9.0')).to be false + expect(test_service_class.version_exists?('3.0.0')).to be false + end - expect(result).to eq({ - content: [{ text: "18.5.0-pre", type: "text" }], - structuredContent: { revision: "2b34553de07", version: "18.5.0-pre" }, - isError: false - }) + context 'when no versions are registered' do + let(:empty_service_class) { Class.new(described_class) } + + it 'returns false' do + expect(empty_service_class.version_exists?('1.0.0')).to be false + end end end - context 'when custom tool perform with error' do - before do - service.set_cred(access_token: nil, current_user: current_user) - project.add_developer(current_user) + describe '.version_metadata' do + it 'returns metadata for existing versions' do + metadata = test_service_class.version_metadata('1.1.0') + expect(metadata[:description]).to eq('Enhanced version of test tool') + expect(metadata[:input_schema][:properties]).to have_key(:age) end - let(:arguments) { { arguments: { project_id: project.id.to_s } } } - let(:service) { test_get_service_class.new(name: service_name) } - let(:test_get_service_class) do - Class.new(described_class) do - def description - 'Test custom tool' - end + it 'returns empty hash for non-existing versions' do + expect(test_service_class.version_metadata('99.99.99')).to eq({}) + end - def auth_ability - :read_code - end + context 'when no versions are registered' do + let(:empty_service_class) { Class.new(described_class) } - def auth_target(params) - project_id = params.dig(:arguments, :project_id) - find_project(project_id) - end + it 'returns empty hash' do + expect(empty_service_class.version_metadata('1.0.0')).to eq({}) + end + end + end - def input_schema - { - type: 'object', - properties: {}, - required: [] - } - end + describe '#initialize' do + context 'when version is specified' do + it 'uses the specified version' do + service = test_service_class.new(name: service_name, version: '1.0.0') + expect(service.version).to eq('1.0.0') + end - protected + it 'raises error for non-existent version' do + expect { test_service_class.new(name: service_name, version: '99.99.99') } + .to raise_error(ArgumentError, 'Version 99.99.99 not found. Available: 1.0.0, 1.1.0, 2.0.0') + end + end - def perform(_arguments = {}) - raise StandardError, 'Something went wrong' - end + context 'when version is not specified' do + it 'uses the latest version' do + service = test_service_class.new(name: service_name) + expect(service.version).to eq('2.0.0') end end - let(:success) { true } - let(:response_code) { 200 } + context 'when no versions are registered' do + let(:empty_service_class) { Class.new(described_class) } - it 'returns Tool execution failed response' do - result = service.execute(request: nil, params: arguments) + it 'raises error' do + expect { empty_service_class.new(name: service_name) } + .to raise_error(ArgumentError, 'No versions registered for ') + end + end + end - expect(result).to eq({ - content: [{ text: "Tool execution failed: Something went wrong", type: "text" }], - structuredContent: {}, - isError: true - }) + describe '#version' do + it 'returns the requested version' do + service = test_service_class.new(name: service_name, version: '1.1.0') + expect(service.version).to eq('1.1.0') end end - context 'when authorize! failed for custom tools' do - let(:service) { test_get_service_class.new(name: service_name) } - let(:test_get_service_class) do - Class.new(described_class) do - def description - 'Test custom tool' - end + describe '#description' do + it 'returns description for the current version' do + service = test_service_class.new(name: service_name, version: '1.0.0') + expect(service.description).to eq('First version of test tool') + end - def auth_ability - :read_code - end + it 'raises error when description is not defined' do + service_class = Class.new(described_class) do + register_version '1.0.0', { input_schema: {} } + end - def auth_target(params) - project_id = params.dig(:arguments, :project_id) - find_project(project_id) - end + service = service_class.new(name: service_name, version: '1.0.0') + expect { service.description } + .to raise_error(NoMethodError, 'Description not defined for version 1.0.0') + end + end - def input_schema - { - type: 'object', - properties: {}, - required: [] - } - end + describe '#input_schema' do + it 'returns input schema for the current version' do + service = test_service_class.new(name: service_name, version: '1.1.0') + expected_schema = { + type: 'object', + properties: { + name: { type: 'string' }, + age: { type: 'integer' } + }, + required: ['name'] + } + expect(service.input_schema).to eq(expected_schema) + end + + it 'raises error when input schema is not defined' do + service_class = Class.new(described_class) do + register_version '1.0.0', { description: 'Test' } end + + service = service_class.new(name: service_name, version: '1.0.0') + expect { service.input_schema } + .to raise_error(NoMethodError, 'Input schema not defined for version 1.0.0') end + end - let(:success) { true } - let(:response_code) { 200 } - let(:current_user) { create(:user) } + describe '#execute' do + let(:service) { test_service_class.new(name: service_name, version: '1.0.0') } - context 'when current_user is not set' do - let(:arguments) { { arguments: {} } } + context 'when custom tool perform without error' do + let(:arguments) { { arguments: { project_id: project.id.to_s, name: 'Alice' } } } - it 'raise current_user is not set' do + before do + service.set_cred(access_token: nil, current_user: current_user) + project.add_developer(current_user) + end + + it 'returns success response' do result = service.execute(request: nil, params: arguments) + expect(result).to eq({ - content: [{ text: "CustomService: current_user is not set", type: "text" }], - structuredContent: {}, - isError: true + content: [{ text: "Hello Alice (v1.0.0)", type: "text" }], + structuredContent: { name: "Alice", version: "1.0.0" }, + isError: false }) end end - context 'when current_user is set' do + context 'when custom tool perform with error' do before do - service.set_cred(current_user: current_user) + service.set_cred(access_token: nil, current_user: current_user) + project.add_developer(current_user) + + allow(service).to receive(:perform).and_raise(StandardError, 'Something went wrong') end - context 'when user lacks permission' do - let(:arguments) { { arguments: { project_id: project.id.to_s } } } + let(:arguments) { { arguments: { project_id: project.id.to_s, name: 'Alice' } } } + + it 'returns Tool execution failed response' do + result = service.execute(request: nil, params: arguments) - it 'raises Gitlab::Access::AccessDeniedError' do + expect(result).to eq({ + content: [{ text: "Tool execution failed: Something went wrong", type: "text" }], + structuredContent: {}, + isError: true + }) + end + end + + context 'when authorize! failed for custom tools' do + let(:service) { test_service_class.new(name: service_name) } + let(:current_user) { create(:user) } + + context 'when current_user is not set' do + let(:arguments) { { arguments: {} } } + + it 'raise current_user is not set' do result = service.execute(request: nil, params: arguments) expect(result).to eq({ - content: [ - { - text: "Tool execution failed: CustomService: User #{current_user.id} " \ - "does not have permission to read_code for target #{project.id}", - type: "text" - } - ], + content: [{ text: ": current_user is not set", type: "text" }], structuredContent: {}, isError: true }) end end - end - context 'when auth_ability is not implemented' do - let(:test_get_service_class) do - Class.new(described_class) do - def description - 'Test custom tool' + context 'when current_user is set' do + before do + service.set_cred(current_user: current_user) + end + + context 'when user lacks permission' do + let(:arguments) { { arguments: { project_id: project.id.to_s } } } + + it 'raises Gitlab::Access::AccessDeniedError' do + result = service.execute(request: nil, params: arguments) + expect(result).to eq({ + content: [ + { + text: "Tool execution failed: CustomService: User #{current_user.id} " \ + "does not have permission to read_code for target #{project.id}", + type: "text" + } + ], + structuredContent: {}, + isError: true + }) + end + end + end + + context 'when auth_ability is not implemented' do + let(:test_service_class) do + Class.new(described_class) do + # Register version 1.0.0 + register_version '1.0.0', { + description: 'First version of test tool', + input_schema: { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + } + } + + def auth_target(params) + params.dig(:arguments, :project_id) + end end + end - def auth_target(params) - params.dig(:arguments, :project_id) + before do + service.set_cred(access_token: nil, current_user: current_user) + project.add_developer(current_user) + end + + context 'when current_user is not set' do + let(:arguments) { { arguments: { project_id: project.id.to_s } } } + + it 'raise current_user is not set' do + result = service.execute(request: nil, params: arguments) + expect(result).to eq({ + content: [ + { text: "Tool execution failed: #auth_ability should be implemented in a subclass", type: "text" } + ], + structuredContent: {}, + isError: true + }) end + end + end - def input_schema - { - type: 'object', - properties: {}, - required: [] + context 'when auth_target is not implemented' do + let(:test_service_class) do + Class.new(described_class) do + # Register version 1.0.0 + register_version '1.0.0', { + description: 'First version of test tool', + input_schema: { + type: 'object', + properties: { name: { type: 'string' } }, + required: ['name'] + } } + + def auth_ability + :read_code + end + end + end + + before do + service.set_cred(access_token: nil, current_user: current_user) + project.add_developer(current_user) + end + + context 'when current_user is not set' do + let(:arguments) { { arguments: { project_id: project.id.to_s } } } + + it 'raise current_user is not set' do + result = service.execute(request: nil, params: arguments) + expect(result).to eq({ + content: [ + { text: "Tool execution failed: #auth_target should be implemented in a subclass", type: "text" } + ], + structuredContent: {}, + isError: true + }) end end end + end + end - before do - service.set_cred(access_token: nil, current_user: current_user) - project.add_developer(current_user) + describe '#perform' do + context 'when version-specific method exists' do + it 'calls the correct version method' do + service = test_service_class.new(name: service_name, version: '1.0.0') + result = service.send(:perform, { name: 'Alice' }) + + expect(result[:content]).to match_array([{ type: 'text', text: 'Hello Alice (v1.0.0)' }]) + expect(result[:structuredContent][:version]).to eq('1.0.0') end - context 'when current_user is not set' do - let(:arguments) { { arguments: { project_id: project.id.to_s } } } + it 'handles different versions correctly' do + service_v1 = test_service_class.new(name: service_name, version: '1.1.0') + service_v2 = test_service_class.new(name: service_name, version: '2.0.0') - it 'raise current_user is not set' do - result = service.execute(request: nil, params: arguments) - expect(result).to eq({ - content: [ - { text: "Tool execution failed: #auth_ability should be implemented in a subclass", type: "text" } - ], - structuredContent: {}, - isError: true - }) - end + result_v1 = service_v1.send(:perform, { name: 'Bob', age: 25 }) + result_v2 = service_v2.send(:perform, { full_name: 'Bob Smith' }) + + expect(result_v1[:content].first[:text]).to eq('Hello Bob, age 25 (v1.1.0)') + expect(result_v2[:content].first[:text]).to eq('Hello Bob Smith (v2.0.0)') end end - context 'when auth_target is not implemented' do - let(:test_get_service_class) do + context 'when version-specific method does not exist' do + let(:service_without_method_class) do Class.new(described_class) do - def description - 'Test custom tool' - end - - def auth_ability - :read_code - end + register_version '3.0.0', { + description: 'Version without implementation', + input_schema: { type: 'object', properties: {}, required: [] } + } - def input_schema - { - type: 'object', - properties: {}, - required: [] - } + def perform_default(_arguments = {}) + ::Mcp::Tools::Response.success( + [{ type: 'text', text: 'Fallback implementation' }], + { fallback: true } + ) end end end - before do - service.set_cred(access_token: nil, current_user: current_user) - project.add_developer(current_user) - end + it 'calls perform_default' do + service = service_without_method_class.new(name: service_name, version: '3.0.0') + result = service.send(:perform, {}) - context 'when current_user is not set' do - let(:arguments) { { arguments: { project_id: project.id.to_s } } } + expect(result[:content]).to match_array([{ type: 'text', text: 'Fallback implementation' }]) + expect(result[:structuredContent][:fallback]).to be true + end + end - it 'raise current_user is not set' do - result = service.execute(request: nil, params: arguments) - expect(result).to eq({ - content: [ - { text: "Tool execution failed: #auth_target should be implemented in a subclass", type: "text" } - ], - structuredContent: {}, - isError: true - }) + context 'when neither version method nor default exists' do + let(:service_no_default_class) do + Class.new(described_class) do + register_version '4.0.0', { + description: 'Version without any implementation', + input_schema: { type: 'object', properties: {}, required: [] } + } end end + + it 'raises NoMethodError' do + service = service_no_default_class.new(name: service_name, version: '4.0.0') + expect { service.send(:perform, {}) } + .to raise_error(NoMethodError, 'No implementation found for version 4.0.0') + end end end describe '#find_project' do let_it_be(:namespace) { create(:group) } let_it_be(:project) { create(:project, :public, namespace: namespace) } - let(:service) { test_get_service_class.new(name: service_name) } - let(:test_get_service_class) do - Class.new(described_class) do - def description - 'Test custom tool' - end - end - end + let(:service) { test_service_class.new(name: service_name) } context 'with project ID' do it 'finds the project' do diff --git a/spec/services/mcp/tools/get_server_version_service_spec.rb b/spec/services/mcp/tools/get_server_version_service_spec.rb index de026c7b259918..8a6a826e6fe129 100644 --- a/spec/services/mcp/tools/get_server_version_service_spec.rb +++ b/spec/services/mcp/tools/get_server_version_service_spec.rb @@ -3,33 +3,144 @@ require 'spec_helper' RSpec.describe Mcp::Tools::GetServerVersionService, feature_category: :mcp_server do - let(:service) { described_class.new(name: 'get_mcp_server_version') } - let(:current_user) { create(:user) } - let_it_be(:oauth_token) { 'test_token_123' } + let(:service_name) { 'get_mcp_server_version' } + + describe 'version registration' do + it 'registers version 0.1.0' do + expect(described_class.version_exists?('0.1.0')).to be true + end + + it 'has 0.1.0 as the latest version' do + expect(described_class.latest_version).to eq('0.1.0') + end + + it 'returns available versions in order' do + expect(described_class.available_versions).to eq(['0.1.0']) + end + end + + describe 'version metadata' do + describe 'version 0.1.0' do + let(:metadata) { described_class.version_metadata('0.1.0') } + + it 'has correct description' do + expect(metadata[:description]).to eq('Get the current version of MCP server.') + end + + it 'has correct input schema' do + expect(metadata[:input_schema]).to eq({ + type: 'object', + properties: {}, + required: [] + }) + end + end + end + + describe 'initialization' do + context 'when no version is specified' do + it 'uses the latest version' do + service = described_class.new(name: service_name) + expect(service.version).to eq('0.1.0') + end + end + + context 'when version 0.1.0 is specified' do + it 'uses version 0.1.0' do + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.version).to eq('0.1.0') + end + end + + context 'when invalid version is specified' do + it 'raises ArgumentError' do + expect { described_class.new(name: service_name, version: '1.0.0') } + .to raise_error(ArgumentError, 'Version 1.0.0 not found. Available: 0.1.0') + end + end + end describe '#description' do - it 'returns the correct description' do - expect(service.description).to eq("Get the current version of MCP server.") + it 'returns correct description' do + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.description).to eq('Get the current version of MCP server.') end end describe '#input_schema' do - it 'returns the expected JSON schema' do - schema = service.input_schema - expect(schema[:type]).to eq('object') + it 'returns correct schema' do + service = described_class.new(name: service_name, version: '0.1.0') + expect(service.input_schema).to eq({ + type: 'object', + properties: {}, + required: [] + }) end end describe '#execute' do + let_it_be(:user) { build(:user) } + let_it_be(:oauth_token) { 'test_token_123' } + + let(:service) { described_class.new(name: service_name, version: '0.1.0') } + before do - service.set_cred(current_user: current_user, access_token: oauth_token) + service.set_cred(current_user: user, access_token: oauth_token) end - context 'with valid arguments' do - it 'returns correct gitlab version' do - response = service.execute(request: nil, params: {}) - expect(response[:isError]).to be false - expect(response[:content].first[:text]).to eq(Gitlab::VERSION) + context 'when using version 0.1.0' do + it 'returns GitLab version information' do + result = service.execute(params: { arguments: {} }) + + expect(result).to eq({ + content: [{ type: 'text', text: Gitlab::VERSION }], + structuredContent: { + version: Gitlab::VERSION, + revision: Gitlab.revision + }, + isError: false + }) + end + + it 'ignores arguments and returns version information' do + result = service.execute(params: { arguments: { 'include_revision' => true } }) + + expect(result).to eq({ + content: [{ type: 'text', text: Gitlab::VERSION }], + structuredContent: { + version: Gitlab::VERSION, + revision: Gitlab.revision + }, + isError: false + }) + end + end + + context 'when using default version behavior' do + it 'falls back to 0.1.0 behavior for the default version' do + result = service.execute(params: { arguments: {} }) + + expect(result).to eq({ + content: [{ type: 'text', text: Gitlab::VERSION }], + structuredContent: { + version: Gitlab::VERSION, + revision: Gitlab.revision + }, + isError: false + }) + end + end + + context 'when current_user is not set' do + it 'returns an error' do + service_without_user = described_class.new(name: service_name, version: '0.1.0') + result = service_without_user.execute(params: { arguments: {} }) + + expect(result).to eq({ + content: [{ type: 'text', text: "Mcp::Tools::GetServerVersionService: current_user is not set" }], + structuredContent: {}, + isError: true + }) end end end diff --git a/spec/services/mcp/tools/version_helper_spec.rb b/spec/services/mcp/tools/version_helper_spec.rb new file mode 100644 index 00000000000000..60798aa680af1f --- /dev/null +++ b/spec/services/mcp/tools/version_helper_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mcp::Tools::VersionHelper, feature_category: :mcp_server do + let(:test_class) do + Class.new do + include Mcp::Tools::VersionHelper + end + end + + let(:instance) { test_class.new } + + describe '#validate_semantic_version' do + context 'with valid semantic versions' do + it 'returns true for basic semantic versions' do + expect(instance.validate_semantic_version('1.0.0')).to be true + expect(instance.validate_semantic_version('0.1.0')).to be true + expect(instance.validate_semantic_version('10.20.30')).to be true + end + + it 'returns true for pre-release versions' do + expect(instance.validate_semantic_version('1.0.0-alpha')).to be true + expect(instance.validate_semantic_version('1.0.0-beta.1')).to be true + expect(instance.validate_semantic_version('1.0.0-rc.1')).to be true + end + + it 'returns true for build metadata' do + expect(instance.validate_semantic_version('1.0.0+build.1')).to be true + expect(instance.validate_semantic_version('1.0.0-alpha+build.1')).to be true + end + end + + context 'with invalid semantic versions' do + it 'returns false for invalid formats' do + expect(instance.validate_semantic_version('1.0')).to be false + expect(instance.validate_semantic_version('1')).to be false + expect(instance.validate_semantic_version('v1.0.0')).to be false + expect(instance.validate_semantic_version('1.0.0.0')).to be false + expect(instance.validate_semantic_version('invalid-version')).to be false + end + + it 'returns false for nil or empty values' do + expect(instance.validate_semantic_version(nil)).to be false + expect(instance.validate_semantic_version('')).to be false + end + end + end +end -- GitLab