Fix MCP orderby validation to return actionable argument errors

src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs

@@ -436,9 +436,20 @@ public async Task<CallToolResult> ExecuteAsync(
             // Parse filter
             string? filter = root.TryGetProperty("filter", out JsonElement filterElement) ? filterElement.GetString() : null;
 
-            // Parse orderby (validation deferred until after groupby is known;
-            // if groupby is absent, orderby is silently ignored per #3279)
-            bool userProvidedOrderby = root.TryGetProperty("orderby", out JsonElement orderbyElement) && !string.IsNullOrWhiteSpace(orderbyElement.GetString());
+            // Validate the JSON type before reading orderby. Preserve null/blank as omitted;
+            // direction validation is deferred until groupby is known (see #3279).
+            bool userProvidedOrderby = root.TryGetProperty("orderby", out JsonElement orderbyElement);
+            if (userProvidedOrderby)
+            {
+                if (orderbyElement.ValueKind != JsonValueKind.String && orderbyElement.ValueKind != JsonValueKind.Null)
+                {
+                    return McpResponseBuilder.BuildErrorResult(toolName, "InvalidArguments",
+                        $"Argument 'orderby' must be a string ('asc' or 'desc'), for example \"desc\". Got: '{orderbyElement.ValueKind}'.", logger);
+                }
+
+                userProvidedOrderby = !string.IsNullOrWhiteSpace(orderbyElement.GetString());
+            }
+
             string orderby = "desc";
 
             // Parse first

src/Azure.DataApiBuilder.Mcp/BuiltInTools/ReadRecordsTool.cs

@@ -29,6 +29,10 @@ namespace Azure.DataApiBuilder.Mcp.BuiltInTools
 {
     public class ReadRecordsTool : IMcpTool
     {
+        private const string INVALID_ORDERBY_MESSAGE =
+            "Argument 'orderby' must be an array of non-empty strings specifying fields and optional directions, " +
+            "for example [\"name asc\", \"year desc\"].";
+
         public ToolType ToolType { get; } = ToolType.BuiltIn;
 
         public bool IsEnabled(RuntimeConfig config) => config.McpDmlTools?.ReadRecords ?? true;
@@ -100,7 +104,7 @@ public async Task<CallToolResult> ExecuteAsync(
                 string? select = null;
                 string? filter = null;
                 int? first = null;
-                IEnumerable<string>? orderby = null;
+                List<string>? orderby = null;
                 string? after = null;
 
                 // Extract arguments
@@ -140,7 +144,23 @@ public async Task<CallToolResult> ExecuteAsync(
 
                 if (root.TryGetProperty("orderby", out JsonElement orderbyElement))
                 {
-                    orderby = (IEnumerable<string>?)orderbyElement.EnumerateArray().Select(e => e.GetString());
+                    if (orderbyElement.ValueKind != JsonValueKind.Array)
+                    {
+                        return McpResponseBuilder.BuildErrorResult(toolName, "InvalidArguments", INVALID_ORDERBY_MESSAGE, logger);
+                    }
+
+                    // Validate and materialize every item before metadata resolution or query parsing.
+                    orderby = new List<string>();
+                    foreach (JsonElement item in orderbyElement.EnumerateArray())
+                    {
+                        string? sort = item.ValueKind == JsonValueKind.String ? item.GetString() : null;
+                        if (string.IsNullOrWhiteSpace(sort))
+                        {
+                            return McpResponseBuilder.BuildErrorResult(toolName, "InvalidArguments", INVALID_ORDERBY_MESSAGE, logger);
+                        }
+
+                        orderby.Add(sort);
+                    }
                 }
 
                 if (root.TryGetProperty("after", out JsonElement afterElement))
@@ -216,20 +236,9 @@ public async Task<CallToolResult> ExecuteAsync(
                     context.FilterClauseInUrl = sqlMetadataProvider.GetODataParser().GetFilterClause(filterQueryString, $"{context.EntityName}.{context.DatabaseObject.FullName}");
                 }
 
-                if (orderby is not null && orderby.Count() != 0)
+                if (orderby is not null && orderby.Count != 0)
                 {
-                    string sortQueryString = $"?{RequestParser.SORT_URL}=";
-                    foreach (string param in orderby)
-                    {
-                        if (string.IsNullOrWhiteSpace(param))
-                        {
-                            return McpResponseBuilder.BuildErrorResult(toolName, "InvalidArguments", "Parameters inside 'orderby' argument cannot be empty or null.", logger);
-                        }
-
-                        sortQueryString += $"{param}, ";
-                    }
-
-                    sortQueryString = sortQueryString.Substring(0, sortQueryString.Length - 2);
+                    string sortQueryString = $"?{RequestParser.SORT_URL}={string.Join(", ", orderby)}";
                     (context.OrderByClauseInUrl, context.OrderByClauseOfBackingColumns) = RequestParser.GenerateOrderByLists(context, sqlMetadataProvider, sortQueryString);
                 }
 

src/Service.Tests/Mcp/AggregateRecordsToolTests.cs

@@ -495,6 +495,66 @@ public async Task AggregateRecords_GroupByDependencyViolation_ReturnsInvalidArgu
 
         #endregion
 
+        #region Input Validation Tests - Orderby Shape (Issue #3810)
+
+        /// <summary>
+        /// Wrong JSON types must return a tool-specific argument error, regardless of grouping.
+        /// </summary>
+        [DataTestMethod]
+        [DataRow("[\"desc\"]")]
+        [DataRow("[\"title desc\"]")]
+        [DataRow("[]")]
+        [DataRow("[null]")]
+        [DataRow("{}")]
+        [DataRow("123")]
+        [DataRow("true")]
+        [DataRow("false")]
+        public async Task AggregateRecords_InvalidOrderbyShape_ReturnsActionableInvalidArguments(string orderby)
+        {
+            foreach (string grouping in new[] { string.Empty, ",\"groupby\":[]", ",\"groupby\":[\"title\"]" })
+            {
+                string json = $"{{\"entity\":\"Book\",\"function\":\"count\",\"orderby\":{orderby}{grouping}}}";
+                CallToolResult result = await ExecuteToolAsync(CreateDefaultServiceProvider(), json);
+
+                string message = AssertErrorResult(result, "InvalidArguments");
+                StringAssert.Contains(message, "'orderby'");
+                StringAssert.Contains(message, "string");
+                StringAssert.Contains(message, "'asc'");
+                StringAssert.Contains(message, "'desc'");
+                StringAssert.Contains(message, "\"desc\"");
+            }
+        }
+
+        /// <summary>
+        /// Preserve existing optional/null/blank handling, normalization, and the grouped descending default.
+        /// </summary>
+        [DataTestMethod]
+        [DataRow(null, "desc")]
+        [DataRow("null", "desc")]
+        [DataRow("\"\"", "desc")]
+        [DataRow("\"   \"", "desc")]
+        [DataRow("\"asc\"", "asc")]
+        [DataRow("\"desc\"", "desc")]
+        [DataRow("\" ASC \"", "asc")]
+        [DataRow("\" DeSc \"", "desc")]
+        public void AggregateRecords_ValidOrderby_PreservesDirectionAndDefault(string? orderby, string expectedDirection)
+        {
+            string orderbyProperty = orderby is null ? string.Empty : $",\"orderby\":{orderby}";
+            using JsonDocument arguments = JsonDocument.Parse(
+                $"{{\"entity\":\"Book\",\"function\":\"count\",\"groupby\":[\"title\"]{orderbyProperty}}}");
+            object?[] parameters = { arguments, CreateConfig(), "aggregate_records", null, null };
+
+            CallToolResult? error = InvokePrivateWithMutableArguments<CallToolResult?>(
+                "TryParseAndValidateArguments", parameters);
+
+            Assert.IsNull(error);
+            AggregateRecordsTool.AggregateArguments parsed = (AggregateRecordsTool.AggregateArguments)parameters[3]!;
+            Assert.AreEqual(expectedDirection, parsed.Orderby);
+            CollectionAssert.AreEqual(new[] { "title" }, parsed.Groupby);
+        }
+
+        #endregion
+
         #region Input Validation Tests - Orderby Without Groupby (Issue #3279)
 
         /// <summary>
@@ -517,17 +577,9 @@ public async Task AggregateRecords_OrderbyWithoutGroupby_PassesValidation(string
 
             CallToolResult result = await ExecuteToolAsync(sp, json);
 
-            // The tool may fail at metadata resolution (no real DB), but must NOT fail with InvalidArguments.
-            // If the tool succeeds, that's also acceptable — the test is focused on input validation.
-            if (result.IsError != true)
-            {
-                return;
-            }
-
-            JsonElement content = ParseContent(result);
-            string errorType = content.GetProperty("error").GetProperty("type").GetString()!;
-            Assert.AreNotEqual("InvalidArguments", errorType,
-                $"orderby without groupby must not be rejected as InvalidArguments. Got error type: {errorType}");
+            // The provider intentionally has no metadata factory. Require that exact boundary,
+            // rather than allowing an UnexpectedError to pass this compatibility test.
+            AssertErrorResult(result, "EntityNotFound");
         }
 
         /// <summary>

src/Service.Tests/Mcp/BuiltInDmlToolValidationTests.cs

@@ -142,6 +142,60 @@ public async Task ReadRecords_MissingEntity_ReturnsInvalidArguments()
             AssertErrorType(result, "InvalidArguments");
         }
 
+        /// <summary>
+        /// Rejects invalid orderby shapes and members before metadata resolution, with a usable example.
+        /// </summary>
+        [DataTestMethod]
+        [DataRow("\"id desc\"")]
+        [DataRow("\"id\"")]
+        [DataRow("\"desc\"")]
+        [DataRow("null")]
+        [DataRow("123")]
+        [DataRow("true")]
+        [DataRow("false")]
+        [DataRow("{}")]
+        [DataRow("[123]")]
+        [DataRow("[true]")]
+        [DataRow("[false]")]
+        [DataRow("[{}]")]
+        [DataRow("[[]]")]
+        [DataRow("[null]")]
+        [DataRow("[\"\"]")]
+        [DataRow("[\"   \"]")]
+        [DataRow("[\"id desc\",123]")]
+        [DataRow("[\"id desc\",null]")]
+        [DataRow("[null,123]")]
+        public async Task ReadRecords_InvalidOrderby_ReturnsActionableInvalidArguments(string orderby)
+        {
+            IServiceProvider sp = CreateServiceProvider(CreateConfig());
+            CallToolResult result = await ExecuteAsync(
+                new ReadRecordsTool(), sp, $"{{\"entity\":\"Book\",\"orderby\":{orderby}}}");
+
+            string message = AssertErrorType(result, "InvalidArguments");
+            StringAssert.Contains(message, "'orderby'");
+            StringAssert.Contains(message, "array of non-empty strings");
+            StringAssert.Contains(message, "[\"name asc\", \"year desc\"]");
+        }
+
+        /// <summary>
+        /// Valid string lists, empty lists, and omitted ordering must still reach metadata resolution.
+        /// This test checks shape validation only, not field resolution or database execution.
+        /// </summary>
+        [DataTestMethod]
+        [DataRow("{\"entity\":\"Book\"}")]
+        [DataRow("{\"entity\":\"Book\",\"orderby\":[]}")]
+        [DataRow("{\"entity\":\"Book\",\"orderby\":[\"id\"]}")]
+        [DataRow("{\"entity\":\"Book\",\"orderby\":[\"id asc\"]}")]
+        [DataRow("{\"entity\":\"Book\",\"orderby\":[\"id desc\"]}")]
+        [DataRow("{\"entity\":\"Book\",\"orderby\":[\"title asc\",\"id desc\"]}")]
+        public async Task ReadRecords_ValidOrderbyShape_ReachesMetadataResolution(string arguments)
+        {
+            IServiceProvider sp = CreateServiceProvider(CreateConfig());
+            CallToolResult result = await ExecuteAsync(new ReadRecordsTool(), sp, arguments);
+
+            AssertErrorType(result, "EntityNotFound");
+        }
+
         [TestMethod]
         public async Task UpdateRecord_MissingFields_ReturnsInvalidArguments()
         {
@@ -229,12 +283,14 @@ private static async Task<CallToolResult> ExecuteAsync(IMcpTool tool, IServicePr
             return await tool.ExecuteAsync(args, sp, CancellationToken.None);
         }
 
-        private static void AssertErrorType(CallToolResult result, string expectedType)
+        private static string AssertErrorType(CallToolResult result, string expectedType)
         {
             Assert.IsTrue(result.IsError == true, "Expected an error result.");
             TextContentBlock block = (TextContentBlock)result.Content[0];
-            JsonElement root = JsonDocument.Parse(block.Text).RootElement;
-            Assert.AreEqual(expectedType, root.GetProperty("error").GetProperty("type").GetString());
+            using JsonDocument payload = JsonDocument.Parse(block.Text);
+            JsonElement error = payload.RootElement.GetProperty("error");
+            Assert.AreEqual(expectedType, error.GetProperty("type").GetString());
+            return error.GetProperty("message").GetString()!;
         }
 
         private static RuntimeConfig CreateConfig(

src/Service.Tests/Mcp/McpServerConfigurationTests.cs

@@ -2,10 +2,22 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Net.Http;
+using System.Text;
+using System.Text.Json;
+using System.Threading.Tasks;
+using Azure.DataApiBuilder.Config.ObjectModel;
+using Azure.DataApiBuilder.Mcp.BuiltInTools;
 using Azure.DataApiBuilder.Mcp.Core;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.TestHost;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using ModelContextProtocol.AspNetCore;
 using ModelContextProtocol.Server;
 
 namespace Azure.DataApiBuilder.Service.Tests.Mcp
@@ -33,5 +45,100 @@ public void ConfigureMcpServer_ConfiguresServerOptions(string? instructions, str
             Assert.IsNotNull(options.Capabilities.Tools);
             Assert.AreEqual(expectedInstructions, options.ServerInstructions);
         }
+
+        /// <summary>
+        /// Exercises discovery and invocation through the real MCP HTTP handler, with no database or network listener.
+        /// Advertised ordering shapes remain different; malformed calls must return actionable tool errors.
+        /// </summary>
+        [DataTestMethod]
+        [DataRow("read_records", "{\"entity\":\"Book\",\"orderby\":\"id desc\"}", "array of non-empty strings")]
+        [DataRow("read_records", "{\"entity\":\"Book\",\"orderby\":[\"id desc\",123]}", "array of non-empty strings")]
+        [DataRow("aggregate_records", "{\"entity\":\"Book\",\"function\":\"count\",\"groupby\":[\"title\"],\"orderby\":[\"desc\"]}", "string ('asc' or 'desc')")]
+        [DataRow("aggregate_records", "{\"entity\":\"Book\",\"function\":\"count\",\"orderby\":[\"desc\"]}", "string ('asc' or 'desc')")]
+        public async Task ConfigureMcpServer_InvalidOrderby_ReturnsActionableToolError(string toolName, string json, string expectedShape)
+        {
+            WebApplicationBuilder builder = WebApplication.CreateBuilder(new WebApplicationOptions
+            {
+                EnvironmentName = "Development",
+                ContentRootPath = AppContext.BaseDirectory
+            });
+            builder.Configuration.Sources.Clear();
+            builder.Logging.ClearProviders();
+            builder.WebHost.UseTestServer();
+            RuntimeConfig config = new(
+                Schema: "test-schema",
+                DataSource: new(DatabaseType.MSSQL, ConnectionString: string.Empty, Options: null),
+                Runtime: new(
+                    Rest: new(),
+                    GraphQL: new(),
+                    Mcp: new(Enabled: true, Path: "/mcp", DmlTools: DmlToolsConfig.Default),
+                    Host: new(Cors: null, Authentication: null, Mode: HostMode.Development)),
+                Entities: new(new Dictionary<string, Entity>()));
+            builder.Services.AddSingleton(TestHelper.GenerateInMemoryRuntimeConfigProvider(config));
+            McpToolRegistry registry = new();
+            registry.ReplaceAll([new ReadRecordsTool(), new AggregateRecordsTool()], config);
+            builder.Services.AddSingleton(registry);
+            builder.Services.ConfigureMcpServer(instructions: null);
+            builder.Services.PostConfigure<HttpServerTransportOptions>(options => options.Stateless = true);
+
+            await using WebApplication app = builder.Build();
+            app.MapMcp("/mcp");
+            await app.StartAsync();
+            using HttpClient client = app.GetTestClient();
+            client.Timeout = TimeSpan.FromSeconds(15);
+            client.DefaultRequestHeaders.Accept.ParseAdd("application/json");
+            client.DefaultRequestHeaders.Accept.ParseAdd("text/event-stream");
+            client.DefaultRequestHeaders.Add("MCP-Protocol-Version", "2025-11-25");
+
+            JsonElement initialized = await PostRpcAsync(client, 1, "initialize", new
+            {
+                protocolVersion = "2025-11-25",
+                capabilities = new { },
+                clientInfo = new { name = "orderby-validation-test", version = "1" }
+            });
+            Assert.IsTrue(initialized.TryGetProperty("result", out _));
+
+            JsonElement listed = await PostRpcAsync(client, 2, "tools/list", new { });
+            JsonElement tools = listed.GetProperty("result").GetProperty("tools");
+            Assert.AreEqual(2, tools.GetArrayLength());
+            foreach (JsonElement tool in tools.EnumerateArray())
+            {
+                string expectedType = tool.GetProperty("name").GetString() == "read_records" ? "array" : "string";
+                Assert.AreEqual(expectedType, tool.GetProperty("inputSchema").GetProperty("properties")
+                    .GetProperty("orderby").GetProperty("type").GetString());
+            }
+
+            using JsonDocument arguments = JsonDocument.Parse(json);
+            JsonElement response = await PostRpcAsync(client, 3, "tools/call", new { name = toolName, arguments = arguments.RootElement });
+            JsonElement result = response.GetProperty("result");
+            Assert.IsTrue(result.GetProperty("isError").GetBoolean());
+            using JsonDocument content = JsonDocument.Parse(result.GetProperty("content")[0].GetProperty("text").GetString()!);
+            Assert.AreEqual(toolName, content.RootElement.GetProperty("toolName").GetString());
+            JsonElement error = content.RootElement.GetProperty("error");
+            Assert.AreEqual("InvalidArguments", error.GetProperty("type").GetString());
+            string message = error.GetProperty("message").GetString()!;
+            StringAssert.Contains(message, "'orderby'");
+            StringAssert.Contains(message, expectedShape);
+            StringAssert.Contains(message, "for example");
+
+            await app.StopAsync();
+        }
+
+        private static async Task<JsonElement> PostRpcAsync(HttpClient client, int id, string method, object parameters)
+        {
+            string json = JsonSerializer.Serialize(new { jsonrpc = "2.0", id, method, @params = parameters });
+            using StringContent body = new(json, Encoding.UTF8, "application/json");
+            using HttpResponseMessage response = await client.PostAsync("/mcp", body);
+            string payload = await response.Content.ReadAsStringAsync();
+            Assert.IsTrue(response.IsSuccessStatusCode, $"HTTP {response.StatusCode}: {payload}");
+
+            if (response.Content.Headers.ContentType?.MediaType == "text/event-stream")
+            {
+                payload = payload.Split('\n').Last(line => line.StartsWith("data:", StringComparison.Ordinal))[5..].Trim();
+            }
+
+            using JsonDocument document = JsonDocument.Parse(payload);
+            return document.RootElement.Clone();
+        }
     }
 }