From ed4f7a74407cd05717d315bd4fa4146cddd3a3d9 Mon Sep 17 00:00:00 2001 From: Mansur Besleney Date: Thu, 8 Jan 2026 13:30:22 +0300 Subject: [PATCH] Refactor MCP command and service logic for clarity Extracted license validation logic in McpCommand to a dedicated method for reuse and clarity. Improved executable path resolution by splitting logic into helper methods. In McpHttpClientService, centralized HTTP exception handling and replaced repeated code with a single method. Refactored tool invocation in McpServerService to use a dedicated handler, improving error handling and logging. Simplified log level parsing in McpLogger for better maintainability. --- .../Volo/Abp/Cli/Commands/McpCommand.cs | 113 +++++++++++------- .../Commands/Services/McpHttpClientService.cs | 43 ++++--- .../Abp/Cli/Commands/Services/McpLogger.cs | 34 +++--- .../Cli/Commands/Services/McpServerService.cs | 106 ++++++++-------- 4 files changed, 162 insertions(+), 134 deletions(-) diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/McpCommand.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/McpCommand.cs index d9dba2c0cc..49ec0b5067 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/McpCommand.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/McpCommand.cs @@ -48,25 +48,7 @@ public class McpCommand : IConsoleCommand, ITransientDependency public async Task ExecuteAsync(CommandLineArgs commandLineArgs) { - var loginInfo = await _authService.GetLoginInfoAsync(); - - if (string.IsNullOrEmpty(loginInfo?.Organization)) - { - throw new CliUsageException("Please log in with your account!"); - } - - var licenseResult = await _apiKeyService.GetApiKeyOrNullAsync(); - - if (licenseResult == null || !licenseResult.HasActiveLicense) - { - var errorMessage = licenseResult?.ErrorMessage ?? "No active license found."; - throw new CliUsageException(errorMessage); - } - - if (licenseResult.LicenseEndTime.HasValue && licenseResult.LicenseEndTime.Value < DateTime.UtcNow) - { - throw new CliUsageException("Your license has expired. Please renew your license to use the MCP server."); - } + await ValidateLicenseAsync(); var option = commandLineArgs.Target; @@ -89,9 +71,8 @@ public class McpCommand : IConsoleCommand, ITransientDependency _mcpLogger.Info(LogSource, "Starting ABP MCP Server..."); var cts = new CancellationTokenSource(); - ConsoleCancelEventHandler cancelHandler = null; - cancelHandler = (sender, e) => + ConsoleCancelEventHandler cancelHandler = (sender, e) => { e.Cancel = true; _mcpLogger.Info(LogSource, "Shutting down ABP MCP Server..."); @@ -128,6 +109,29 @@ public class McpCommand : IConsoleCommand, ITransientDependency } } + private async Task ValidateLicenseAsync() + { + var loginInfo = await _authService.GetLoginInfoAsync(); + + if (string.IsNullOrEmpty(loginInfo?.Organization)) + { + throw new CliUsageException("Please log in with your account!"); + } + + var licenseResult = await _apiKeyService.GetApiKeyOrNullAsync(); + + if (licenseResult == null || !licenseResult.HasActiveLicense) + { + var errorMessage = licenseResult?.ErrorMessage ?? "No active license found."; + throw new CliUsageException(errorMessage); + } + + if (licenseResult.LicenseEndTime.HasValue && licenseResult.LicenseEndTime.Value < DateTime.UtcNow) + { + throw new CliUsageException("Your license has expired. Please renew your license to use the MCP server."); + } + } + private Task PrintConfigurationAsync() { var abpCliPath = GetAbpCliExecutablePath(); @@ -158,20 +162,34 @@ public class McpCommand : IConsoleCommand, ITransientDependency private string GetAbpCliExecutablePath() { - // Try to find the abp CLI executable + var processPath = TryGetExecutablePathFromCurrentProcess(); + if (processPath != null) + { + return processPath; + } + + var environmentPath = TryGetExecutablePathFromEnvironmentPath(); + if (environmentPath != null) + { + return environmentPath; + } + + // Default to "abp" and let the system resolve it + return "abp"; + } + + private string TryGetExecutablePathFromCurrentProcess() + { try { using (var process = Process.GetCurrentProcess()) { var processPath = process.MainModule?.FileName; - if (!string.IsNullOrEmpty(processPath)) + if (!string.IsNullOrEmpty(processPath) && + Path.GetFileName(processPath).StartsWith("abp", StringComparison.OrdinalIgnoreCase)) { - // If running as a published executable - if (Path.GetFileName(processPath).StartsWith("abp", StringComparison.OrdinalIgnoreCase)) - { - return processPath; - } + return processPath; } } } @@ -180,29 +198,34 @@ public class McpCommand : IConsoleCommand, ITransientDependency // Ignore errors getting process path } - // Check if abp is in PATH + return null; + } + + private string TryGetExecutablePathFromEnvironmentPath() + { var pathEnv = Environment.GetEnvironmentVariable("PATH"); - if (!string.IsNullOrEmpty(pathEnv)) + if (string.IsNullOrEmpty(pathEnv)) { - var paths = pathEnv.Split(Path.PathSeparator); - foreach (var path in paths) + return null; + } + + var paths = pathEnv.Split(Path.PathSeparator); + foreach (var path in paths) + { + var abpPath = Path.Combine(path, "abp.exe"); + if (File.Exists(abpPath)) { - var abpPath = Path.Combine(path, "abp.exe"); - if (File.Exists(abpPath)) - { - return abpPath; - } - - abpPath = Path.Combine(path, "abp"); - if (File.Exists(abpPath)) - { - return abpPath; - } + return abpPath; + } + + abpPath = Path.Combine(path, "abp"); + if (File.Exists(abpPath)) + { + return abpPath; } } - // Default to "abp" and let the system resolve it - return "abp"; + return null; } public string GetUsageInfo() diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs index dc6bd84521..2a4c18bc88 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpHttpClientService.cs @@ -16,6 +16,8 @@ namespace Volo.Abp.Cli.Commands.Services; public class McpHttpClientService : ITransientDependency { private const string LogSource = nameof(McpHttpClientService); + // TODO: Remove hardcoded URL after testing + private const string TestServerUrl = "http://localhost:5100"; private static readonly JsonSerializerOptions JsonSerializerOptionsWeb = new(JsonSerializerDefaults.Web); private static class ErrorMessages @@ -74,7 +76,7 @@ public class McpHttpClientService : ITransientDependency public async Task CallToolAsync(string toolName, JsonElement arguments) { - var baseUrl = "http://localhost:5100";//await GetMcpServerUrlAsync(); + var baseUrl = TestServerUrl;//await GetMcpServerUrlAsync(); var url = $"{baseUrl}/tools/call"; try @@ -161,7 +163,7 @@ public class McpHttpClientService : ITransientDependency public async Task CheckServerHealthAsync() { - var baseUrl = "http://localhost:5100";//await GetMcpServerUrlAsync(); + var baseUrl = TestServerUrl;//await GetMcpServerUrlAsync(); try { @@ -178,7 +180,7 @@ public class McpHttpClientService : ITransientDependency public async Task> GetToolDefinitionsAsync() { - var baseUrl = "http://localhost:5100";//await GetMcpServerUrlAsync(); + var baseUrl = TestServerUrl;//await GetMcpServerUrlAsync(); var url = $"{baseUrl}/tools"; try @@ -204,24 +206,15 @@ public class McpHttpClientService : ITransientDependency } catch (HttpRequestException ex) { - _mcpLogger.Error(LogSource, "Network error fetching tool definitions", ex); - - // Throw sanitized exception - throw CreateToolDefinitionException("Network connectivity issue. Please check your internet connection and try again."); + throw CreateHttpException(ex, "Network error fetching tool definitions"); } catch (TaskCanceledException ex) { - _mcpLogger.Error(LogSource, "Timeout fetching tool definitions", ex); - - // Throw sanitized exception - throw CreateToolDefinitionException("Request timed out. Please try again."); + throw CreateHttpException(ex, "Timeout fetching tool definitions"); } catch (JsonException ex) { - _mcpLogger.Error(LogSource, "JSON parsing error", ex); - - // Throw sanitized exception - throw CreateToolDefinitionException("Invalid response format received."); + throw CreateHttpException(ex, "JSON parsing error"); } catch (Exception ex) when (ex.Message.StartsWith("Failed to fetch tool definitions:")) { @@ -230,13 +223,25 @@ public class McpHttpClientService : ITransientDependency } catch (Exception ex) { - _mcpLogger.Error(LogSource, "Unexpected error fetching tool definitions", ex); - - // Throw sanitized exception - throw CreateToolDefinitionException("An unexpected error occurred. Please try again later."); + throw CreateHttpException(ex, "Unexpected error fetching tool definitions"); } } + private Exception CreateHttpException(Exception ex, string context) + { + _mcpLogger.Error(LogSource, context, ex); + + var userMessage = ex switch + { + HttpRequestException => "Network connectivity issue. Please check your internet connection and try again.", + TaskCanceledException => "Request timed out. Please try again.", + JsonException => "Invalid response format received.", + _ => "An unexpected error occurred. Please try again later." + }; + + return CreateToolDefinitionException(userMessage); + } + private class McpToolsResponse { public List Tools { get; set; } diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs index dd3c0f2f78..1a81486241 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpLogger.cs @@ -157,43 +157,39 @@ public class McpLogger : IMcpLogger, ISingletonDependency private static McpLogLevel GetConfiguredLogLevel() { + var envValue = Environment.GetEnvironmentVariable(CliConsts.McpLogLevelEnvironmentVariable); + var isEmpty = string.IsNullOrWhiteSpace(envValue); + #if DEBUG // In development builds, allow full control via environment variable - var envValue = Environment.GetEnvironmentVariable(CliConsts.McpLogLevelEnvironmentVariable); - - if (string.IsNullOrWhiteSpace(envValue)) + if (isEmpty) { return McpLogLevel.Info; // Default level } - - return envValue.ToLowerInvariant() switch - { - "debug" => McpLogLevel.Debug, - "info" => McpLogLevel.Info, - "warning" => McpLogLevel.Warning, - "error" => McpLogLevel.Error, - "none" => McpLogLevel.None, - _ => McpLogLevel.Info - }; + + return ParseLogLevel(envValue, allowDebug: true); #else // In release builds, restrict to Warning or higher (ignore env variable for Debug/Info) - var envValue = Environment.GetEnvironmentVariable(CliConsts.McpLogLevelEnvironmentVariable); - - if (string.IsNullOrWhiteSpace(envValue)) + if (isEmpty) { return McpLogLevel.Info; // Default level } - return envValue.ToLowerInvariant() switch + return ParseLogLevel(envValue, allowDebug: false); +#endif + } + + private static McpLogLevel ParseLogLevel(string value, bool allowDebug) + { + return value.ToLowerInvariant() switch { - "debug" => McpLogLevel.Info, // Cap Debug to Info + "debug" => allowDebug ? McpLogLevel.Debug : McpLogLevel.Info, "info" => McpLogLevel.Info, "warning" => McpLogLevel.Warning, "error" => McpLogLevel.Error, "none" => McpLogLevel.None, _ => McpLogLevel.Info }; -#endif } } diff --git a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs index c1938498a5..0c8c4ba21d 100644 --- a/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs +++ b/framework/src/Volo.Abp.Cli.Core/Volo/Abp/Cli/Commands/Services/McpServerService.cs @@ -130,62 +130,66 @@ public class McpServerService : ITransientDependency name, description, JsonSerializer.SerializeToElement(inputSchema), - async (context, cancellationToken) => - { - _mcpLogger.Debug(LogSource, $"Tool '{name}' called with arguments: {context.Params.Arguments}"); - - try - { - var argumentsDict = context.Params.Arguments; - var argumentsJson = JsonSerializer.SerializeToElement(argumentsDict); - var resultJson = await _mcpHttpClient.CallToolAsync( - name, - argumentsJson - ); - - // Try to deserialize the response as CallToolResult - // The HTTP client should return JSON in the format expected by MCP - try - { - var callToolResult = JsonSerializer.Deserialize(resultJson); - if (callToolResult != null) - { - // Check if the HTTP client returned an error - if (callToolResult.IsError == true) - { - _mcpLogger.Warning(LogSource, $"Tool '{name}' returned an error"); - } - else - { - _mcpLogger.Debug(LogSource, $"Tool '{name}' executed successfully"); - } - - return callToolResult; - } - } - catch (Exception deserializeEx) - { - _mcpLogger.Error(LogSource, $"Failed to deserialize response as CallToolResult: {deserializeEx.Message}"); - _mcpLogger.Debug(LogSource, $"Response was: {resultJson.Substring(0, Math.Min(500, resultJson.Length))}"); - } - - // Fallback: return error result if deserialization fails - return CreateErrorResult(ToolErrorMessages.InvalidResponseFormat); - } - catch (Exception ex) - { - // Log detailed error for debugging - _mcpLogger.Error(LogSource, $"Tool '{name}' execution failed", ex); - - // Return sanitized error to client - return CreateErrorResult(ToolErrorMessages.UnexpectedError); - } - } + (context, cancellationToken) => HandleToolInvocationAsync(name, context, cancellationToken) ); options.ToolCollection.Add(tool); } + private async ValueTask HandleToolInvocationAsync( + string toolName, + RequestContext context, + CancellationToken cancellationToken) + { + _mcpLogger.Debug(LogSource, $"Tool '{toolName}' called with arguments: {context.Params.Arguments}"); + + try + { + var argumentsJson = JsonSerializer.SerializeToElement(context.Params.Arguments); + var resultJson = await _mcpHttpClient.CallToolAsync(toolName, argumentsJson); + + var callToolResult = TryDeserializeResult(resultJson, toolName); + if (callToolResult != null) + { + LogToolResult(toolName, callToolResult); + return callToolResult; + } + + return CreateErrorResult(ToolErrorMessages.InvalidResponseFormat); + } + catch (Exception ex) + { + _mcpLogger.Error(LogSource, $"Tool '{toolName}' execution failed", ex); + return CreateErrorResult(ToolErrorMessages.UnexpectedError); + } + } + + private CallToolResult TryDeserializeResult(string resultJson, string toolName) + { + try + { + return JsonSerializer.Deserialize(resultJson); + } + catch (Exception ex) + { + _mcpLogger.Error(LogSource, $"Failed to deserialize response as CallToolResult: {ex.Message}"); + _mcpLogger.Debug(LogSource, $"Response was: {resultJson.Substring(0, Math.Min(500, resultJson.Length))}"); + return null; + } + } + + private void LogToolResult(string toolName, CallToolResult result) + { + if (result.IsError == true) + { + _mcpLogger.Warning(LogSource, $"Tool '{toolName}' returned an error"); + } + else + { + _mcpLogger.Debug(LogSource, $"Tool '{toolName}' executed successfully"); + } + } + private class AbpMcpServerTool : McpServerTool { private readonly string _name;