Improve breaking-change-doc version detection and issue link#130349
Conversation
Rework the breaking-change-doc skill to address dotnet#130184: Get-VersionInfo.ps1: determine the version from release-branch existence (with cadence-aware Preview->RC->GA rollover) plus own-commit and release-tag containment, instead of the unreliable tag+1 heuristic. Detect potential backports via the primary PR's cross-reference graph and report an earlier milestone as tentative (with a FallbackVersion) when an unverified linked release-branch PR is found. Build-IssueComment.ps1: build a hybrid pre-filled issue link that keeps the body in the URL only when it fits (~8000-byte budget) and otherwise falls back to a copy-paste code block, avoiding the 414 error. Source labels/assignee from the docs issue template and drop the redaction-prone mailto link. SKILL.md: document the above and remove duplicated/stale content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the breaking-change-doc skill’s helper scripts and documentation to (1) determine the correct .NET release milestone using release-branch/tag signals rather than tag proximity heuristics and (2) generate a GitHub issue-creation link that avoids HTTP 414 failures by conditionally omitting the URL-encoded body.
Changes:
- Reworked version detection (
Get-VersionInfo.ps1) to use release branches + compare containment, with tag-based refinement for older/pruned preview/RC branches and tentative detection via cross-referenced backport PRs. - Updated PR-comment / issue-link generation (
Build-IssueComment.ps1) to use a hybrid prefill strategy with a conservative URL-length budget and template-sourced labels/assignees. - Updated skill documentation (
SKILL.md) to reflect the new behavior and parameters.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/skills/breaking-change-doc/SKILL.md |
Updates the skill instructions to match the new version detection and issue-link behavior/parameters. |
.github/skills/breaking-change-doc/Get-VersionInfo.ps1 |
Replaces tag-based “next version” inference with branch/tag/containment-based milestone detection + tentative backport signals. |
.github/skills/breaking-change-doc/Build-IssueComment.ps1 |
Generates an issue-creation link that avoids 414 by dropping the body when needed and embedding it in the PR comment for copy/paste. |
The tag-refinement path probed the compare API once per candidate tag, linearly, until it found containment -- up to ~27 calls for dotnet/runtime and rate-limit-prone in CI. Since containment is monotonic along release order (each tag on a major.minor line is a superset of the previous), replace the linear scan with a binary search over the version-ordered candidates, bounding it to ~log2(n) compare calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ConvertFrom-ReleaseBranch: return $null for unrecognized release/* labels (e.g. release/11.0-rtm) instead of a bogus Rank=0 milestone that would sort first in the containment scan and mis-detect the version. Get-BackportPRs: check $LASTEXITCODE on the cross-reference timeline call and, on failure, warn and return no candidates instead of failing silently. Backport detection only enriches the result, so a timeline hiccup shouldn't abort version detection, but the warning surfaces the gap in the log. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Get-EarliestContainingTag paginated every tag in the repo (~195 for dotnet/runtime) and filtered in memory. Query only the relevant tags server-side via git/matching-refs/tags/v<major>.<minor>. and strip the refs/tags/ prefix, cutting the payload to the ~27 tags for that release line. The endpoint returns 200 with an empty array (not 404) when nothing matches, so the existing exit-code guard still holds; the candidate set and binary search are otherwise unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Get-NextMilestoneVersion returned the GA version itself when the highest branched milestone was GA, so a change on main that is not contained in a fully-shipped release line was reported as that same GA version instead of the next major's first preview. This surfaced as a wrong FallbackVersion (e.g. .NET 10 instead of .NET 11 Preview 1 for a change merged during the release/N fork-to-rename window). Roll the GA case over to (major+1).0 Preview 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ericstj
left a comment
There was a problem hiding this comment.
Local review focused on script correctness and unhandled edge cases. The overall design is sound — compare-API containment (�ehind_by == 0), the binary search over persistent release tags, and the cadence-aware rollover all check out, and I confirmed the headline fix against the live repo (main=11.0, highest branch release/11.0-preview6, release/10.0 GA with preview branches pruned). A few edge cases below; the highest-value ones are the Backports JSON shape and the large-body fallback still failing.
Note
This review was generated with AI assistance from Copilot (GitHub Copilot CLI).
A bare release/N.M branch is cut around the RC1 fork -- months before GA (verified: release/10.0 existed from 2025-08-16, ~3 months before GA) -- yet is parsed as a GA milestone by name, so a PR merged directly into it (Step 2) or a backport targeting it (Step 5b) was reported as the coarse ".NET N" instead of an RC stage. Add Resolve-ReleaseBranchStage: a separate release/N.M-rc<K> branch exists for each RC that has been taken (these stay active and can receive fixes), while the bare branch keeps stabilizing toward the next stage. When the product has not GA'd (no vN.M.0 tag), map the highest existing rc branch number K to the bare branch's stage -- no rc branch -> RC 1, rc<K> -> RC (K+1), final RC branch exists -> GA -- and apply it in both the direct-merge and backport paths. The main-PR next-milestone prediction still treats the bare branch as GA so it can roll over to the next major. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The hybrid link strategy embeds the issue body in the comment when it is too long for the URL, but this was warn-only: an extreme body could push the comment past GitHub's 65536-char limit and the later post would fail. Refactor the comment assembly into a reusable template and, when the full body overflows the comment budget, truncate the embedded copy (with a marker and an [!IMPORTANT] note pointing at the complete issue-draft.md artifact) so the post still succeeds. Add a -MaxCommentLength parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The embedded copy-paste block used a fixed four-backtick fence, which assumed bodies only ever contain three-backtick code fences; a body with a >=4-backtick fence would break the <details> block. Compute the fence as one backtick longer than the longest backtick run in the body (minimum 3) so a code fence of any length can't prematurely close it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jeffhandley
left a comment
There was a problem hiding this comment.
Thanks for tackling this, @svick!
Note that the detection job failures due to inability to reach the PAT pool will be addressed in gh-aw v0.82.6.
|
@jeffhandley Thanks for the review. |
Addresses the problems described in #130184 in the
breaking-change-docskill.Version detection (
Get-VersionInfo.ps1)Replaces the unreliable
last tag + 1heuristic with release-branch–based detection:release/<major>.<minor>-preview<N>(and-rc<N>/ GA) branches, with cadence-aware rollover (7 previews, then 2 RCs, then GA), so a change onmaincorrectly rolls Preview 7 → RC 1 and RC 2 → GA.FallbackVersion— when an unverified linked release-branch PR is found.This fixes the observed case where a
mainchange was mislabeled Preview 6 instead of Preview 7.Issue creation link (
Build-IssueComment.ps1)labelsandassigneesfrom the dotnet/docs issue template instead of hardcodingPri1/doc-idea(which don't exist — see Remove outdated labels from breaking change issue template docs#54637).mailto:link with a plain-text notification-alias reminder, fixing the broken email formatting.Skill doc (
SKILL.md)Updated to match the above, plus removed duplicated/stale content.
Not addressed here
The
400 bad request: Authorization header is badly formattedsecurity-scan failure is a PAT-pool secret configuration issue, not a skill-script problem, so it's out of scope for this PR./cc @ericstj @jeffhandley
Note
This PR description and the code changes were generated with AI assistance from Copilot (GitHub Copilot CLI).