Fix: remove .enabled suffix from individual DML tool configure option#3408
Fix: remove .enabled suffix from individual DML tool configure option#3408souvikghosh04 wants to merge 2 commits intomainfrom
Conversation
#3373) The dab configure command used option names like --runtime.mcp.dml-tools.describe-entities.enabled which implied a nested object structure ({ describe-entities: { enabled: true } }). The schema expects direct booleans ({ describe-entities: true }). Removed the .enabled suffix from 7 individual DML tool option names in ConfigureOptions.cs. Added 3 test methods (5 test cases) covering individual, bulk, and multi-tool configure scenarios.
souvikghosh04
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the dab configure CLI option names for individual MCP DML tools to match the runtime config schema by removing the incorrect .enabled suffix (e.g., runtime.mcp.dml-tools.describe-entities.enabled → runtime.mcp.dml-tools.describe-entities). This aligns CLI usage with the expected JSON shape for per-tool booleans under runtime.mcp.dml-tools.
Changes:
- Updated 7 MCP DML tool
ConfigureOptionslong option names to remove the.enabledsuffix. - Added unit tests validating that configuring individual tools and the bulk
runtime.mcp.dml-tools.enabledtoggle updates the runtime config as expected. - Added a multi-option test to ensure multiple per-tool updates apply independently.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Cli/Commands/ConfigureOptions.cs | Renames long option strings for individual MCP DML tool toggles to match schema paths (no .enabled suffix). |
| src/Cli.Tests/ConfigureOptionsTests.cs | Adds tests for per-tool and bulk MCP DML tool configuration updates. |
souvikghosh04
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
aaronburtle,
anushakolan,
rusamant,
sourabh1007,
stuartpa and
vadeveka
as code owners
| // Arrange | ||
| SetupFileSystemWithInitialConfig(INITIAL_CONFIG); | ||
|
|
||
| // Act: Set describe-entities via the corrected option name (no .enabled suffix) |
There was a problem hiding this comment.
The tests in this file do not explicitly check whether the .enabled is present or not in the option name. Rather, it is directly invoking the constructor.
There are already be other tests that verify configuration via the constructor - like the E2E tests. please remove these tests as they are not necessary. Please do a self review of tests that are really necessary
Why make this change?
dab configure individual DML tool options had an extra
.enabledsuffix, producing invalid nested config instead of the direct booleans the schema expects.What is this change?
Removed
.enabledsuffix from 7 individual DML tool names in ConfigureOptions.cs (e.g.,--runtime.mcp.dml-tools.describe-entities.enabledis nowruntime.mcp.dml-tools.describe-entities).Bulk toggle (
--runtime.mcp.dml-tools.enabled)How was this tested?
Sample Request(s)
Fixed: direct boolean, matches schema
dab configure --runtime.mcp.dml-tools.describe-entities trueMultiple tools
dab configure --runtime.mcp.dml-tools.describe-entities true --runtime.mcp.dml-tools.create-record false