Skip to content

Fix: remove .enabled suffix from individual DML tool configure option #3408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
souvikghosh04 wants to merge 2 commits into
base: main
Choose a base branch
from
+130 −7
Open

Fix: remove .enabled suffix from individual DML tool configure option #3408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6690fc
fix: remove .enabled suffix from individual DML tool configure option…
souvikghosh04 Apr 2, 2026
4d5553a
Copilot review fixes
souvikghosh04 Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions src/Cli.Tests/ConfigureOptionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,96 @@ public void TestConfigureDescriptionForMcpSettings(string descriptionValue)
Assert.AreEqual(descriptionValue, runtimeConfig.Runtime.Mcp.Description);
}

/// <summary>
/// Tests that running "dab configure --runtime.mcp.dml-tools.{tool} {value}" updates
/// the individual DML tool boolean in the runtime config. Most tools are configured as
/// direct booleans in the schema (e.g., "describe-entities": true) rather than nested
/// objects with .enabled; "aggregate-records" additionally supports an object form
/// when specifying a query-timeout.
/// </summary>
[DataTestMethod]
[DataRow(true, DisplayName = "Enable individual DML tool: describe-entities")]
[DataRow(false, DisplayName = "Disable individual DML tool: describe-entities")]
public void TestConfigureIndividualDmlToolForMcpSettings(bool updatedValue)
{
// Arrange
SetupFileSystemWithInitialConfig(INITIAL_CONFIG);

// Act: Set describe-entities via the corrected option name (no .enabled suffix)
Copy link
Copy Markdown
Collaborator

@Aniruddh25 Aniruddh25 Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

ConfigureOptions options = new(
runtimeMcpDmlToolsDescribeEntitiesEnabled: updatedValue,
config: TEST_RUNTIME_CONFIG_FILE
);
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);

// Assert
Assert.IsTrue(isSuccess);
string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE);
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? runtimeConfig));
Assert.IsNotNull(runtimeConfig.Runtime?.Mcp?.DmlTools);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.DescribeEntities);
}

/// <summary>
/// Tests that running "dab configure --runtime.mcp.dml-tools.enabled {value}" sets all
/// DML tools at once via the bulk toggle.
/// </summary>
[DataTestMethod]
[DataRow(true, DisplayName = "Enable all DML tools at once")]
[DataRow(false, DisplayName = "Disable all DML tools at once")]
public void TestConfigureAllDmlToolsForMcpSettings(bool updatedValue)
{
// Arrange
SetupFileSystemWithInitialConfig(INITIAL_CONFIG);

// Act: Set all tools via the bulk enabled toggle
ConfigureOptions options = new(
runtimeMcpDmlToolsEnabled: updatedValue,
config: TEST_RUNTIME_CONFIG_FILE
);
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);

// Assert
Assert.IsTrue(isSuccess);
string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE);
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? runtimeConfig));
Assert.IsNotNull(runtimeConfig.Runtime?.Mcp?.DmlTools);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.DescribeEntities);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.CreateRecord);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.ReadRecords);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.UpdateRecord);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.DeleteRecord);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.ExecuteEntity);
Assert.AreEqual(updatedValue, runtimeConfig.Runtime.Mcp.DmlTools.AggregateRecords);
}

/// <summary>
/// Tests that running "dab configure" with multiple individual DML tool options
/// correctly updates each tool independently.
/// </summary>
[TestMethod]
public void TestConfigureMultipleIndividualDmlToolsForMcpSettings()
{
// Arrange
SetupFileSystemWithInitialConfig(INITIAL_CONFIG);

// Act: Enable describe-entities, disable create-record
ConfigureOptions options = new(
runtimeMcpDmlToolsDescribeEntitiesEnabled: true,
runtimeMcpDmlToolsCreateRecordEnabled: false,
config: TEST_RUNTIME_CONFIG_FILE
);
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);

// Assert
Assert.IsTrue(isSuccess);
string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE);
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? runtimeConfig));
Assert.IsNotNull(runtimeConfig.Runtime?.Mcp?.DmlTools);
Assert.AreEqual(true, runtimeConfig.Runtime.Mcp.DmlTools.DescribeEntities);
Assert.AreEqual(false, runtimeConfig.Runtime.Mcp.DmlTools.CreateRecord);
}

/// <summary>
/// Validates that `dab configure --show-effective-permissions` correctly displays
/// effective permissions without modifying the config file.
Expand Down
33 changes: 33 additions & 0 deletions src/Cli.Tests/EndToEndTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1271,4 +1271,37 @@ public void TestUpdateDatabaseType(string dbType, bool isSuccess)
// Assert
Assert.AreEqual(isSuccess, isError == 0);
}

