Fix MCP CLI logging: apply colors and respect config log-level#3508
Open
anushakolan wants to merge 4 commits intomainfrom
Open
Fix MCP CLI logging: apply colors and respect config log-level#3508anushakolan wants to merge 4 commits intomainfrom
anushakolan wants to merge 4 commits intomainfrom
Conversation
anushakolan
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
aaronburtle,
rusamant,
sourabh1007,
souvikghosh04,
stuartpa and
vadeveka
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes CLI logging behavior when running Data API Builder in MCP stdio mode by (1) ensuring CLI startup logs respect an explicitly configured runtime.telemetry.log-level even when no --LogLevel flag is provided, and (2) applying the same color formatting for CLI log prefixes when writing to stderr in MCP mode.
Changes:
- Added CLI globals to track whether the runtime config explicitly overrides log level, and what that level is (
Utils.IsLogLevelOverriddenByConfig,Utils.ConfigLogLevel). - Updated CLI startup/config loading to set these globals when
RuntimeConfig.HasExplicitLogLevel()is true. - Updated
CustomLoggerProviderto use CLI → config → none precedence in MCP stdio mode and to apply color formatting for stderr output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Cli/Utils.cs | Adds static state to track config-based log level overrides for CLI logging behavior. |
| src/Cli/ConfigGenerator.cs | Detects explicit log level in config and sets CLI globals so MCP stdio mode can emit CLI logs accordingly. |
| src/Cli/CustomLoggerProvider.cs | Implements CLI/config precedence for MCP stdio mode and applies colors consistently when logging to stderr. |
Aniruddh25
approved these changes
May 6, 2026
souvikghosh04
approved these changes
May 8, 2026
Contributor
souvikghosh04
left a comment
souvikghosh04
left a comment
There was a problem hiding this comment.
Approved with some comments and suggestions.
Comment on lines
+124
to
+132
| // Apply colors so the abbreviation matches the visual style of engine logs. | ||
| ConsoleColor mcpOriginalForeGroundColor = Console.ForegroundColor; | ||
| ConsoleColor mcpOriginalBackGroundColor = Console.BackgroundColor; | ||
| Console.ForegroundColor = _logLevelToForeGroundConsoleColorMap.GetValueOrDefault(logLevel, ConsoleColor.White); | ||
| Console.BackgroundColor = _logLevelToBackGroundConsoleColorMap.GetValueOrDefault(logLevel, ConsoleColor.Black); | ||
| Console.Error.Write($"{mcpAbbreviation}:"); | ||
| Console.ForegroundColor = mcpOriginalForeGroundColor; | ||
| Console.BackgroundColor = mcpOriginalBackGroundColor; | ||
| Console.Error.WriteLine($" {formatter(state, exception)}"); |
Contributor
There was a problem hiding this comment.
should the original foreground/background color be restored to default after the specific scenario exits? also, what if there is an unknown exception and should the colour be restored back to default?
| Cli.Utils.IsLogLevelOverriddenByConfig = true; | ||
| Cli.Utils.ConfigLogLevel = LogLevel.Information; | ||
|
|
||
| (string stdout, string stderr) = CaptureConsole(() => |
Contributor
There was a problem hiding this comment.
in these tests, does CaptureConsole() capture the colour info or is there a way to add some tests for them?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
Two bugs were discovered in the CLI logging behavior when running DAB in MCP stdio mode:
start --mcp-stdio --LogLevel <level>was used, the CLI's own startup logs (e.g., "Setting minimum LogLevel: Information.") appeared on stderr without any colors, while engine logs were properly colored. This created an inconsistent visual experience.start --mcp-stdiowas used with theruntime.telemetry.log-levelset in the config file (and no--LogLevelCLI flag), CLI startup logs were completely suppressed, even though the user had clearly expressed an intent to see logs at that level.What is this change?
The CLI logger (
CustomLoggerProvider) only knew about CLI overrides (--LogLevel). It never learned that the runtime config file can also explicitly request a log level. As a result:LogLevel.None(suppress all) whenever--LogLevelwas absent in MCP mode, ignoring the config.This change makes the CLI logger aware of both override sources and applies colors consistently in both stdout (non-MCP) and stderr (MCP) paths.
Modified files:
src/Cli/Utils.cs— AddedIsLogLevelOverriddenByConfig(bool) andConfigLogLevel(LogLevel) static properties.src/Cli/ConfigGenerator.cs— InTryStartEngineWithOptions(), sets the new properties whenRuntimeConfig.HasExplicitLogLevel()is true.src/Cli/CustomLoggerProvider.cs:Log(): suppresses only when neither CLI nor config set a level; applies foreground/background colors before writing the abbreviation to stderr in MCP mode.How was this tested?
Manually tested all combinations of MCP / non-MCP × CLI override / config override / no override:
--LogLevel Informationinfo:log-levelonlyinfo:--LogLevel Warning+ configlog-level: Informationinfo:shown--LogLevel Warning(StaticWebApps config)warn:shownNote
This is a follow-up bug fix on top of PR #3419 (MCP Set Log Level) and PR #3484 (MCP notifications/message). It targets
main.