Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions src/Cli.Tests/ValidateConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,4 +359,29 @@ private async Task ValidatePropertyOptionsFails(ConfigureOptions options)
JsonSchemaValidationResult result = await validator.ValidateConfigSchema(config, TEST_RUNTIME_CONFIG_FILE, mockLoggerFactory.Object);
Assert.IsFalse(result.IsValid);
}

/// <summary>
/// Validates that WarnIfNoEntitiesDefined logs a warning when the config has
/// an empty entities collection and no autoentities or data-source-files.
/// This calls the extracted method directly, avoiding any DB connection.
/// </summary>
[TestMethod]
public void TestValidateWarnsOnZeroEntities()
{
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(INITIAL_CONFIG, out RuntimeConfig? config));

Mock<ILogger<ConfigGenerator>> mockLogger = new();
SetLoggerForCliConfigGenerator(mockLogger.Object);

ConfigGenerator.WarnIfNoEntitiesDefined(config!);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of directly calling the WarnIfNoEntitiesDefined function, test the scenario for which the bug is identified.
In this case, can we call IsConfigValid and verify the warning is printed?


mockLogger.Verify(
x => x.Log(
LogLevel.Warning,
It.IsAny<EventId>(),
It.Is<It.IsAnyType>((v, t) => v.ToString()!.Contains("No entities are defined in this configuration.")),
It.IsAny<Exception?>(),
It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
Times.Once);
}
}
22 changes: 19 additions & 3 deletions src/Cli/ConfigGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2648,10 +2648,13 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi

bool isValid = runtimeConfigValidator.TryValidateConfig(runtimeConfigFile, LoggerFactoryForCli).Result;

// Additional validation: warn if fields are missing and MCP is enabled
if (isValid)
if (runtimeConfigProvider.TryGetConfig(out RuntimeConfig? config) && config is not null)
{
if (runtimeConfigProvider.TryGetConfig(out RuntimeConfig? config) && config is not null)
// Run config-structure warnings regardless of validation outcome.
WarnIfNoEntitiesDefined(config);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if there are 0 entities, or autoentities, it is still a valid configuration.

But do we still need to check whether any entity has missing fields when MCP is enabled - i.e. perform the below validation?

We should still Warn if Unauthenticated provider is used with authenticated or custom roles

Copy link
Copy Markdown
Contributor Author

@aaronburtle aaronburtle Apr 3, 2026

Choose a reason for hiding this comment

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

If there are no entities, then the for loop below iterates zero times and this is a no-op, so there wont really be any validation happening; there is nothing to validate. On the other hand, if validation fails for some other reason the user wont see any "missing fields with MCP" warnings, which may be an enhancement we want to add, but shouldn't be scoped to this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar to unauthenticated provider with non-anonymous roles. Since it is inside if(isValid) it only fires when everything else passes. If we move it outside then it would always run, but with 0 entities, the warning can never trigger.


// Additional validation: warn if fields are missing and MCP is enabled
if (isValid)
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.

Could you please clarify with @JerryNixon, if it is a config validation error or warning in case:

  1. entities/autoentities both sections are completely missing - i.e. null from the config -> we error out today on this case.
  2. entities/autoentities - either is present but count is 0 - no validation error - but warning emitted as per this PR.

{
bool mcpEnabled = config.IsMcpEnabled;
if (mcpEnabled)
Expand Down Expand Up @@ -3622,5 +3625,18 @@ private static bool ValidateFields(

return true;
}

/// <summary>
/// Logs a warning if no entities, autoentities, or data-source-files are configured.
/// </summary>
internal static void WarnIfNoEntitiesDefined(RuntimeConfig config)
{
if (config.Entities.Entities.Count == 0
&& config.Autoentities.Autoentities.Count == 0
&& (config.DataSourceFiles?.SourceFiles is null || !config.DataSourceFiles.SourceFiles.Any()))
{
_logger.LogWarning("No entities are defined in this configuration.");
}
}
}
}