Port CI analysis to C# from powershell#125871
Conversation
Add Get-CIStatus.cs as a C# single-file script (dotnet run) equivalent of the existing Get-CIStatus.ps1. This removes the PowerShell dependency since only the .NET SDK is needed. Supports all three modes: PR analysis, BuildId analysis, and direct Helix job queries with the same CLI flags as the PS1 version. Update SKILL.md to document both scripts and recommend the C# version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
FYI @jjonescz I think this is what's necessary to fully isolate a C# script from the surrounding environment: Directory.Build.props, Directory.Build.targets, global.json. Feels pretty expensive. I wonder if there's a way to make this simpler. So far I've not wanted scripts to import these things by default -- just the opposite, actually. |
There was a problem hiding this comment.
Pull request overview
Ports the ci-analysis skill’s CI failure investigation script from PowerShell to a C# (dotnet run) implementation to avoid a PowerShell dependency on non-Windows systems.
Changes:
- Removed the PowerShell-based
Get-CIStatus.ps1and added a C# portGet-CIStatus.cs. - Added local MSBuild/global.json scaffolding under
.github/skills/ci-analysis/scriptsto support running the script via the .NET SDK. - Updated
ci-analysisskill documentation to reference the new C# script and invocation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
.github/skills/ci-analysis/scripts/global.json |
Pins/configures an SDK for running the script from the scripts/ directory. |
.github/skills/ci-analysis/scripts/Get-CIStatus.ps1 |
Removes the previous PowerShell implementation. |
.github/skills/ci-analysis/scripts/Get-CIStatus.cs |
Introduces the new C# implementation of CI/Helix/AzDO analysis. |
.github/skills/ci-analysis/scripts/Directory.Build.targets |
Adds local MSBuild targets file (currently empty). |
.github/skills/ci-analysis/scripts/Directory.Build.props |
Adds local MSBuild props file (currently empty). |
.github/skills/ci-analysis/SKILL.md |
Updates documentation and quick-start commands to use the new C# script. |
| var workItemName = ExtractWorkItemFromUrl(url); | ||
| var helixLog = await CachedGet(url); | ||
| if (helixLog is not null) | ||
| { | ||
| var failureInfo = FormatTestFailure(helixLog); | ||
| if (failureInfo is not null) | ||
| { | ||
| WriteColor(failureInfo, ConsoleColor.White); | ||
| await ShowKnownIssues(workItemName, failureInfo); | ||
| } | ||
| } |
There was a problem hiding this comment.
-MaxFailureLines is parsed into options.MaxFailureLines, but the value is never used: FormatTestFailure is called without passing options.MaxFailureLines, and the helper defaults to maxLines = 50. Wire options.MaxFailureLines through so the CLI flag actually affects output.
| if (cached is not null) return cached; | ||
| } | ||
|
|
||
| var response = await http.GetAsync(url); |
There was a problem hiding this comment.
CachedGet doesn't dispose the HttpResponseMessage. Over many requests this can lead to socket/resource exhaustion. Wrap the response in a using (or await using) and keep the current content-reading logic.
| var response = await http.GetAsync(url); | |
| using var response = await http.GetAsync(url); |
|
|
||
| | Script | Runtime | Command | | ||
| |--------|---------|---------| | ||
| | `Get-CIStatus.cs` | .NET 10+ (`dotnet run`) | `dotnet run scripts/Get-CIStatus.cs -- -BuildId 123` | |
There was a problem hiding this comment.
The table says the script runs on “.NET 10+”, but the repo’s root global.json currently pins a .NET 11 preview SDK. Unless this script is intentionally designed to run independently of the repo SDK selection, this runtime requirement looks inaccurate and may mislead users running it in-tree.
| | `Get-CIStatus.cs` | .NET 10+ (`dotnet run`) | `dotnet run scripts/Get-CIStatus.cs -- -BuildId 123` | | |
| | `Get-CIStatus.cs` | Repo .NET SDK (`dotnet run`, currently .NET 11 preview via `global.json`) | `dotnet run scripts/Get-CIStatus.cs -- -BuildId 123` | |
| // Multi-build summary | ||
| if (buildIds.Count > 1) | ||
| { | ||
| WriteColor("\n=== Overall Summary ===", ConsoleColor.Magenta); | ||
| WriteColor($"Analyzed {buildIds.Count} builds", ConsoleColor.White); | ||
| WriteColor($"Total failed jobs: {totalFailedJobs}", ConsoleColor.Red); | ||
| if (knownIssuesFromBuildAnalysis.Count > 0) | ||
| { | ||
| WriteColor("\nKnown Issues (from Build Analysis):", ConsoleColor.Yellow); | ||
| foreach (var issue in knownIssuesFromBuildAnalysis) | ||
| { | ||
| WriteColor($" - #{issue.Number}: {issue.Title}", ConsoleColor.Gray); | ||
| WriteColor($" {issue.Url}", ConsoleColor.DarkGray); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Recommendation | ||
| WriteColor("\n=== Recommendation ===", ConsoleColor.Magenta); | ||
| if (knownIssuesFromBuildAnalysis.Count > 0) | ||
| { | ||
| WriteColor("KNOWN ISSUES DETECTED", ConsoleColor.Yellow); | ||
| WriteColor($"{knownIssuesFromBuildAnalysis.Count} tracked issue(s) found that may correlate with failures above.", ConsoleColor.White); | ||
| } | ||
| else if (totalFailedJobs == 0 && totalLocalFailures == 0) | ||
| { | ||
| WriteColor("BUILD SUCCESSFUL", ConsoleColor.Green); | ||
| WriteColor("No failures detected.", ConsoleColor.White); | ||
| } | ||
| else if (prChangedFiles.Count > 0 && HasPRCorrelation(prChangedFiles, allFailuresForCorrelation)) | ||
| { | ||
| WriteColor("LIKELY PR-RELATED", ConsoleColor.Red); | ||
| WriteColor("Failures appear to correlate with files changed in this PR.", ConsoleColor.White); | ||
| } | ||
| else if (prChangedFiles.Count > 0) | ||
| { | ||
| WriteColor("POSSIBLY TRANSIENT", ConsoleColor.Yellow); | ||
| WriteColor("No known issues matched, but failures don't clearly correlate with PR changes.", ConsoleColor.White); | ||
| WriteColor("Consider:", ConsoleColor.Gray); | ||
| WriteColor(" 1. Check if same tests are failing on main branch", ConsoleColor.Gray); | ||
| WriteColor(" 2. Search for existing issues: gh issue list --label 'Known Build Error' --search '<test name>'", ConsoleColor.Gray); | ||
| } | ||
| else | ||
| { | ||
| WriteColor("REVIEW REQUIRED", ConsoleColor.Yellow); | ||
| WriteColor("Could not automatically determine failure cause.", ConsoleColor.White); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
This C# port no longer emits the [CI_ANALYSIS_SUMMARY] JSON block that the skill documentation/workflow expects from the script output. Please restore emitting a structured summary (including no-build / error cases) so agents can reliably parse results without scraping console text.
| async Task<List<string>> GetPRChangedFiles(int pr, int maxFiles = 100) | ||
| { | ||
| try | ||
| { | ||
| var (countStr, _, ec1) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files | length\""); | ||
| if (ec1 != 0) return []; | ||
| if (int.TryParse(countStr.Trim(), out var count) && count > maxFiles) | ||
| { | ||
| WriteColor($"PR has {count} changed files - skipping detailed correlation (limit: {maxFiles})", ConsoleColor.Gray); | ||
| return []; | ||
| } | ||
|
|
||
| var (filesStr, _, ec2) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files[].path\""); | ||
| if (ec2 != 0) return []; | ||
| return filesStr.Split('\n', StringSplitOptions.RemoveEmptyEntries).ToList(); | ||
| } | ||
| catch { return []; } |
There was a problem hiding this comment.
Several methods are marked async but contain no await (e.g., GetBuildIdsFromPR, GetBuildAnalysisKnownIssues, GetPRChangedFiles). This will produce CS1998 warnings and adds unnecessary overhead. Consider removing async and returning Task.FromResult(...), or make the implementations truly async.
| async Task<List<string>> GetPRChangedFiles(int pr, int maxFiles = 100) | |
| { | |
| try | |
| { | |
| var (countStr, _, ec1) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files | length\""); | |
| if (ec1 != 0) return []; | |
| if (int.TryParse(countStr.Trim(), out var count) && count > maxFiles) | |
| { | |
| WriteColor($"PR has {count} changed files - skipping detailed correlation (limit: {maxFiles})", ConsoleColor.Gray); | |
| return []; | |
| } | |
| var (filesStr, _, ec2) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files[].path\""); | |
| if (ec2 != 0) return []; | |
| return filesStr.Split('\n', StringSplitOptions.RemoveEmptyEntries).ToList(); | |
| } | |
| catch { return []; } | |
| Task<List<string>> GetPRChangedFiles(int pr, int maxFiles = 100) | |
| { | |
| try | |
| { | |
| var (countStr, _, ec1) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files | length\""); | |
| if (ec1 != 0) | |
| return Task.FromResult(new List<string>()); | |
| if (int.TryParse(countStr.Trim(), out var count) && count > maxFiles) | |
| { | |
| WriteColor($"PR has {count} changed files - skipping detailed correlation (limit: {maxFiles})", ConsoleColor.Gray); | |
| return Task.FromResult(new List<string>()); | |
| } | |
| var (filesStr, _, ec2) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json files --jq \".files[].path\""); | |
| if (ec2 != 0) | |
| return Task.FromResult(new List<string>()); | |
| var files = filesStr.Split('\n', StringSplitOptions.RemoveEmptyEntries).ToList(); | |
| return Task.FromResult(files); | |
| } | |
| catch | |
| { | |
| return Task.FromResult(new List<string>()); | |
| } |
| async Task<List<int>> GetBuildIdsFromPR(int pr) | ||
| { | ||
| ValidateRepository(options.Repository); | ||
| WriteColor($"Finding builds for PR #{pr} in {options.Repository}...", ConsoleColor.Cyan); | ||
|
|
||
| var (stdout, stderr, exitCode) = RunProcess("gh", $"pr checks {pr} --repo {options.Repository}"); | ||
| var combined = stdout + stderr; | ||
|
|
||
| if (exitCode != 0 && !combined.Contains("buildId=")) | ||
| throw new Exception($"Failed to fetch CI status for PR #{pr} in {options.Repository}"); | ||
|
|
There was a problem hiding this comment.
When gh isn't installed or isn't on PATH, the first RunProcess("gh", ...) call will throw a platform-specific exception and the script will just print a generic error. The previous PowerShell script surfaced a clear prerequisite message. Consider adding an explicit check (or a friendly catch) to print actionable guidance (install gh or use -BuildId).
| var match = Regex.Match(line, @"fail.*buildId=(\d+)"); | ||
| if (match.Success && int.TryParse(match.Groups[1].Value, out var id)) | ||
| { | ||
| var pipelineName = line.Split("\tfail")[0].Trim(); |
There was a problem hiding this comment.
Parsing gh pr checks output uses line.Split("\tfail")[0] to derive the pipeline name. This assumes a tab before fail and will produce incorrect names if the output format changes or uses spaces. Consider using a regex/whitespace split (like the previous PowerShell version) or switch to gh pr checks --json for structured parsing.
| var match = Regex.Match(line, @"fail.*buildId=(\d+)"); | |
| if (match.Success && int.TryParse(match.Groups[1].Value, out var id)) | |
| { | |
| var pipelineName = line.Split("\tfail")[0].Trim(); | |
| var match = Regex.Match(line, @"^(?<name>.+?)\s+fail.*buildId=(?<id>\d+)"); | |
| if (match.Success && int.TryParse(match.Groups["id"].Value, out var id)) | |
| { | |
| var pipelineName = match.Groups["name"].Value.Trim(); |
| { | ||
| "sdk": { | ||
| "version": "10.0.100", | ||
| "rollForward": "latestMajor" |
There was a problem hiding this comment.
This nested global.json pins a stable 10.0.100 SDK but does not set allowPrerelease: true. In this repo, the root global.json currently requires a preview SDK; if only preview SDKs are installed, dotnet typically won't roll forward from a stable request to a preview SDK, so running the script from this directory may fail with “requested SDK not found”. Consider removing this global.json, aligning it with the repo's SDK version, or adding allowPrerelease/a version that can resolve in the repo’s expected environment.
| "rollForward": "latestMajor" | |
| "rollForward": "latestMajor", | |
| "allowPrerelease": true |
|
People have committed various versions of the this skill to multiple repos and it is unmanageable in this state. Instead of making changes here I'd rather address the distribution problem and find a proper home see dotnet/arcade-skills#4 (comment) ( see dotnet/sdk#53526 ) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Seems extremely reasonable to me. How do you want me to deal with this code? Do you want me to push to your open branch, or wait until you merge? |
| // PR correlation | ||
| if (prChangedFiles.Count > 0 && allFailuresForCorrelation.Count > 0) | ||
| ShowPRCorrelation(prChangedFiles, allFailuresForCorrelation); | ||
|
|
||
| // Multi-build summary | ||
| if (buildIds.Count > 1) | ||
| { | ||
| WriteColor("\n=== Overall Summary ===", ConsoleColor.Magenta); | ||
| WriteColor($"Analyzed {buildIds.Count} builds", ConsoleColor.White); | ||
| WriteColor($"Total failed jobs: {totalFailedJobs}", ConsoleColor.Red); | ||
| if (knownIssuesFromBuildAnalysis.Count > 0) | ||
| { | ||
| WriteColor("\nKnown Issues (from Build Analysis):", ConsoleColor.Yellow); | ||
| foreach (var issue in knownIssuesFromBuildAnalysis) | ||
| { | ||
| WriteColor($" - #{issue.Number}: {issue.Title}", ConsoleColor.Gray); | ||
| WriteColor($" {issue.Url}", ConsoleColor.DarkGray); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Recommendation | ||
| WriteColor("\n=== Recommendation ===", ConsoleColor.Magenta); | ||
| if (knownIssuesFromBuildAnalysis.Count > 0) | ||
| { | ||
| WriteColor("KNOWN ISSUES DETECTED", ConsoleColor.Yellow); | ||
| WriteColor($"{knownIssuesFromBuildAnalysis.Count} tracked issue(s) found that may correlate with failures above.", ConsoleColor.White); | ||
| } | ||
| else if (totalFailedJobs == 0 && totalLocalFailures == 0) | ||
| { | ||
| WriteColor("BUILD SUCCESSFUL", ConsoleColor.Green); | ||
| WriteColor("No failures detected.", ConsoleColor.White); | ||
| } | ||
| else if (prChangedFiles.Count > 0 && HasPRCorrelation(prChangedFiles, allFailuresForCorrelation)) | ||
| { | ||
| WriteColor("LIKELY PR-RELATED", ConsoleColor.Red); | ||
| WriteColor("Failures appear to correlate with files changed in this PR.", ConsoleColor.White); | ||
| } | ||
| else if (prChangedFiles.Count > 0) | ||
| { | ||
| WriteColor("POSSIBLY TRANSIENT", ConsoleColor.Yellow); | ||
| WriteColor("No known issues matched, but failures don't clearly correlate with PR changes.", ConsoleColor.White); | ||
| WriteColor("Consider:", ConsoleColor.Gray); | ||
| WriteColor(" 1. Check if same tests are failing on main branch", ConsoleColor.Gray); | ||
| WriteColor(" 2. Search for existing issues: gh issue list --label 'Known Build Error' --search '<test name>'", ConsoleColor.Gray); | ||
| } | ||
| else | ||
| { | ||
| WriteColor("REVIEW REQUIRED", ConsoleColor.Yellow); | ||
| WriteColor("Could not automatically determine failure cause.", ConsoleColor.White); | ||
| } | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
SKILL.md requires the script to emit a structured [CI_ANALYSIS_SUMMARY] JSON block for agents to parse, but this C# port never prints that marker or JSON summary. Please add the summary emission (including key fields like mode/repository/builds/failed jobs/canceled jobs/known issues/pr correlation/recommendationHint) and ensure it’s emitted even in error/no-build cases (e.g., merge conflicts / no builds) so the skill remains machine-readable.
| async Task<List<KnownIssue>> GetBuildAnalysisKnownIssues(int pr) | ||
| { | ||
| var issues = new List<KnownIssue>(); | ||
| try | ||
| { | ||
| var (sha, _, exitCode) = RunProcess("gh", $"pr view {pr} --repo {options.Repository} --json headRefOid --jq .headRefOid"); | ||
| sha = sha.Trim(); | ||
| if (exitCode != 0 || !Regex.IsMatch(sha, @"^[a-fA-F0-9]{40}$")) return issues; | ||
|
|
||
| var (json, _, exitCode2) = RunProcess("gh", $"api repos/{options.Repository}/commits/{sha}/check-runs --jq \".check_runs[] | select(.name == \\\"Build Analysis\\\") | .output\""); |
There was a problem hiding this comment.
GetBuildAnalysisKnownIssues is marked async but has no await, which triggers CS1998 and can fail the script build under the repo’s warnings-as-errors settings. Make it non-async (or use proper async process execution) and update call sites accordingly.
| string? FormatTestFailure(string logContent, int maxLines = 50, int maxFailures = 3) | ||
| { | ||
| var lines = logContent.Split('\n'); | ||
| var allFailures = new List<string>(); | ||
| var currentFailure = new List<string>(); | ||
| var inFailure = false; | ||
| var emptyLineCount = 0; | ||
|
|
||
| var failurePattern = new Regex(@"\[FAIL\]|Assert\.\w+\(\)\s+Failure|Expected:.*but was:|BUG:|FAILED\s*$|END EXECUTION - FAILED|System\.\w+Exception:"); | ||
|
|
||
| foreach (var line in lines) |
There was a problem hiding this comment.
-MaxFailureLines is parsed/stored but never applied: FormatTestFailure always uses its default maxLines = 50, and call sites don’t pass options.MaxFailureLines. Either thread options.MaxFailureLines through to FormatTestFailure (and use it in Helix modes too) or remove the option to avoid a misleading CLI.
| -HelixJob <guid> Helix job ID (GUID) | ||
| -WorkItem <name> Helix work item name (requires -HelixJob) | ||
| -Repository <r> GitHub repo (default: dotnet/runtime) | ||
| -Organization <org> AzDO organization (default: dnceng-public) |
There was a problem hiding this comment.
The usage text lists -Organization but omits -Project, even though -project is accepted and Options.Project is used to form AzDO URLs. Please document -Project <projectGuid> in the help output so users can discover/override it when analyzing other AzDO projects.
| -Organization <org> AzDO organization (default: dnceng-public) | |
| -Organization <org> AzDO organization (default: dnceng-public) | |
| -Project <projectGuid> AzDO project GUID to analyze |
| @@ -0,0 +1,2 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
I think this is what's necessary to fully isolate a C# script from the surrounding environment: Directory.Build.props, Directory.Build.targets, global.json. Feels pretty expensive. I wonder if there's a way to make this simpler.
You probably don't need Directory.Build.targets, you can just set ImportDirectoryBuildTargets=false inside Directory.Build.props. But this doesn't feel file-based apps specific, but rather a general dotnet/msbuild feature. Also there is no new situation, what if this was just a normal console app project for a tool?
Also it depends on what you want to isolate. Maybe you need NuGet.config too? And .editorconfig, .globalconfig.
So far I've not wanted scripts to import these things by default -- just the opposite, actually.
I'm curious to know why exactly you need this? Don't you want the C# code in the file-based app to adhere to the same rules as the rest of the C# code in your repo (and hence using the same analyzers that are imported through global Directory.Build.props file)?
There was a problem hiding this comment.
That being said, a feature request for this is dotnet/sdk#52804.
Doing CI analysis in powershell adds a pwsh dependency. I'd like to do this on non-Windows systems where it's very unlikely that I have pwsh installed. A dotnet 10+ SDK is already a requirement for building the repo, so I'll always have that installed.