Cherry-pick: MCP bug fixes to release/2.0 (#3294, #3303)#3503
Open
souvikghosh04 wants to merge 2 commits intorelease/2.0from
Open
Cherry-pick: MCP bug fixes to release/2.0 (#3294, #3303)#3503souvikghosh04 wants to merge 2 commits intorelease/2.0from
souvikghosh04 wants to merge 2 commits intorelease/2.0from
Conversation
## Why make this change? - Closes #3279 The aggregate_records MCP tool always rejects requests without groupby because the orderby schema has "default": "desc", causing LLMs to always include it, which then triggers a validation error requiring groupby. Simple aggregations like SELECT COUNT(*) FROM table had to go through this kind of behaviour and complexity. ## What is this change? Removed "default": "desc" from the orderby input schema so LLMs no longer auto-populate it. Updated ValidateGroupByDependencies to silently ignore orderby when groupby is absent instead of returning an error. Ordering is meaningless without grouping, so the parameter is harmlessly cleared via a ref flag. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests - [x] Manual Tests (VS code GHCP) --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
## Why make this change? - Closes #3284 MCP endpoint returns 404 when the mcp config block is omitted, despite mcp.enabled defaulting to true across schema, CLI, and model layers. ## What is this change? `McpEndpointRouteBuilderExtensions.TryGetMcpOptions`: Falls back to new `McpRuntimeOptions()` when `Runtime.Mcp` is `null`, so the `/mcp` endpoint is registered by default. `RestService.GetRouteAfterPathBase`: Guards the `GlobalMcpEndpointDisabled` exception with an `IsMcpEnabled` check so MCP path requests are not unconditionally rejected. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests ## Sample Request(s) /mcp /health
souvikghosh04
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
aaronburtle,
anushakolan,
rusamant,
sourabh1007,
stuartpa and
vadeveka
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Cherry-picks two MCP fixes into release/2.0: one to stop aggregate_records from effectively requiring groupby, and one to make omitted runtime.mcp behave like the documented default-enabled configuration.
Changes:
- Removes the
orderbyschema default fromaggregate_recordsand changes validation soorderbyis ignored whengroupbyis absent. - Switches MCP enablement checks to use default-enabled semantics when the
runtime.mcpblock is omitted. - Adds unit tests around MCP defaults, REST path guarding, and aggregate tool validation/schema behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Service.Tests/UnitTests/RestServiceUnitTests.cs |
Adds tests for MCP path handling when MCP is disabled or omitted. |
src/Service.Tests/Mcp/AggregateRecordsToolTests.cs |
Adds coverage for orderby/groupby validation and schema metadata. |
src/Service.Tests/Configuration/McpRuntimeOptionsSerializationTests.cs |
Verifies MCP default enablement/path behavior during config parsing. |
src/Core/Services/RestService.cs |
Fixes MCP-path rejection to depend on actual MCP enablement. |
src/Cli/ConfigGenerator.cs |
Aligns CLI validation with RuntimeConfig.IsMcpEnabled. |
src/Azure.DataApiBuilder.Mcp/Core/McpEndpointRouteBuilderExtensions.cs |
Maps /mcp using default MCP options when runtime.mcp is missing. |
src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs |
Updates aggregate tool schema text and defers orderby validation until groupby is known. |
| ""enum"": [""asc"", ""desc""], | ||
| ""description"": ""Sort grouped results by the aggregated value. Requires groupby."", | ||
| ""default"": ""desc"" | ||
| ""description"": ""Sort direction for grouped results by the aggregated value. Only applies when groupby is provided; ignored otherwise."" |
Comment on lines
+25
to
+30
| if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig)) | ||
| { | ||
| return endpoints; | ||
| } | ||
|
|
||
| McpRuntimeOptions mcpOptions = runtimeConfig?.Runtime?.Mcp ?? new McpRuntimeOptions(); |
Comment on lines
+30
to
+32
| McpRuntimeOptions mcpOptions = runtimeConfig?.Runtime?.Mcp ?? new McpRuntimeOptions(); | ||
|
|
||
| if (!mcpOptions.Enabled) |
Aniruddh25
approved these changes
May 5, 2026
Contributor
Approving the PR assuming the issues would be addressed. |
anushakolan
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-picks MCP bug fixes from main to release/2.0: