Include compiler error details (file, line, column) in Build action structured response#426
Conversation
…ne, column, code, message) Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/570f2997-b3f4-4ce6-b332-fee45087c67c Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/570f2997-b3f4-4ce6-b332-fee45087c67c Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…de review Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/570f2997-b3f4-4ce6-b332-fee45087c67c Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…tructured response Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/570f2997-b3f4-4ce6-b332-fee45087c67c Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the dotnet_project Build action by returning structured build diagnostics (file/line/column/code/message/severity) alongside the existing raw CLI output, enabling callers to consume compiler/MSBuild errors without re-parsing text output.
Changes:
- Added
BuildDiagnosticandBuildResulttypes and implementedErrorResultFactory.ParseBuildOutput(...)to extract per-diagnostic details from build output. - Updated
dotnet_projectBuild action to always attach aBuildResultasStructuredContent. - Extended tests (unit + scenario) to validate structured build outputs and added a scenario client API to return full
CallToolResult.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| DotNetMcp/Tools/Cli/DotNetCliTools.Project.Consolidated.cs | Attaches BuildResult structured content for Build action responses. |
| DotNetMcp/Errors/ErrorResultFactory.cs | Parses build output into BuildResult and populates ErrorResult file/line/column fields. |
| DotNetMcp/Errors/ErrorResult.cs | Adds file/line/column to ErrorResult; introduces BuildDiagnostic and BuildResult DTOs. |
| DotNetMcp.Tests/Tools/ConsolidatedProjectToolTests.cs | Asserts Build responses include structured content. |
| DotNetMcp.Tests/Scenarios/McpScenarioClient.cs | Adds API to return full CallToolResult (incl. structured content) in scenario tests. |
| DotNetMcp.Tests/Scenarios/BuildErrorScenarioTests.cs | Updates scenario to validate structured errors[] diagnostics details. |
| DotNetMcp.Tests/Errors/ErrorResultFactoryTests.cs | Adds unit tests for ParseBuildOutput and ErrorResult location fields. |
| .config/dotnet-tools.json | Adds dotnet-ef tool entry to the local tool manifest. |
Comments suppressed due to low confidence (1)
DotNetMcp/Errors/ErrorResultFactory.cs:731
- ParseBuildOutput defaults exitCode to 0, so if the output doesn't contain an "Exit Code:" line (e.g., when DotnetProject.Build returns a validation error like "Error: ..." before invoking dotnet), the resulting BuildResult will incorrectly report Success=true. Track whether an exit code was found and fall back to other signals (e.g., any parsed error diagnostics, "Build FAILED" outcome, or leading "Error:") to set Success=false when appropriate.
// Determine exit code from the trailing "Exit Code: N" line.
var exitCode = 0;
foreach (var line in lines)
{
var trimmed = line.TrimStart();
if (trimmed.StartsWith("Exit Code:", StringComparison.OrdinalIgnoreCase))
{
var codeStr = trimmed.Substring("Exit Code:".Length).Trim();
if (int.TryParse(codeStr, out var parsed))
exitCode = parsed;
break;
}
}
var success = exitCode == 0;
| return new BuildResult | ||
| { | ||
| Success = success, | ||
| Project = string.IsNullOrWhiteSpace(project) ? null : project, |
There was a problem hiding this comment.
BuildResult.Project is forwarded from the requested/auto-discovered project path, which can be an absolute local path (WorkspaceDiscovery returns full paths). Consider sanitizing/redacting this value (similar to File/Message) before putting it into structured content to avoid leaking user machine paths.
| return new BuildResult | |
| { | |
| Success = success, | |
| Project = string.IsNullOrWhiteSpace(project) ? null : project, | |
| static string? SanitizeProjectForStructuredContent(string? projectValue) | |
| { | |
| if (string.IsNullOrWhiteSpace(projectValue)) | |
| { | |
| return null; | |
| } | |
| return System.IO.Path.IsPathRooted(projectValue) | |
| ? System.IO.Path.GetFileName(projectValue) | |
| : projectValue; | |
| } | |
| return new BuildResult | |
| { | |
| Success = success, | |
| Project = SanitizeProjectForStructuredContent(project), |
| // Regex to match "Build FAILED" or "Build succeeded" outcome line. | ||
| [GeneratedRegex(@"Build\s+(FAILED|succeeded)", RegexOptions.IgnoreCase)] | ||
| private static partial Regex BuildOutcomeRegex(); | ||
|
|
There was a problem hiding this comment.
BuildOutcomeRegex() is declared but not used by ParseBuildOutput. If you intended to use it as a fallback when the "Exit Code:" line is missing, wire it in; otherwise remove it to avoid dead code and potential analyzer warnings.
This issue also appears on line 716 of the same file.
| // Regex to match "Build FAILED" or "Build succeeded" outcome line. | |
| [GeneratedRegex(@"Build\s+(FAILED|succeeded)", RegexOptions.IgnoreCase)] | |
| private static partial Regex BuildOutcomeRegex(); |
| "dotnet-ef": { | ||
| "version": "10.0.5", | ||
| "commands": [ | ||
| "dotnet-ef" | ||
| ], | ||
| "rollForward": false | ||
| } |
There was a problem hiding this comment.
The local tool manifest (.config/dotnet-tools.json) was modified to add dotnet-ef. This repo’s guidance is to keep the manifest stable and not change tool entries/versions unless explicitly requested; please revert this change if it was incidental (e.g., from running dotnet tool commands).
| "dotnet-ef": { | |
| "version": "10.0.5", | |
| "commands": [ | |
| "dotnet-ef" | |
| ], | |
| "rollForward": false | |
| } |
|
@copilot apply changes based on the comments in this thread |
…ath, fix CodeQL LINQ/ternary hints Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/c59670df-878a-42a5-b128-84a0288337ae Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Applied all review feedback in commit
|
…lute path detection Agent-Logs-Url: https://github.com/jongalloway/dotnet-mcp/sessions/c59670df-878a-42a5-b128-84a0288337ae Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
The
dotnet_projectBuild action returned error/warning counts but not the actual diagnostic messages, forcing AI agents to re-parse raw CLI output to identify what failed and where.Changes
New types
BuildDiagnostic—file,line,column,code,message,severityfor each compiler/MSBuild diagnosticBuildResult— structured build summary:success,project,configuration,errorCount,warningCount,summary,errors,warningsParsing
ErrorResultFactory.ParseBuildOutput(rawOutput, project, configuration)— parses the rawDotNetCommandExecutortext output into aBuildResult, extracting diagnostics via the existingCompilerErrorRegexErrorResultgainsFile?,Line?,Column?fields, now populated inParseErrorLinefrom regex capturesBuild action
DotnetProject(Build path) now returnsBuildResultasStructuredContentalongside the unchanged raw text:{ "success": false, "project": "NewsletterApp.csproj", "configuration": "Release", "errorCount": 1, "warningCount": 0, "summary": "Build FAILED (1 error(s), 0 warning(s))", "errors": [ { "file": "NewsletterApp.Rendering.cs", "line": 9, "column": 51, "code": "CS0246", "message": "The type or namespace name 'IRenderable' could not be found (are you missing a using directive or an assembly reference?)", "severity": "error" } ] }Tests
ParseBuildOutput(success, failure with errors/warnings, null/empty input) andErrorResultlocation fieldsConsolidatedProjectToolTests: assertsStructuredContentis always present on Build responsesBuildErrorScenarioTests: updated to verify structurederrorsarray withfile/line/column/code/messageMcpScenarioClient.CallToolAsync()added to exposeCallToolResult(includingStructuredContent) in scenario tests