Repository navigation
[mgmt] improve mgmt-review workflow - #39870
Closed
ZiWei Chen (kazrael2119) wants to merge 8 commits into
Closed
ZiWei Chen (kazrael2119) wants to merge 8 commits into
ZiWei Chen (kazrael2119) wants to merge 8 commits into
Conversation
ZiWei Chen (kazrael2119)
requested review from
a team,
Jialin Huang (JialinHuang803),
Mary Gao (MaryGao),
Renhe Li (lirenhe) and
Qiaoqiao Zhang (qiaozha)
as code owners
September 4, 2026 07:59
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
ZiWei Chen (kazrael2119)
requested
a balanced review from Copilot
and removed request for
Copilot
September 4, 2026 08:09
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The generated workflow is stale, the Copilot author match is incorrect, and the described Event Processor integration is missing.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Improves management SDK reviews by coordinating Copilot review completion and expanding API review guidance.
Changes:
- Adds optional Copilot review polling and cleanup.
- Expands versioning and breaking-change rules.
- Regenerates the executable workflow.
File summaries
| File | Description |
|---|---|
.github/workflows/mgmt-review.md |
Adds Copilot waiting and cleanup steps. |
.github/workflows/mgmt-review.lock.yml |
Updates generated workflow output. |
.github/prompts/mgmt-review-guidelines.md |
Adds management review rules. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 4
- Review effort level: Balanced
Comment on lines
+97
to
+99
| const hasCopilotComment = thread.comments.nodes.some( | ||
| (comment) => comment.author?.login === 'Copilot', | ||
| ); |
|
|
||
| For every new package version, whether stable or preview, evaluate missing `Breaking Changes` entries against the applicable CHANGELOG comparison baseline defined above, not against APIs introduced only after that baseline. When any public API symbol or member appears to have been removed, inspect the intervening `CHANGELOG.md` entries after the comparison baseline and before the new version. This applies to operation groups, interfaces, enums, models, methods, properties, and other public API elements. If one of those entries records that the same API was added after the baseline (for example, `Added operation group <name>`, `Added interface <name>`, or `Added enum <name>`), its removal is not a breaking change relative to that baseline. Do not report a missing `Breaking Changes` entry for that removal. This check only determines whether the new changelog entry is missing a breaking-change entry; it does not override any specific public API design rules below. | ||
|
|
||
| When the new version is a beta release for a new major version and its latest `CHANGELOG.md` entry contains breaking changes, find the most recent earlier beta release in the same major-version series whose entry also contains breaking changes. For example, compare `4.0.0-beta.3` with `4.0.0-beta.2` or `4.0.0` with `4.0.0-beta.2`, but do not compare beta releases from different major-version series. Compare the two `Breaking Changes` lists and leave an inline comment on the new version line that identifies entries added to or removed from the list. If the lists have no differences, or no earlier beta in the same major-version series contains breaking changes, do not leave this comparison comment. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The generated workflow is stale, and the new resolution, unassignment, and label-transition paths are incomplete.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/mgmt-review.md:113
- This step ends after resolving threads and never removes the
copilot-swe-agent[bot]assignee. Once the lock file is correctly regenerated, both completed waits and direct/manual runs will omit the unassign behavior required by the PR description.
.github/workflows/mgmt-review.md:99
- The GraphQL review-thread author login is
copilot-pull-request-reviewer(with an optional[bot]suffix), notCopilot. This exact comparison therefore leaveshasCopilotCommentfalse and prevents the workflow from resolving Copilot threads.
const hasCopilotComment = thread.comments.nodes.some(
(comment) => comment.author?.login === 'Copilot',
);
.github/prompts/mgmt-review-guidelines.md:59
- The opening condition limits this comparison to beta releases, but the same sentence's example requires comparing a stable
4.0.0with an earlier beta. An agent can follow the condition and skip the stable case; explicitly include stable releases in the condition.
When the new version is a beta release for a new major version and its latest `CHANGELOG.md` entry contains breaking changes, find the most recent earlier beta release in the same major-version series whose entry also contains breaking changes. For example, compare `4.0.0-beta.3` with `4.0.0-beta.2` or `4.0.0` with `4.0.0-beta.2`, but do not compare beta releases from different major-version series. Compare the two `Breaking Changes` lists and leave an inline comment on the new version line that identifies entries added to or removed from the list. If the lists have no differences, or no earlier beta in the same major-version series contains breaking changes, do not leave this comparison comment.
- Files reviewed: 2/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Concurrency cancellation, incorrect Copilot identity matching, missing unassignment, and contradictory guidance remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
.github/workflows/mgmt-review.md:191
- GraphQL exposes Copilot review comments under
copilot-pull-request-reviewer(with a possible[bot]suffix on other API surfaces), notCopilot. This predicate therefore stays false and leaves all Copilot threads unresolved. Normalize the suffix and compare against the actual login.
(comment) => comment.author?.login === 'Copilot',
.github/prompts/mgmt-review-guidelines.md:59
- The opening condition limits this comparison to beta targets, but the same rule requires comparing a stable
4.0.0target with an earlier beta. An agent can follow the condition and skip that stable case; explicitly include the first stable release in a new major series.
When the new version is a beta release for a new major version and its latest `CHANGELOG.md` entry contains breaking changes, find the most recent earlier beta release in the same major-version series whose entry also contains breaking changes. For example, compare `4.0.0-beta.3` with `4.0.0-beta.2` or `4.0.0` with `4.0.0-beta.2`, but do not compare beta releases from different major-version series. Compare the two `Breaking Changes` lists and leave an inline comment on the new version line that identifies entries added to or removed from the list. If the lists have no differences, or no earlier beta in the same major-version series contains breaking changes, do not leave this comparison comment.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
| if: needs.pre_activation.outputs.ready == 'true' && needs.pre_activation.outputs.pr_number != '' | ||
| concurrency: | ||
| group: "gh-aw-${{ github.workflow }}-${{ github.event.pull_request.number || github.event.inputs.item_number || github.run_id }}-${{ github.event.label.name || '' }}" | ||
| group: "gh-aw-${{ github.workflow }}-${{ github.event.check_suite.pull_requests[0].number || github.event.pull_request.number || github.event.inputs.item_number || github.run_id }}" |
Comment on lines
+205
to
+206
| cursor = threads.pageInfo.hasNextPage ? threads.pageInfo.endCursor : null; | ||
| } while (cursor); |
Member
Author
|
close as we will split this pr's changes to several new prs |
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.
improve mgmt-review workflow：
improve check logic
mgmt-neededlabel(still add in-progress and added label),Management Review Workflow
checks: readandissues: writepermissions.wait_for_comilot_reviewboolean input.copilot pull request viewercheck:Copilotcomments.Management Review Guidelines
version X.Y.Z.Promise<T>orPromise<T | undefined>becomesPromise<T | void>is not reported.deleting client operation group parameters.Promise<T>becomesPollerLike<OperationState<void>, void>, report .import type {}toimport {}