/// <summary>
/// End-to-end test verifying that the corrected CLI option names for individual
/// MCP DML tools (without the .enabled suffix) are correctly parsed by
/// CommandLineParser and produce the expected config output.
/// </summary>
[DataTestMethod]
[DataRow("--runtime.mcp.dml-tools.describe-entities", "true", DisplayName = "E2E: configure describe-entities via CLI")]
[DataRow("--runtime.mcp.dml-tools.create-record", "false", DisplayName = "E2E: configure create-record via CLI")]
[DataRow("--runtime.mcp.dml-tools.read-records", "true", DisplayName = "E2E: configure read-records via CLI")]
[DataRow("--runtime.mcp.dml-tools.update-record", "true", DisplayName = "E2E: configure update-record via CLI")]
[DataRow("--runtime.mcp.dml-tools.delete-record", "false", DisplayName = "E2E: configure delete-record via CLI")]
[DataRow("--runtime.mcp.dml-tools.execute-entity", "true", DisplayName = "E2E: configure execute-entity via CLI")]
[DataRow("--runtime.mcp.dml-tools.aggregate-records", "false", DisplayName = "E2E: configure aggregate-records via CLI")]
public void TestConfigureIndividualDmlToolViaCli(string optionName, string value)
{
// Initialize the config file.
string[] initArgs = { "init", "-c", TEST_RUNTIME_CONFIG_FILE, "--host-mode", "development", "--database-type",
"mssql", "--connection-string", TEST_ENV_CONN_STRING };
Program.Execute(initArgs, _cliLogger!, _fileSystem!, _runtimeConfigLoader!);

Assert.IsTrue(_runtimeConfigLoader!.TryLoadConfig(TEST_RUNTIME_CONFIG_FILE, out RuntimeConfig? runtimeConfig));
Assert.IsNotNull(runtimeConfig);

// Act: Run configure with the individual DML tool option through the full CLI parsing path.
string[] runtimeArgs = { "configure", "-c", TEST_RUNTIME_CONFIG_FILE, optionName, value };
int exitCode = Program.Execute(runtimeArgs, _cliLogger!, _fileSystem!, _runtimeConfigLoader!);

// Assert: Command succeeds and the config contains MCP DML tools section.
Assert.AreEqual(0, exitCode);
Assert.IsTrue(_runtimeConfigLoader!.TryLoadConfig(TEST_RUNTIME_CONFIG_FILE, out RuntimeConfig? updatedConfig));
Assert.IsNotNull(updatedConfig?.Runtime?.Mcp?.DmlTools);
}
}
14 changes: 7 additions & 7 deletions src/Cli/Commands/ConfigureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,25 +212,25 @@ public ConfigureOptions(
[Option("runtime.mcp.dml-tools.enabled", Required = false, HelpText = "Enable DAB's MCP DML tools endpoint. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsEnabled { get; }

[Option("runtime.mcp.dml-tools.describe-entities.enabled", Required = false, HelpText = "Enable DAB's MCP describe entities tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.describe-entities", Required = false, HelpText = "Enable DAB's MCP describe entities tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsDescribeEntitiesEnabled { get; }

[Option("runtime.mcp.dml-tools.create-record.enabled", Required = false, HelpText = "Enable DAB's MCP create record tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.create-record", Required = false, HelpText = "Enable DAB's MCP create record tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsCreateRecordEnabled { get; }

[Option("runtime.mcp.dml-tools.read-records.enabled", Required = false, HelpText = "Enable DAB's MCP read record tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.read-records", Required = false, HelpText = "Enable DAB's MCP read record tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsReadRecordsEnabled { get; }

[Option("runtime.mcp.dml-tools.update-record.enabled", Required = false, HelpText = "Enable DAB's MCP update record tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.update-record", Required = false, HelpText = "Enable DAB's MCP update record tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsUpdateRecordEnabled { get; }

[Option("runtime.mcp.dml-tools.delete-record.enabled", Required = false, HelpText = "Enable DAB's MCP delete record tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.delete-record", Required = false, HelpText = "Enable DAB's MCP delete record tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsDeleteRecordEnabled { get; }

[Option("runtime.mcp.dml-tools.execute-entity.enabled", Required = false, HelpText = "Enable DAB's MCP execute entity tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.execute-entity", Required = false, HelpText = "Enable DAB's MCP execute entity tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsExecuteEntityEnabled { get; }

[Option("runtime.mcp.dml-tools.aggregate-records.enabled", Required = false, HelpText = "Enable DAB's MCP aggregate records tool. Default: true (boolean).")]
[Option("runtime.mcp.dml-tools.aggregate-records", Required = false, HelpText = "Enable DAB's MCP aggregate records tool. Default: true (boolean).")]
public bool? RuntimeMcpDmlToolsAggregateRecordsEnabled { get; }

[Option("runtime.mcp.dml-tools.aggregate-records.query-timeout", Required = false, HelpText = "Set the execution timeout in seconds for the aggregate-records MCP tool. Default: 30 (integer). Range: 1-600.")]
Expand Down
Loading