Added changes to set McpServerOptions.Instructions in HTTPS/SSE mode.#3423
Added changes to set McpServerOptions.Instructions in HTTPS/SSE mode.#3423anushakolan wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns MCP HTTP/SSE initialization behavior with the stdio transport by propagating runtime.mcp.description into the MCP initialize response’s instructions field, and adds a regression test to prevent regressions.
Changes:
- Pass
runtime.mcp.descriptioninto MCP server configuration during DI setup. - Set MCP server options (
ServerInstructions) so HTTP/SSE initialize includesinstructions. - Add an MSSQL integration test that calls MCP
initializeand assertsresult.instructionsis present (with helper logic to parse JSON vs SSE responses).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Azure.DataApiBuilder.Mcp/Core/McpServiceCollectionExtensions.cs | Passes runtime MCP description into MCP server configuration during service registration. |
| src/Azure.DataApiBuilder.Mcp/Core/McpServerConfiguration.cs | Adds an instructions parameter and wires it to MCP server options used by HTTP/SSE initialize. |
| src/Service.Tests/Configuration/ConfigurationTests.cs | Adds a regression test + helper to invoke MCP initialize and validate returned instructions. |
| while (retryCount < RETRY_COUNT) | ||
| { | ||
| object payload = new | ||
| { | ||
| jsonrpc = "2.0", | ||
| id = 1, | ||
| method = "initialize", | ||
| @params = new | ||
| { | ||
| protocolVersion = "2025-03-26", | ||
| capabilities = new { }, | ||
| clientInfo = new { name = "dab-test", version = "1.0.0" } | ||
| } | ||
| }; | ||
|
|
||
| HttpRequestMessage mcpRequest = new(HttpMethod.Post, mcp.Path) | ||
| { | ||
| Content = JsonContent.Create(payload) | ||
| }; | ||
| mcpRequest.Headers.Add("Accept", "application/json, text/event-stream"); | ||
|
|
There was a problem hiding this comment.
GetMcpInitializeResponse duplicates the initialize request payload + retry loop logic already implemented in GetMcpResponse. This increases the chance the two helpers drift (e.g., protocolVersion/capabilities/headers/retry conditions). Consider extracting a shared helper for building/sending the MCP initialize request (and optionally returning the response body) to keep behavior consistent.
| HttpRequestMessage mcpRequest = new(HttpMethod.Post, mcp.Path) | ||
| { | ||
| Content = JsonContent.Create(payload) | ||
| }; | ||
| mcpRequest.Headers.Add("Accept", "application/json, text/event-stream"); | ||
|
|
||
| HttpResponseMessage mcpResponse = await httpClient.SendAsync(mcpRequest); |
There was a problem hiding this comment.
Inside the retry loop, HttpRequestMessage/HttpResponseMessage are created repeatedly but never disposed. In long/parallel test runs this can retain resources longer than needed; prefer wrapping both in using (and reading content before disposal) within each iteration.
| HttpRequestMessage mcpRequest = new(HttpMethod.Post, mcp.Path) | |
| { | |
| Content = JsonContent.Create(payload) | |
| }; | |
| mcpRequest.Headers.Add("Accept", "application/json, text/event-stream"); | |
| HttpResponseMessage mcpResponse = await httpClient.SendAsync(mcpRequest); | |
| using HttpRequestMessage mcpRequest = new(HttpMethod.Post, mcp.Path) | |
| { | |
| Content = JsonContent.Create(payload) | |
| }; | |
| mcpRequest.Headers.Add("Accept", "application/json, text/event-stream"); | |
| using HttpResponseMessage mcpResponse = await httpClient.SendAsync(mcpRequest); |
| foreach (string line in ssePayload.Split('\n')) | ||
| { | ||
| string trimmed = line.Trim(); | ||
| if (trimmed.StartsWith("data:", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| string data = trimmed.Substring("data:".Length).Trim(); | ||
| if (!string.IsNullOrWhiteSpace(data) && data.StartsWith('{')) | ||
| { | ||
| return data; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return string.Empty; |
There was a problem hiding this comment.
ExtractJsonFromSsePayload only returns a single data: line that starts with {. SSE events can legally split a JSON payload across multiple data: lines (which should be concatenated with newlines). Consider accumulating consecutive data: lines for the event and then parsing the combined string to avoid flaky parsing if the transport changes formatting/chunking.
| foreach (string line in ssePayload.Split('\n')) | |
| { | |
| string trimmed = line.Trim(); | |
| if (trimmed.StartsWith("data:", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| string data = trimmed.Substring("data:".Length).Trim(); | |
| if (!string.IsNullOrWhiteSpace(data) && data.StartsWith('{')) | |
| { | |
| return data; | |
| } | |
| } | |
| } | |
| return string.Empty; | |
| List<string> eventDataLines = new(); | |
| static string GetJsonPayload(List<string> dataLines) | |
| { | |
| if (dataLines.Count == 0) | |
| { | |
| return string.Empty; | |
| } | |
| string combinedPayload = string.Join("\n", dataLines); | |
| return !string.IsNullOrWhiteSpace(combinedPayload) && combinedPayload.TrimStart().StartsWith('{') | |
| ? combinedPayload | |
| : string.Empty; | |
| } | |
| foreach (string rawLine in ssePayload.Split('\n')) | |
| { | |
| string line = rawLine.TrimEnd('\r'); | |
| if (string.IsNullOrWhiteSpace(line)) | |
| { | |
| string jsonPayload = GetJsonPayload(eventDataLines); | |
| if (!string.IsNullOrEmpty(jsonPayload)) | |
| { | |
| return jsonPayload; | |
| } | |
| eventDataLines.Clear(); | |
| continue; | |
| } | |
| if (line.StartsWith("data:", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| string data = line.Substring("data:".Length); | |
| if (data.StartsWith(' ')) | |
| { | |
| data = data.Substring(1); | |
| } | |
| eventDataLines.Add(data); | |
| } | |
| } | |
| return GetJsonPayload(eventDataLines); |
Why make this change?
Closes #3283
runtime.mcp.descriptionwas configured, but MCP HTTP/SSE initialize did not return it asinstructions. This made the engine behavior inconsistent with expected MCP initialization output.Related discussion: #3282
What is this change?
Wired runtime MCP description into MCP server options for HTTP/SSE initialization:
runtime.mcp.description->ServerInstructionssrc/Azure.DataApiBuilder.Mcp/Core/McpServiceCollectionExtensions.cssrc/Azure.DataApiBuilder.Mcp/Core/McpServerConfiguration.csAdded/updated regression coverage for initialize response instructions:
src/Service.Tests/Configuration/ConfigurationTests.csTestMcpInitializeIncludesInstructionsFromRuntimeDescriptionHow the bug was simulated:
initialize;result.instructionswas missing.How it was verified after fix:
initializeagain, and confirmedresult.instructionsis present.How was this tested?
Ran:
dotnet test src/Service.Tests/Azure.DataApiBuilder.Service.Tests.csproj --filter "FullyQualifiedName~ConfigurationTests.TestMcpInitializeIncludesInstructionsFromRuntimeDescription" -v minimalSample Request(s)
MCP initialize request (HTTP):
Expected response snippet after fix:
{ "result": { "instructions": "Use SQL tools to query the database." } }