Repository navigation
Updated strategy for writing status checks safely. - #1845
Merged
Merged
Conversation
- Replace the Octokit annotation collection with tool-neutral diagnostic records containing severity, repository-relative file path, start/end lines, diagnostic ID, and message. - Preserve `Success` tracking and existing human-readable console output. - When `GITHUB_ACTIONS` is `true`, additionally emit workflow commands: | Logger method | Workflow command | | --- | --- | | `LogFailure` | `::error` | | `LogWarning` | `::warning` | | `LogNotice` | `::notice` | | `ConsoleOnlyLog` | None | - Route command generation through one shared formatting helper. - Normalize source paths to repository-relative paths with `/` separators. - Escape `%`, carriage returns, and newlines as `%25`, `%0D`, and `%0A`, respectively. Escape `%` first so newly introduced escapes are not escaped again. - Additionally escape `:` and `,` as `%3A` and `%2C` in property values, including file paths and titles. - Emit each command on a single output line. - Omit line properties when no valid source line is available. Include `endLine` only when it is at least `line`. - Preserve raw values in diagnostic records and JSON; apply escaping only when rendering workflow commands. - Preserve all diagnostics in the report, regardless of GitHub's annotation display limits. Example command: ```text ::error file=standard/expressions.md,line=42,endLine=44,title=Example tester - EX001::Expected output does not match.
Replace `BuildCheckRunResult` and `CreateCheckRun` with a method such as
`WriteReportAsync(outputPath)`. Create the parent directory and serialize the tool
name, success state, and complete diagnostic collection.
Example report:
```json
{
"tool": "Example tester",
"success": false,
"diagnostics": [
{
"severity": "error",
"file": "standard/expressions.md",
"startLine": 42,
"endLine": 44,
"id": "EX001",
"message": "Expected output does not match."
}
]
}
```
Update publishing callers in:
- `tools/ExampleTester/Program.cs`
- `tools/MarkdownConverter/Program.cs`
- `tools/MarkdownConverter/Spec/Reporter.cs`
- `tools/StandardAnchorTags/Program.cs`
Use an optional `DIAGNOSTICS_REPORT_PATH` environment variable to select the report
destination instead of using `GH_TOKEN` and `HEAD_SHA`. Local execution without
the variable should retain console reporting without requiring authentication.
Write accumulated diagnostics in `finally` blocks after logger initialization,
including the converter's early failure paths. Failures before the tool starts,
such as compilation failures, may produce no report and remain visible in the
workflow log. Do not suppress report-writing errors.
Preserve existing exit-code behavior: an `::error` annotation does not itself fail
a step, so tools must still return a nonzero exit code on failure.
Remove obsolete check-run-specific code, tests, and dependencies where no longer
used. Check other callers before removing public APIs, CLI options, or packages.
Use the updated reporting mechanism instead of the check run.
This reverts commit 1557a76.
Replace check-run payload tests in `tools/Utilities.Tests/StatusCheckLoggerTests.cs` with report and workflow-command coverage. Include: - Severity mapping to `error`, `warning`, and `notice`. - Escaping of percent signs, carriage returns, newlines, colons, and commas in the appropriate command components. - Multiline messages rendered as a single command output line. - Repository-relative paths with normalized separators. - Missing or invalid source line numbers and invalid end-line ranges. - No workflow commands outside GitHub Actions. - Console-only diagnostics remaining console-only. - Complete report retention with more than 50 diagnostics. - Parent-directory creation and valid JSON serialization with raw messages. - Success/failure tracking and preservation of failed tool exit codes. - Report generation after a later exception or early failure once the logger has been initialized. Run the smallest relevant existing test suites and builds. Verify the workflow behavior on a fork PR: a failed tool still fails its job, available reports upload, and runner-generated annotations appear without repository write permissions.
Add notes on where the updated logs are stored when running locally.
BillWagner
marked this pull request as ready for review
October 21, 2026 20:13
jskeet
approved these changes
Oct 23, 2026
jskeet
left a comment
jskeet
left a comment
Contributor
There was a problem hiding this comment.
This review was conducted while the first coffee of the morning was still getting through to my brain, so it's still a little fuzzy, but it looks good to me... and I definitely like the way we're able to operate the GitHub actions with significantly reduced permissions now.
| { | ||
| var ownerOption = new Option<string>("--owner") { Description = "The GitHub owner org (for example, \"dotnet\")" }; | ||
| var repoOption = new Option<string>("--repo") { Description = "The GitHub repo name (for example, \"csharpstandard\")" }; | ||
| var ownerOption = new Option<string>("--owner") { Description = "Legacy option; no longer used" }; |
Contributor
There was a problem hiding this comment.
Are these removed in a later commit? If not, is it worth filing a tidy-up issue so we remove them when it's safe to do so?
Member
Author
There was a problem hiding this comment.
I did leave them for now, just in case I missed a shell script that uses them. See #1846
| step and warn if the tool could not produce a report; they do not change a | ||
| failed job's result. | ||
|
|
||
| Runner-generated annotations appear on the Actions job check. GitHub may also |
Contributor
There was a problem hiding this comment.
Not really sure what this second sentence means, but it's not a big issue.
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.
Replace custom GitHub check-run publishing with downloadable JSON diagnostics and runner-generated annotations. Keep the
pull_requesttrigger and existing human-readable console logs. Do not usepull_request_target, repository write permissions, or GitHub API calls to publish results.The observable changes once this PR is merged are that PRs from forks (as well as feature branches) will have annotations displayed correctly for all our custom tools.