mcp icon indicating copy to clipboard operation
mcp copied to clipboard

Prevent tools from being added without consolidated mode

Open joshfree opened this issue 1 month ago • 4 comments

Today new tool PRs are very likely to break the build since CI doesn't guard against tools being added to the server but forgetting to register them for consolidated server mode.

I think we've had at least 5 breaks now because of this CI gap.

This issue is to update CI to stop build breaks due to new tools not being added to consolidated mode.

joshfree avatar Nov 16 '25 20:11 joshfree

@fanyang-mono -

I know we've recently added these -

11/11 - new-command documentation - https://github.com/microsoft/mcp/blob/main/servers/Azure.Mcp.Server/docs/new-command.md#consolidated-mode-requirements

11/14 - PR checklist - https://github.com/microsoft/mcp/pull/1185

Lets brainstorm if there are additional ways to catch this prior to CI

kvenkatrajan avatar Nov 17 '25 13:11 kvenkatrajan

There is a couple things we could do here to prevent new tools being added but not included for consolidate mode:

  • Fasted to implement: Enable this live test to run for all PR: https://github.com/microsoft/mcp/blob/dd130f4d629103f2262cd9a53e654a8850e665d5/core/Azure.Mcp.Core/tests/Azure.Mcp.Core.LiveTests/Areas/Server/ServerCommandTests.cs#L583
    • I've added a unit test: https://github.com/microsoft/mcp/blob/dd130f4d629103f2262cd9a53e654a8850e665d5/core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/Discovery/ConsolidatedToolDiscoveryStrategyTests.cs#L14 Ideally, it is supposed to catch all the issues. But somehow it is not. I need to investigate it.
  • Alternatively, a static check can be added to compare all the available tools vs tools being included in consolidated-tools.json

fanyang-mono avatar Nov 17 '25 15:11 fanyang-mono

Thanks @fanyang-mono

As discussed, we now have a repro and should look at investigating why the unit test didnt fail for this PR - https://github.com/microsoft/mcp/pull/1096

kvenkatrajan avatar Nov 17 '25 16:11 kvenkatrajan

Lets brainstorm if there are additional ways to catch this prior to CI

Yeah the PR-checklist is insufficient as a quality gate to prevent bugs. We need a test enabled to fail CI.

joshfree avatar Nov 17 '25 17:11 joshfree