Bug fix for setting dml tools based on bool value as set in config

[souvikghosh04]

Why make this change?

Bug fix for setting dml tools correctly.

Closes on - #2942

What is this change?

This fix addresses the inconsistency in config where dml-tools can be either set as bool or a dictionary object containing individual tool names and their bool values. Without this fix, we need to individually set each dml-tools to true currently.

With this change, we will have the following scenarios-

• set dml-tools to true will enable all dml tools (default is true)
  Sample- "dml-tools": true

• set dml-tools to false will disable all dml tools
  Sample-"dml-tools": false

• individual tool names can be specified as dictionary object for dml-tools to turn them on (default is true if a tool is not specified).

How was this tested?

• Integration Tests
• Unit Tests
• Manual testing using various combinations and scenarios

Scenario 1: Enable all tools using a single boolean value

"dml-tools": true

Note: default is true so even if dml-tools unspecified it will default to true.

Scenario 2: Disable execute-entities only and keep other tools enabled as default.

      "dml-tools": {
        "execute-entities":  false
      }

Scenario 3: Use full list of tools and enable or disable them

      "dml-tools": {
        "execute-entity": true,
        "delete-record": false,
        "update-record": false,
        "read-records": false,
        "describe-entities": true
      }

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Copilot]

Pull Request Overview

This PR fixes the FromBoolean method in DmlToolsConfig to properly set individual tool properties. Previously, when creating a config using FromBoolean, all individual tool properties were set to null, which was inconsistent with the method's intent of setting all tools to the same state.

Key changes:

• Updated FromBoolean to explicitly set all individual tool properties (DescribeEntities, CreateRecord, ReadRecords, UpdateRecord, DeleteRecord, ExecuteEntity) to the provided enabled value instead of null

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Aniruddh25]

Approving since this is not harmful, but also waiting on clarification on need of AllToolsEnabled

[Aniruddh25]

Actually, the copilot feedback comment is useful. Please check it out. I think serialization will be a problem here - since this is not tested.

[Aniruddh25]

Manual testing using various combinations and scenarios

Can you please list out what combinations were tested here to understand if we missed a scenario?

[Copilot]

@souvikghosh04 I've opened a new pull request, #2934, to work on those changes. Once the pull request is ready, I'll request review from you.

[souvikghosh04]

/azp run

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[souvikghosh04]

Approving. But the description is not clear - it says default is false if not specified. I thought default was true for all the dml-tools even when we use the dictionary format of object for the dml-tools property.

this we are handling it by in McpRuntimeOptions.cs constructor-

        // if DmlTools is null, set All tools enabled by default
        if (DmlTools is null)
        {
            this.DmlTools = DmlToolsConfig.FromBoolean(true);
        }
        else
        {
            this.DmlTools = DmlTools;
        }

so the description needs to be fixed, It shouldnt say default is false if not specified.

the default here refers to the unspecified tools which will be set to false by default. the default value of dml-tools is true if unspecified but default value of individual tools will be false if unspecified. I have the updated the description and example.

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

Approving. But the description is not clear - it says default is false if not specified. I thought default was true for all the dml-tools even when we use the dictionary format of object for the dml-tools property.

this we are handling it by in McpRuntimeOptions.cs constructor-

        // if DmlTools is null, set All tools enabled by default
        if (DmlTools is null)
        {
            this.DmlTools = DmlToolsConfig.FromBoolean(true);
        }
        else
        {
            this.DmlTools = DmlTools;
        }

so the description needs to be fixed, It shouldnt say default is false if not specified.

the default here refers to the unspecified tools which will be set to false by default. the default value of dml-tools is true if unspecified but default value of individual tools will be false if unspecified. I have the updated the description and example.

As discussed offline, this is incorrect behavior. Default value of unspecified tools in the dictionary object should be true. The PR needs to be fixed to reflect that.

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

LGTM, please fix the description.

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[souvikghosh04]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[anushakolan]

LGTM, but the MS SQL Integration tests are still failing.