Skip to content

[MCP]Fixed CLI bug in fields.primary-key updation when using update entity command. #3077

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
anushakolan wants to merge 4 commits into
base: main
Choose a base branch
from
+163 −1
Open

[MCP]Fixed CLI bug in fields.primary-key updation when using update entity command. #3077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a440af
[MCP]Fixed CLI bug in ields.primary-key updation when using update e…
anushakolan Jan 23, 2026
1669dbf
Merge branch 'main' into dev/anushakolan/cli-suprise-behavior
anushakolan Jan 23, 2026
c881404
Addressed comments
anushakolan Jan 26, 2026
c7ae4f1
Merge branch 'main' into dev/anushakolan/cli-suprise-behavior
anushakolan Jan 27, 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
158 changes: 158 additions & 0 deletions src/Cli.Tests/UpdateEntityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,164 @@ public void TestUpdateEntityDescription()
Assert.AreEqual("Updated description", updatedRuntimeConfig.Entities["MyEntity"].Description);
}

/// <summary>
/// Updating a field's description without --fields.primary-key
/// should not change its existing primary-key flag.
/// </summary>
[TestMethod]
public void TestUpdateFieldDescriptionPreservesPrimaryKeyWhenNoFlagProvided()
Copy link
Contributor

@RubenCerna2079 RubenCerna2079 Jan 28, 2026

Choose a reason for hiding this comment

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

I see that both tests are basically the same, I think it would be better for them to be the same test and just have a variable that changes the fieldsPrimaryKeyCollection field.

{
string initialConfig = GetInitialConfigString() + "," + @"
""entities"": {
""MyEntity"": {
""source"": ""MyTable"",
""fields"": [
{
""name"": ""Id"",
""description"": ""Primary key"",
""primary-key"": true
}
],
""permissions"": [
{
""role"": ""anonymous"",
""actions"": [""read""]
}
]
}
}
}";

UpdateOptions options = new(
source: null,
permissions: null,
entity: "MyEntity",
sourceType: null,
sourceParameters: null,
sourceKeyFields: null,
restRoute: null,
graphQLType: null,
fieldsToInclude: null,
fieldsToExclude: null,
policyRequest: null,
policyDatabase: null,
relationship: null,
cardinality: null,
targetEntity: null,
linkingObject: null,
linkingSourceFields: null,
linkingTargetFields: null,
relationshipFields: null,
map: null,
cacheEnabled: null,
cacheTtl: null,
config: TEST_RUNTIME_CONFIG_FILE,
restMethodsForStoredProcedure: null,
graphQLOperationForStoredProcedure: null,
description: null,
parametersNameCollection: null,
parametersDescriptionCollection: null,
parametersRequiredCollection: null,
parametersDefaultCollection: null,
fieldsNameCollection: new[] { "Id" },
fieldsAliasCollection: null,
fieldsDescriptionCollection: new[] { "Unique Key" },
fieldsPrimaryKeyCollection: null,
mcpDmlTools: null,
mcpCustomTool: null
);

Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(initialConfig, out RuntimeConfig? runtimeConfig), "Parsed config file.");
Assert.IsTrue(TryUpdateExistingEntity(options, runtimeConfig!, out RuntimeConfig updatedRuntimeConfig), "Successfully updated entity in the config.");

Entity updatedEntity = updatedRuntimeConfig.Entities["MyEntity"];
Assert.IsNotNull(updatedEntity.Fields);
Assert.AreEqual(1, updatedEntity.Fields!.Count);
FieldMetadata field = updatedEntity.Fields[0];
Assert.AreEqual("Id", field.Name);
Assert.AreEqual("Unique Key", field.Description);
Assert.IsTrue(field.PrimaryKey, "Primary key flag should be preserved when --fields.primary-key is not provided.");
}

/// <summary>
/// When --fields.primary-key false is explicitly provided, it should
/// change an existing primary-key flag from true to false.
/// </summary>
[TestMethod]
public void TestUpdateFieldPrimaryKeyCanBeClearedWithExplicitFalse()
{
string initialConfig = GetInitialConfigString() + "," + @"
""entities"": {
""MyEntity"": {
""source"": ""MyTable"",
""fields"": [
{
""name"": ""Id"",
""description"": ""Primary key"",
""primary-key"": true
}
],
""permissions"": [
{
""role"": ""anonymous"",
""actions"": [""read""]
}
]
}
}
}";

UpdateOptions options = new(
source: null,
permissions: null,
entity: "MyEntity",
sourceType: null,
sourceParameters: null,
sourceKeyFields: null,
restRoute: null,
graphQLType: null,
fieldsToInclude: null,
fieldsToExclude: null,
policyRequest: null,
policyDatabase: null,
relationship: null,
cardinality: null,
targetEntity: null,
linkingObject: null,
linkingSourceFields: null,
linkingTargetFields: null,
relationshipFields: null,
map: null,
cacheEnabled: null,
cacheTtl: null,
config: TEST_RUNTIME_CONFIG_FILE,
restMethodsForStoredProcedure: null,
graphQLOperationForStoredProcedure: null,
description: null,
parametersNameCollection: null,
parametersDescriptionCollection: null,
parametersRequiredCollection: null,
parametersDefaultCollection: null,
fieldsNameCollection: new[] { "Id" },
fieldsAliasCollection: null,
fieldsDescriptionCollection: new[] { "Unique Key" },
fieldsPrimaryKeyCollection: new[] { false },
mcpDmlTools: null,
mcpCustomTool: null
);

Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(initialConfig, out RuntimeConfig? runtimeConfig), "Parsed config file.");
Assert.IsTrue(TryUpdateExistingEntity(options, runtimeConfig!, out RuntimeConfig updatedRuntimeConfig), "Successfully updated entity in the config.");

Entity updatedEntity = updatedRuntimeConfig.Entities["MyEntity"];
Assert.IsNotNull(updatedEntity.Fields);
Assert.AreEqual(1, updatedEntity.Fields!.Count);
FieldMetadata field = updatedEntity.Fields[0];
Assert.AreEqual("Id", field.Name);
Assert.AreEqual("Unique Key", field.Description);
Assert.IsFalse(field.PrimaryKey, "Primary key flag should be cleared when --fields.primary-key false is provided.");
}

private static string GetInitialConfigString()
{
return @"{" +
Expand Down
6 changes: 5 additions & 1 deletion src/Cli/ConfigGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1747,6 +1747,7 @@ public static bool TryUpdateExistingEntity(UpdateOptions options, RuntimeConfig
List<FieldMetadata> updatedFieldsList = ComposeFieldsFromOptions(options);
Dictionary<string, FieldMetadata> updatedFieldsDict = updatedFieldsList.ToDictionary(f => f.Name, f => f);
List<FieldMetadata> mergedFields = [];
bool primaryKeyOptionProvided = options.FieldsPrimaryKeyCollection?.Any() == true;

foreach (FieldMetadata field in existingFields)
{
Expand All @@ -1757,7 +1758,10 @@ public static bool TryUpdateExistingEntity(UpdateOptions options, RuntimeConfig
Name = updatedField.Name,
Alias = updatedField.Alias ?? field.Alias,
Description = updatedField.Description ?? field.Description,
PrimaryKey = updatedField.PrimaryKey
// If --fields.primary-key was not provided at all,
// keep the existing primary-key flag. Otherwise,
// use the value coming from updatedField.
PrimaryKey = primaryKeyOptionProvided ? updatedField.PrimaryKey : field.PrimaryKey
});
updatedFieldsDict.Remove(field.Name); // Remove so only new fields remain
}
Expand Down
Loading