Adjust breaking change workflow based on issues seen#126607
Adjust breaking change workflow based on issues seen#126607ericstj wants to merge 1 commit intodotnet:mainfrom
Conversation
ericstj
commented
Apr 7, 2026
- Agent is sometimes ignoring the result of the version calculation script - make it more clear how to handle results.
- Comment was duplicating issue body. Remove this.
- Tag the PR's assignees to get attention.
1. Agent is sometimes ignoring the result of the version calculation script - make it more clear how to handle results. 2. Comment was duplicating issue body. Remove this. 3. Tag the PR's assignees to get attention.
There was a problem hiding this comment.
Pull request overview
Updates the breaking-change-doc skill workflow instructions and helper script to reduce version-detection ambiguity, avoid duplicating the full issue draft in PR comments, and notify PR assignees via @mentions.
Changes:
- Expanded Step 1 metadata collection to include PR assignees and clarified Step 2 version-script handling/logging.
- Updated the PR comment generation workflow to pass assignees for @-mentions and to keep PR comments brief (no inline issue body).
- Enhanced
Build-IssueComment.ps1to accept an-Assigneesparameter and emit URL-length diagnostics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/skills/breaking-change-doc/SKILL.md | Clarifies workflow steps (version detection output handling, assignee metadata, brief PR comment intent). |
| .github/skills/breaking-change-doc/Build-IssueComment.ps1 | Generates a shorter PR comment and supports notifying PR assignees via a new -Assignees parameter. |
| if ($Assignees.Trim()) { | ||
| $mentionLine = "`n`n/cc $($Assignees.Trim())" |
There was a problem hiding this comment.
$Assignees.Trim() will throw if the caller passes $null (PowerShell parameters can be explicitly set to null). Consider using [string]::IsNullOrWhiteSpace($Assignees) (and trimming once) so the script is robust when there are no PR assignees or the metadata value is missing.
| if ($Assignees.Trim()) { | |
| $mentionLine = "`n`n/cc $($Assignees.Trim())" | |
| $trimmedAssignees = "" | |
| if (-not [string]::IsNullOrWhiteSpace($Assignees)) { | |
| $trimmedAssignees = $Assignees.Trim() | |
| $mentionLine = "`n`n/cc $trimmedAssignees" |
|
|
||
| If the script returns an error, fall back to reading the PR's base branch and recent | ||
| tags manually to estimate the version. | ||
| Use `EstimatedVersion` as the version for the breaking change issue. |
There was a problem hiding this comment.
Step 2 has conflicting guidance: it says to fall back to manual tag detection when the JSON contains an Error field, but then unconditionally says to use EstimatedVersion anyway. That unconditional line can cause the workflow to ignore the error case; consider removing it or rewording it to apply only when Error is absent.
| Use `EstimatedVersion` as the version for the breaking change issue. | |
| Only use `EstimatedVersion` as the version for the breaking change issue when | |
| the JSON does **not** contain an `Error` field. |