[MCP] Initialize metadata providers in stdio mode - #3784
[MCP] Initialize metadata providers in stdio mode#3784Christos Despotakis (despotak) wants to merge 2 commits into
Conversation
dab start --mcp-stdio returns from Program.StartEngine before host.Run(), so
Startup.Configure never executes -- and with it PerformOnConfigChangeAsync, the
only caller of IMetadataProviderFactory.InitializeAsync(). Entity names reach the
tool registry from config, but no entity ever receives a database object, so every
MCP tool call fails with:
Database object for entity '<name>' has not been inferred.
The identical configuration serves the same entity correctly over REST, because
the web path does call host.Run().
This is a side effect of Azure#3676 (Avoid starting web host in MCP stdio mode). That
change was correct in itself -- stdio mode should not bind an HTTP port -- but
PerformOnConfigChangeAsync did more than serve HTTP, and nothing took over its
metadata-initialization duty on the stdio path.
RunMcpStdioHost now initializes the metadata providers itself, before registering
tools. The existing assertions that StartAsync and StopAsync are never called
still hold, so Azure#3676 is preserved; the unit test gains a stub factory and an
assertion that InitializeAsync is invoked exactly once.
Verified against SQL Server: describe_entities and read_records both succeed on a
one-entity and a twenty-eight-entity configuration, and REST is unchanged.
Fixes Azure#3783
Co-authored-by: Νύξ (Nyx) 🌑 <noreply@anthropic.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Restores schema inference for MCP stdio mode by explicitly initializing the metadata providers on the stdio startup path (which bypasses ASP.NET Core Startup.Configure). This addresses the 2.1.x regression where tools were registered from config but all tool calls failed because database objects were never inferred.
Changes:
- Initialize
IMetadataProviderFactoryinsideMcpStdioHelper.RunMcpStdioHostbefore registering MCP tools. - Extend the existing unit test to assert
IMetadataProviderFactory.InitializeAsync()is invoked exactly once in stdio mode.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Service/Utilities/McpStdioHelper.cs | Initializes metadata providers during stdio startup so entities have inferred database objects before tool calls. |
| src/Service.Tests/UnitTests/McpStdioHelperTests.cs | Adds a stub IMetadataProviderFactory and asserts InitializeAsync() is called once without starting the web host. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Core.Services.MetadataProviders.IMetadataProviderFactory metadataProviderFactory = | ||
| host.Services.GetRequiredService<Core.Services.MetadataProviders.IMetadataProviderFactory>(); |
There was a problem hiding this comment.
nit- just import using Azure.DataApiBuilder.Core.Services.MetadataProviders instead of calling the same multiple times.
| // "Database object for entity '<name>' has not been inferred." | ||
| Core.Services.MetadataProviders.IMetadataProviderFactory metadataProviderFactory = | ||
| host.Services.GetRequiredService<Core.Services.MetadataProviders.IMetadataProviderFactory>(); | ||
| metadataProviderFactory.InitializeAsync().GetAwaiter().GetResult(); |
There was a problem hiding this comment.
since this inside a try/finally that only disposes the host, so any initialization exception propagates raw out of RunMcpStdioHost (which otherwise returns bool). Consider wrapping it so a metadata-inference failure produces a clear, logged error over the stdio channel rather than an unhandled exception
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, directly addresses the reported regression in stdio mode, and is covered by a focused unit test that preserves the existing “does not start web host” guarantees.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Why make this change?
Closes #3783.
Summary of the linked issue: in every
2.1.xbuild,dab start --mcp-stdioregisters entities but never infers their database objects, so every MCP tool call fails withDatabase object for entity '<name>' has not been inferred.while the identical config serves the same entity correctly over REST.2.0.12is unaffected.--mcp-stdioRelated: #3676, #3675 (the change this regressed from), and #3430 — see the note at the bottom.
What is this change?
Schema inference is reachable only through the ASP.NET Core startup path, which stdio mode skips:
Program.cs—StartEnginereturns before the host is started:McpStdioHelper.RunMcpStdioHost— initialises the tool registry and nothing else, soStartup.Configurenever runs.Startup.cs:822—Configureis the only place that callsPerformOnConfigChangeAsync(app)(line 866).Startup.cs:1431—PerformOnConfigChangeAsyncis the only caller ofIMetadataProviderFactory.InitializeAsync().So entity names reach the tool registry from config, while
IMetadataProviderFactoryis never initialised and no entity receives a database object. That is exactly the observed split:tools/listsucceeds, every tool call fails.This is a side effect of #3676 "Avoid starting web host in MCP stdio mode". That change was right — stdio mode should not bind an HTTP port — but
PerformOnConfigChangeAsyncdid more than serve HTTP, and nothing took over its metadata-initialisation duty on the stdio path.The fix is 10 lines:
RunMcpStdioHostresolvesIMetadataProviderFactoryfrom DI and initialises it before registering tools.Program.csis untouched, and #3676's behaviour is preserved — the existing assertions thatStartAsync/StopAsyncare never called still pass.How was this tested?
RunMcpStdioHost_DoesNotStartWebHostgains a stubIMetadataProviderFactoryand an assertion thatInitializeAsyncis called exactly once. Its original assertions are unchanged and still pass, so this cannot silently re-introduce the web host.Also verified by hand against SQL Server, on
mainbuilt from source with the pinned SDK (10.0.302), before and after the patch:describe_entities, 1-entity confighas not been inferredsuccess(1 entity)describe_entities, 28-entity confighas not been inferredsuccess(28 entities)read_recordson a tablesuccess, rows returnedGET /api/<entity>HTTP 200HTTP 200(unchanged)dotnet format --verify-no-changeson both touched files exits 0.Sample Request(s)
Driving the stdio server by hand, so no MCP client is involved:
Before:
{"toolName":"describe_entities","status":"error", "error":{"type":"DataApiBuilderError", "message":"Database object for entity 'MyTable' has not been inferred."}}After:
{"entities":[{"name":"MyTable","description":"","fields":[],"permissions":["READ"]}], "count":1,"status":"success"}One note for reviewers
#3430 reports that stdio blocks the
initializeresponse until introspection finishes (~17 s against a remote instance with 53 entities). Restoring inference on this path necessarily brings that latency back — it was only absent because inference was not happening at all. If you would prefer inference to run asynchronously afterinitializereturns, that is a larger change and I am happy to rework this accordingly; this PR deliberately restores correctness first.Investigated and written with Claude Code (Νύξ) 🌑 — the version bracket, the REST/stdio control and the root-cause trace were worked out together. Reviewed and submitted by me.