From 323cca52a0aaddd90edb3623ae9e9ae91c3dba3f Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 23 Oct 2026 15:11:31 +0800 Subject: [PATCH 01/11] docs: add Python management SDK release rotation runbook Fixes #49651 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f63e6e2b-238f-4cd4-952e-43d460067877 --- doc/dev/mgmt/README.md | 1 + doc/dev/mgmt/mgmt-release-rotation-runbook.md | 174 ++++++++++++++++++ doc/dev/mgmt/mgmt_release.md | 5 + 3 files changed, 180 insertions(+) create mode 100644 doc/dev/mgmt/mgmt-release-rotation-runbook.md diff --git a/doc/dev/mgmt/README.md b/doc/dev/mgmt/README.md index ac4df62f6439..b316352ca3a4 100644 --- a/doc/dev/mgmt/README.md +++ b/doc/dev/mgmt/README.md @@ -3,6 +3,7 @@ The documentation in that folder is intended for developers at Microsoft, or for advanced contributors, in the ARM (Azure Resource Management) world. In order of workflow: +- [mgmt-release-rotation-runbook.md](mgmt-release-rotation-runbook.md) : Weekly DRI guide for management SDK PR review, approval, merge, and release follow-up - [generation.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/generation.md) : How to generate the SDK (TypeSpec is the recommended approach; legacy Swagger/AutoRest instructions are also included) - [tests.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/tests.md) : How to test management SDK (recordings, etc.) - [mgmt_release.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/mgmt_release.md) : How to finish preparing the package, changelog, version, etc. diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md new file mode 100644 index 000000000000..2dde9d0020b6 --- /dev/null +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -0,0 +1,174 @@ +# Python management SDK release rotation runbook + +Use this runbook during the weekly Go/JavaScript/Python directly responsible +individual (DRI) rotation. It covers generated `azure-mgmt-*` release PRs, from +triage through release follow-up. Route handwritten SDK or generator changes to +the Python owner. + +**Contacts:** the **service team** is the submitter/contact in the PR or linked +release plan. The **Python owner** is a management SDK maintainer listed under +`/**/*mgmt*/` in [CODEOWNERS](../../../.github/CODEOWNERS) +(currently @ChenxiJiang333, @msyyc, and @Azure/azure-arm-sdk-reviewers). +Ask the Python owner to coordinate generator, engineering-system, or vendor support. +Rotation assignment does not grant repository, CODEOWNER, or release permissions. + +## 1. Find PRs needing attention + +Use these live GitHub queries; open each PR for its current status. + +| Queue | Purpose | +| --- | --- | +| [All open management AutoPRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle) | Includes drafts and PRs without release/review labels. Start here. | +| [Review queue](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+label%3Amgmt-review-needed+draft%3Afalse) | Review requested; not proof that checks or human review are complete. | +| [Approved release candidates](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+draft%3Afalse+review%3Aapproved) | Check for merge blockers and hand off to the service team. Do not add duplicate approval. | +| [Held release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3AHoldOn) | Wait for the planned window or the owner's decision. | +| [Recently merged release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Amerged+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+sort%3Aupdated-desc) | Follow up on publishing for this rotation's packages. Merged does not mean released. | + +`mgmt-review-needed` requests automated review when applied. +`auto-release` opts changed packages into post-merge release. +Neither label is approval; either can remain on a draft or an already reviewed PR. +An updated timestamp can reflect bot activity, not a recent service-team response. + +| Current state | Next action | +| --- | --- | +| Draft | Ask the service team to complete release prerequisites and mark the PR ready when intended. Do not mark it ready on their behalf. | +| `HoldOn`, `blocking-release`, or an explicit human hold | Keep the hold. Ask the person who owns it what must change before proceeding. | +| CI queued/running | Wait for the current revision's checks. Ask the Python owner about stuck or missing pipelines. | +| CI failed | Read the existing failure analysis and route the stated fix to the service team or Python owner. Do not restart review instead of fixing CI. | +| Ready, no valid approval | Continue with sections 2 and 3. | +| Approved, not merged | Identify the actual merge blocker or ask the service team to merge through its release plan. | + +## 2. Check readiness and the automated review + +Read the PR description, release plan, latest human decisions, and current checks. +Confirm the service team intends to release the listed package, SDK version, and +API version. Follow the linked +[SDK release prerequisites](https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites). +A bot-created PR is not itself a release request. + +Find the latest **Management SDK PR review** comment from the +[Python review workflow](../../../.github/workflows/mgmt-sdk-pr-review.md). +Check that its pinned evidence covers the current changes. An older complete +comment does not cover a later push. + +| Review result | Reviewer action | +| --- | --- | +| Complete, no findings or unverified checks | Use the completed checks and evidence; do not reconstruct them. Continue with human review below. | +| Blocking findings | Ask the service team to make the listed source/configuration fixes and regenerate. Route emitter/generator issues to the Python owner. | +| Warning or suggestion | Read the finding and record its fix or justified disposition. Ask the Python owner when Python-specific judgment is needed. | +| Unverified checks, incomplete review, partial breaking-change attribution, or missing evidence | Ask the Python owner to complete or resolve the missing review. Missing evidence is not a pass. | +| Missing or stale result | If authorized, request a fresh analysis by applying `mgmt-review-needed`. If it is already present, remove and reapply it only when no review is active. Otherwise ask the Python owner. | + +The workflow runs on the label event, not every subsequent push. Manual workflow +dispatch is disabled by default; it is not the rotation's fallback. + +The workflow checks Python package versions, preview/stable metadata, client +consistency, README usage, and API-version drift. Its +[management review rules](../../../.github/instructions/reviewer/management.instructions.md) +define the scope. Do not repeat completed checks or review every generated model, +operation, sample, and test. + +**Human review still covers:** + +- Breaking changes and their disposition. **TypeSpec/API** attribution explains + origin; it does not approve the break. Ask the service team to confirm intended + service changes. Send generator/migration changes, unclear attribution, and + exceptions to the Python owner. A major version bump alone is not permission. + Use the [breaking-change guide](sdk-breaking-changes-guide.md) for known patterns. +- Outstanding Copilot and human review comments. Ask for real defects to be fixed + upstream; record a reason when a suggestion does not apply. Do not require + output-only edits to satisfy a generated-code suggestion. +- First-release onboarding not covered by the result: confirm required namespace + approval, ownership, and a release pipeline with the Python owner. For a + confirmed first release, a client-name finding requires `client.tsp` + customization and regeneration, not a hand edit to `_client.py`. + +## 3. Approve or re-approve + +Approve only if you are an authorized reviewer, the service team has confirmed +readiness, current required CI passes, review findings have justified +dispositions, and no unresolved review, evidence gap, or hold remains. +The automated comment is advisory; it never grants human approval. + +After a push or regeneration, inspect what changed since the previous approval. +Check the current GitHub review decision and branch requirements, not just an old +**Approved** event in the conversation. A previous approval can remain visible +while GitHub still requires review. + +Recheck changed versions, changelog, API versions, customizations, and any affected +findings. Request fresh automated analysis when its evidence is stale, and wait +for current CI. Re-approve only after reviewing the new changes; do not blindly +repeat approval. If the current approval is still valid, do not add another one. +Route API-version changes after approval or unclear generated changes to the +Python owner. + +## 4. Hand off merge and follow the release + +Tell the service team when review is complete and point to its release plan's +next action. The service team normally merges; an authorized DRI may help only +when the service team requests it and all branch requirements are satisfied. +Never bypass required reviews, checks, or holds. + +**Treat merge of an `auto-release` PR as a publishing action.** Confirm release +intent and timing before merge. The +[release template](../../../eng/pipelines/templates/stages/archetype-python-release.yml) +uses internal post-merge CI on `main` to select changed, declared package artifacts. +This automatic path does **not** wait for the manual `package-publish` approval +gate. Do not merge early assuming another approval will stop publication. + +After merge, locate `python - ` in the +[internal Python pipelines](https://dev.azure.com/azure-sdk/internal/_build?view=folders) +and find the run for the merged commit. Check `AutoReleasePrepare` and the target +package's `Release: ` stage. Confirm the exact version is available on +[PyPI](https://pypi.org/) and check the linked release plan's status. +Release-plan completion can fail separately from publication; do not republish +an existing version to repair tracking. + +If publishing is absent, skipped, failed, or unclear, send the PR, package/version, +merged commit, pipeline link, and failed/skipped stage to the Python owner. +A missing `auto-release` label or artifact mapping can leave no automatic release. +Adding the label after the run is not a reliable recovery. Check for an active run +or an already published version before any retry. + +For an intentional manual release, let the Python owner coordinate the +[unified release process](../release.md#releasing-through-unified-pipelines). +Only authorized release approvers validate and approve the intended package's +artifacts. Do not queue a second release by default, approve unrelated packages, +or upload manually with `twine`. + +## 5. Common partner questions and escalation + +| Question or symptom | Response / owner | +| --- | --- | +| "CI is green. Can you approve?" | Check readiness, the current automated result, breaking changes, and unresolved comments. Green CI alone is insufficient. | +| "You approved already. Can you approve again?" | Compare the new changes and current review requirements using section 3. Do not add duplicate approval when none is needed. | +| "Can you mark this draft ready or remove the hold?" | Ask the service team or hold owner to confirm readiness and timing. An approved draft is still a draft. | +| "Why did TypeSpec migration introduce breaking changes?" | Point to the attribution and breaking-change guide. Ask the Python owner to resolve unintended generator/migration breaks; do not accept them merely because code is generated. | +| "The first package has no pipeline / needs a different client name." | Ask the Python owner to coordinate onboarding or pipeline preparation. The service team fixes naming in `client.tsp` and regenerates. | +| "Playback tests fail after the API version changed." | Route recording updates to the service team with the existing failure analysis and [management test guide](tests.md). Do not hand-edit recordings or disable tests to obtain green CI. | +| "The PR merged. Where is the package?" | Inspect the post-merge release run and PyPI using section 4. Escalate the exact failed stage, not a request for another PR approval. | + +Historical examples inspected on 2026-10-23 illustrate these distinctions: +[an approved draft](https://github.com/Azure/azure-sdk-for-python/pull/48930), +[an older approval with review still required](https://github.com/Azure/azure-sdk-for-python/pull/49633), +[a corrected first-release naming blocker](https://github.com/Azure/azure-sdk-for-python/pull/49457), +and [approval followed by a service-team merge/release handoff](https://github.com/Azure/azure-sdk-for-python/pull/48686). +These examples are not the live queue. + +## 6. US-based coverage and rotation handoff + +US-based colleagues can triage the queue, relay existing findings, request +service-team confirmation, and identify the next owner while Python maintainers +are offline. Authorized rotation reviewers can approve ordinary cases after the +checks above. Geography does not grant or restrict approval authority. + +Do not give courtesy approvals, grant breaking-change exceptions, clear holds, +or merge early to bridge a time-zone gap. Do not regenerate packages, patch +generated output, rerun every check, or repeat completed automated analysis merely +to show progress. Use the Python owner for unresolved Python-specific decisions; +keep the blocker visible rather than substituting an uninformed approval. + +Leave one clear next action in the existing PR thread. At handoff, give the next +DRI the PR link, package/version, current blocker, responsible owner, required +action, and release deadline or planned window. Include the release run if merged. +Never hand off "approved" as a substitute for verified release completion. diff --git a/doc/dev/mgmt/mgmt_release.md b/doc/dev/mgmt/mgmt_release.md index 6a3f074fac22..7f934a94b602 100644 --- a/doc/dev/mgmt/mgmt_release.md +++ b/doc/dev/mgmt/mgmt_release.md @@ -1,5 +1,10 @@ # Management Package Release +For weekly cross-language DRI coverage, use the +[management SDK release rotation runbook](mgmt-release-rotation-runbook.md). +This page covers package preparation; the runbook covers PR triage through +release follow-up, including the post-merge automatic release path. + ## General Release Process Overview Once Swagger PR is merged and a release plan/request is submitted, here are steps that you need to do in order to release a new package: From 9449440b41121417ca2045b972a770b2112f8c37 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 23 Oct 2026 15:32:22 +0800 Subject: [PATCH 02/11] docs: make release runbook standalone and clarify bot review requests Address review feedback: write for reviewers unfamiliar with Python releases, explain the maintainer-applied review label and bot follow-up, and remove changes to existing documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f63e6e2b-238f-4cd4-952e-43d460067877 --- doc/dev/mgmt/README.md | 1 - doc/dev/mgmt/mgmt-release-rotation-runbook.md | 69 ++++++++++++------- doc/dev/mgmt/mgmt_release.md | 5 -- 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/doc/dev/mgmt/README.md b/doc/dev/mgmt/README.md index b316352ca3a4..ac4df62f6439 100644 --- a/doc/dev/mgmt/README.md +++ b/doc/dev/mgmt/README.md @@ -3,7 +3,6 @@ The documentation in that folder is intended for developers at Microsoft, or for advanced contributors, in the ARM (Azure Resource Management) world. In order of workflow: -- [mgmt-release-rotation-runbook.md](mgmt-release-rotation-runbook.md) : Weekly DRI guide for management SDK PR review, approval, merge, and release follow-up - [generation.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/generation.md) : How to generate the SDK (TypeSpec is the recommended approach; legacy Swagger/AutoRest instructions are also included) - [tests.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/tests.md) : How to test management SDK (recordings, etc.) - [mgmt_release.md](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/mgmt_release.md) : How to finish preparing the package, changelog, version, etc. diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index 2dde9d0020b6..eb7ac48a7775 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -1,15 +1,23 @@ # Python management SDK release rotation runbook -Use this runbook during the weekly Go/JavaScript/Python directly responsible -individual (DRI) rotation. It covers generated `azure-mgmt-*` release PRs, from -triage through release follow-up. Route handwritten SDK or generator changes to -the Python owner. +This runbook is for Go/JavaScript/Python rotation reviewers who are unfamiliar +with Python SDK releases. It explains how to find a release PR, use the bot's +review, decide the next action, and follow the release. You do not need a local +Python environment or detailed knowledge of generated Python code for routine cases. +The reviewer on duty is the directly responsible individual (**DRI**). Ask the +Python owner to review handwritten SDK or code-generation tool changes. + +Python management SDKs manage Azure resources and use package names such as +`azure-mgmt-network`. An **AutoPR** contains automatically generated SDK changes. +**CI** is the set of automated PR checks. A **release plan**, linked from the PR, +tracks the service team's release request. Released Python packages are published +to the Python Package Index (**PyPI**). **Contacts:** the **service team** is the submitter/contact in the PR or linked release plan. The **Python owner** is a management SDK maintainer listed under `/**/*mgmt*/` in [CODEOWNERS](../../../.github/CODEOWNERS) (currently @ChenxiJiang333, @msyyc, and @Azure/azure-arm-sdk-reviewers). -Ask the Python owner to coordinate generator, engineering-system, or vendor support. +Ask the Python owner to coordinate code-generation, pipeline, or vendor support. Rotation assignment does not grant repository, CODEOWNER, or release permissions. ## 1. Find PRs needing attention @@ -19,12 +27,13 @@ Use these live GitHub queries; open each PR for its current status. | Queue | Purpose | | --- | --- | | [All open management AutoPRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle) | Includes drafts and PRs without release/review labels. Start here. | -| [Review queue](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+label%3Amgmt-review-needed+draft%3Afalse) | Review requested; not proof that checks or human review are complete. | +| [PRs with automated review requested](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+label%3Amgmt-review-needed+draft%3Afalse) | Only PRs where a maintainer has added the label. This is not the full queue or proof of readiness. | | [Approved release candidates](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+draft%3Afalse+review%3Aapproved) | Check for merge blockers and hand off to the service team. Do not add duplicate approval. | | [Held release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3AHoldOn) | Wait for the planned window or the owner's decision. | | [Recently merged release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Amerged+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+sort%3Aupdated-desc) | Follow up on publishing for this rotation's packages. Merged does not mean released. | -`mgmt-review-needed` requests automated review when applied. +An SDK maintainer manually adds `mgmt-review-needed` to request automated review. +Its absence does not mean a PR needs no attention; start from all open AutoPRs. `auto-release` opts changed packages into post-merge release. Neither label is approval; either can remain on a draft or an already reviewed PR. An updated timestamp can reflect bot activity, not a recent service-team response. @@ -35,10 +44,10 @@ An updated timestamp can reflect bot activity, not a recent service-team respons | `HoldOn`, `blocking-release`, or an explicit human hold | Keep the hold. Ask the person who owns it what must change before proceeding. | | CI queued/running | Wait for the current revision's checks. Ask the Python owner about stuck or missing pipelines. | | CI failed | Read the existing failure analysis and route the stated fix to the service team or Python owner. Do not restart review instead of fixing CI. | -| Ready, no valid approval | Continue with sections 2 and 3. | +| Ready, no valid approval | Read the bot review using section 2. If no review comment exists, ask an SDK maintainer to add `mgmt-review-needed`. | | Approved, not merged | Identify the actual merge blocker or ask the service team to merge through its release plan. | -## 2. Check readiness and the automated review +## 2. Request and read the bot review Read the PR description, release plan, latest human decisions, and current checks. Confirm the service team intends to release the listed package, SDK version, and @@ -46,33 +55,45 @@ API version. Follow the linked [SDK release prerequisites](https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites). A bot-created PR is not itself a release request. -Find the latest **Management SDK PR review** comment from the -[Python review workflow](../../../.github/workflows/mgmt-sdk-pr-review.md). -Check that its pinned evidence covers the current changes. An older complete -comment does not cover a later push. +For a ready PR, use this process rather than reviewing all generated code yourself: + +1. Look in the PR conversation for the latest **Management SDK PR review** bot comment. +2. If there is no bot review comment, ask an **SDK maintainer** to manually add + `mgmt-review-needed` under **Labels** in the PR sidebar. The label triggers the + [Python review workflow](../../../.github/workflows/mgmt-sdk-pr-review.md); + creating the AutoPR alone does not request this review. +3. Wait for the workflow to finish and the bot to leave its review comment. + The label itself is not a result. If no comment appears after the run finishes, + ask the Python owner to investigate. +4. Read the comment's findings, unverified checks, and suggested actions. + Blocking items explain what prevents approval and what needs to happen next. + Use the table below to identify who should act. +5. Check that the comment covers the latest PR changes. If commits were added + after the review, ask the SDK maintainer for an updated review. | Review result | Reviewer action | | --- | --- | | Complete, no findings or unverified checks | Use the completed checks and evidence; do not reconstruct them. Continue with human review below. | -| Blocking findings | Ask the service team to make the listed source/configuration fixes and regenerate. Route emitter/generator issues to the Python owner. | +| Blocking findings | Do not approve. Follow the comment's explanation and next action. Ask the service team to make the listed source/configuration fixes and regenerate; send code-generation tool issues to the Python owner. | | Warning or suggestion | Read the finding and record its fix or justified disposition. Ask the Python owner when Python-specific judgment is needed. | | Unverified checks, incomplete review, partial breaking-change attribution, or missing evidence | Ask the Python owner to complete or resolve the missing review. Missing evidence is not a pass. | -| Missing or stale result | If authorized, request a fresh analysis by applying `mgmt-review-needed`. If it is already present, remove and reapply it only when no review is active. Otherwise ask the Python owner. | +| Missing or outdated result | Ask the SDK maintainer to add `mgmt-review-needed`. If it is already present, the maintainer can remove and reapply it when no review is active. Wait for the new comment before continuing. | -The workflow runs on the label event, not every subsequent push. Manual workflow -dispatch is disabled by default; it is not the rotation's fallback. +Adding the label triggers the workflow; leaving it on the PR does not request +another review after each push. Do not request another run while one is active. -The workflow checks Python package versions, preview/stable metadata, client -consistency, README usage, and API-version drift. Its +The bot checks package versions, preview/stable settings, client naming and usage, +and API-version changes. Its [management review rules](../../../.github/instructions/reviewer/management.instructions.md) define the scope. Do not repeat completed checks or review every generated model, operation, sample, and test. **Human review still covers:** -- Breaking changes and their disposition. **TypeSpec/API** attribution explains - origin; it does not approve the break. Ask the service team to confirm intended - service changes. Send generator/migration changes, unclear attribution, and +- Breaking changes and their disposition. The comment's **TypeSpec/API** category + means the change comes from the service specification; it does not approve the + break. Ask the service team to confirm intended service changes. Send + code-generation/migration changes, unclear attribution, and exceptions to the Python owner. A major version bump alone is not permission. Use the [breaking-change guide](sdk-breaking-changes-guide.md) for known patterns. - Outstanding Copilot and human review comments. Ask for real defects to be fixed @@ -96,8 +117,8 @@ Check the current GitHub review decision and branch requirements, not just an ol while GitHub still requires review. Recheck changed versions, changelog, API versions, customizations, and any affected -findings. Request fresh automated analysis when its evidence is stale, and wait -for current CI. Re-approve only after reviewing the new changes; do not blindly +findings. Ask the SDK maintainer for a new bot review if the previous review is +outdated, and wait for current CI. Re-approve only after reviewing the new changes; do not blindly repeat approval. If the current approval is still valid, do not add another one. Route API-version changes after approval or unclear generated changes to the Python owner. diff --git a/doc/dev/mgmt/mgmt_release.md b/doc/dev/mgmt/mgmt_release.md index 7f934a94b602..6a3f074fac22 100644 --- a/doc/dev/mgmt/mgmt_release.md +++ b/doc/dev/mgmt/mgmt_release.md @@ -1,10 +1,5 @@ # Management Package Release -For weekly cross-language DRI coverage, use the -[management SDK release rotation runbook](mgmt-release-rotation-runbook.md). -This page covers package preparation; the runbook covers PR triage through -release follow-up, including the post-merge automatic release path. - ## General Release Process Overview Once Swagger PR is merged and a release plan/request is submitted, here are steps that you need to do in order to release a new package: From a1a50596cca3d80de41faa61f8366a30f1fa96b2 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 23 Oct 2026 17:45:00 +0800 Subject: [PATCH 03/11] docs: simplify management release rotation triage Use one PR queue, focus review on bot findings and CI, clarify re-approval, and fix documentation links. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1be9a77a-185e-4a49-bb2e-9f1f4d952fcc --- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 140 ++++++------------ 1 file changed, 49 insertions(+), 91 deletions(-) diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index eb7ac48a7775..804c6e11f010 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -15,94 +15,54 @@ to the Python Package Index (**PyPI**). **Contacts:** the **service team** is the submitter/contact in the PR or linked release plan. The **Python owner** is a management SDK maintainer listed under -`/**/*mgmt*/` in [CODEOWNERS](../../../.github/CODEOWNERS) -(currently @ChenxiJiang333, @msyyc, and @Azure/azure-arm-sdk-reviewers). +`/**/*mgmt*/` in [CODEOWNERS](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/CODEOWNERS). Ask the Python owner to coordinate code-generation, pipeline, or vendor support. Rotation assignment does not grant repository, CODEOWNER, or release permissions. ## 1. Find PRs needing attention -Use these live GitHub queries; open each PR for its current status. +Start from one [management AutoPR queue](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+sort%3Aupdated-desc). +It lists open, non-draft PRs, most recently updated first. Open each PR and check +for these three cases: -| Queue | Purpose | +| Needs attention | Next action | | --- | --- | -| [All open management AutoPRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle) | Includes drafts and PRs without release/review labels. Start here. | -| [PRs with automated review requested](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+label%3Amgmt-review-needed+draft%3Afalse) | Only PRs where a maintainer has added the label. This is not the full queue or proof of readiness. | -| [Approved release candidates](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+draft%3Afalse+review%3Aapproved) | Check for merge blockers and hand off to the service team. Do not add duplicate approval. | -| [Held release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3AHoldOn) | Wait for the planned window or the owner's decision. | -| [Recently merged release PRs](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Amerged+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+label%3Aauto-release+sort%3Aupdated-desc) | Follow up on publishing for this rotation's packages. Merged does not mean released. | - -An SDK maintainer manually adds `mgmt-review-needed` to request automated review. -Its absence does not mean a PR needs no attention; start from all open AutoPRs. -`auto-release` opts changed packages into post-merge release. -Neither label is approval; either can remain on a draft or an already reviewed PR. -An updated timestamp can reflect bot activity, not a recent service-team response. - -| Current state | Next action | -| --- | --- | -| Draft | Ask the service team to complete release prerequisites and mark the PR ready when intended. Do not mark it ready on their behalf. | -| `HoldOn`, `blocking-release`, or an explicit human hold | Keep the hold. Ask the person who owns it what must change before proceeding. | -| CI queued/running | Wait for the current revision's checks. Ask the Python owner about stuck or missing pipelines. | -| CI failed | Read the existing failure analysis and route the stated fix to the service team or Python owner. Do not restart review instead of fixing CI. | -| Ready, no valid approval | Read the bot review using section 2. If no review comment exists, ask an SDK maintainer to add `mgmt-review-needed`. | -| Approved, not merged | Identify the actual merge blocker or ask the service team to merge through its release plan. | +| No valid approval | Use the bot review and CI in section 2, then approve when ready. | +| New commits after approval | Inspect the changes and re-approve if required, using section 3. | +| New PR-author comments after approval | Read the request and respond or route it to the Python owner. A comment alone does not require another approval. | + +GitHub search does not isolate commits or author comments after approval. Check +the PR timeline; bot activity also updates the sort order. The queue does not +filter by `mgmt-review-needed` or `auto-release`: neither label proves readiness. +Drafts are excluded; the service team must mark them ready when intended. +Keep `HoldOn`, `blocking-release`, and explicit human holds until the owner clears +them. If approval is current and no request needs a response, do not approve again. ## 2. Request and read the bot review -Read the PR description, release plan, latest human decisions, and current checks. -Confirm the service team intends to release the listed package, SDK version, and -API version. Follow the linked +Confirm the service team's release intent in the PR or release plan, including the +package, SDK version, and API version. The service team owns the [SDK release prerequisites](https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites). -A bot-created PR is not itself a release request. - -For a ready PR, use this process rather than reviewing all generated code yourself: - -1. Look in the PR conversation for the latest **Management SDK PR review** bot comment. -2. If there is no bot review comment, ask an **SDK maintainer** to manually add - `mgmt-review-needed` under **Labels** in the PR sidebar. The label triggers the - [Python review workflow](../../../.github/workflows/mgmt-sdk-pr-review.md); - creating the AutoPR alone does not request this review. -3. Wait for the workflow to finish and the bot to leave its review comment. - The label itself is not a result. If no comment appears after the run finishes, - ask the Python owner to investigate. -4. Read the comment's findings, unverified checks, and suggested actions. - Blocking items explain what prevents approval and what needs to happen next. - Use the table below to identify who should act. -5. Check that the comment covers the latest PR changes. If commits were added - after the review, ask the SDK maintainer for an updated review. - -| Review result | Reviewer action | -| --- | --- | -| Complete, no findings or unverified checks | Use the completed checks and evidence; do not reconstruct them. Continue with human review below. | -| Blocking findings | Do not approve. Follow the comment's explanation and next action. Ask the service team to make the listed source/configuration fixes and regenerate; send code-generation tool issues to the Python owner. | -| Warning or suggestion | Read the finding and record its fix or justified disposition. Ask the Python owner when Python-specific judgment is needed. | -| Unverified checks, incomplete review, partial breaking-change attribution, or missing evidence | Ask the Python owner to complete or resolve the missing review. Missing evidence is not a pass. | -| Missing or outdated result | Ask the SDK maintainer to add `mgmt-review-needed`. If it is already present, the maintainer can remove and reapply it when no review is active. Wait for the new comment before continuing. | - -Adding the label triggers the workflow; leaving it on the PR does not request -another review after each push. Do not request another run while one is active. - -The bot checks package versions, preview/stable settings, client naming and usage, -and API-version changes. Its -[management review rules](../../../.github/instructions/reviewer/management.instructions.md) -define the scope. Do not repeat completed checks or review every generated model, -operation, sample, and test. - -**Human review still covers:** - -- Breaking changes and their disposition. The comment's **TypeSpec/API** category - means the change comes from the service specification; it does not approve the - break. Ask the service team to confirm intended service changes. Send - code-generation/migration changes, unclear attribution, and - exceptions to the Python owner. A major version bump alone is not permission. - Use the [breaking-change guide](sdk-breaking-changes-guide.md) for known patterns. -- Outstanding Copilot and human review comments. Ask for real defects to be fixed - upstream; record a reason when a suggestion does not apply. Do not require - output-only edits to satisfy a generated-code suggestion. -- First-release onboarding not covered by the result: confirm required namespace - approval, ownership, and a release pipeline with the Python owner. For a - confirmed first release, a client-name finding requires `client.tsp` - customization and regeneration, not a hand edit to `_client.py`. + +For routine review, focus on the bot comment and current CI: + +1. Read the latest **Management SDK PR review** bot comment. If it is missing or + outdated, ask an SDK maintainer to add `mgmt-review-needed`. This label triggers + the [review workflow](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/workflows/mgmt-sdk-pr-review.md). + If the label is already present, remove and reapply it only when no review is + active. Wait for the updated comment; ask the Python owner if none appears. +2. Follow the comment's findings and next actions. Ask the service team to resolve + reported source/configuration issues. Send incomplete or unverified results, + unclear breaking changes, and Python-specific decisions to the Python owner. + Missing evidence is not a pass; the bot does not authorize breaking changes. +3. Check CI for the current revision. Wait for running checks. For failures, use + the existing failure analysis and route the fix to the service team or Python + owner. Do not restart review instead of fixing CI. +4. Once the findings and outstanding human comments are resolved, use section 3 + to approve. Do not repeat completed bot checks or inspect every generated file. + +Leaving `mgmt-review-needed` on the PR does not request another review after a push. +Do not request another run while one is active. ## 3. Approve or re-approve @@ -111,17 +71,15 @@ readiness, current required CI passes, review findings have justified dispositions, and no unresolved review, evidence gap, or hold remains. The automated comment is advisory; it never grants human approval. -After a push or regeneration, inspect what changed since the previous approval. -Check the current GitHub review decision and branch requirements, not just an old -**Approved** event in the conversation. A previous approval can remain visible -while GitHub still requires review. +After new commits, inspect the changes since approval and request an updated bot +result when the previous result is outdated. Wait for current CI and resolve any +new findings. Ask the Python owner about API-version changes or unclear generated +changes. Re-approve only after reviewing the new changes and only if required by +the current GitHub review decision and branch requirements. An old **Approved** +event alone does not prove approval is current. -Recheck changed versions, changelog, API versions, customizations, and any affected -findings. Ask the SDK maintainer for a new bot review if the previous review is -outdated, and wait for current CI. Re-approve only after reviewing the new changes; do not blindly -repeat approval. If the current approval is still valid, do not add another one. -Route API-version changes after approval or unclear generated changes to the -Python owner. +Read new PR-author comments even when approval remains valid. Respond to the +request or route it to the Python owner; do not add a duplicate approval. ## 4. Hand off merge and follow the release @@ -132,7 +90,7 @@ Never bypass required reviews, checks, or holds. **Treat merge of an `auto-release` PR as a publishing action.** Confirm release intent and timing before merge. The -[release template](../../../eng/pipelines/templates/stages/archetype-python-release.yml) +[release template](https://github.com/Azure/azure-sdk-for-python/blob/main/eng/pipelines/templates/stages/archetype-python-release.yml) uses internal post-merge CI on `main` to select changed, declared package artifacts. This automatic path does **not** wait for the manual `package-publish` approval gate. Do not merge early assuming another approval will stop publication. @@ -152,7 +110,7 @@ Adding the label after the run is not a reliable recovery. Check for an active r or an already published version before any retry. For an intentional manual release, let the Python owner coordinate the -[unified release process](../release.md#releasing-through-unified-pipelines). +[unified release process](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/release.md#releasing-through-unified-pipelines). Only authorized release approvers validate and approve the intended package's artifacts. Do not queue a second release by default, approve unrelated packages, or upload manually with `twine`. @@ -164,9 +122,9 @@ or upload manually with `twine`. | "CI is green. Can you approve?" | Check readiness, the current automated result, breaking changes, and unresolved comments. Green CI alone is insufficient. | | "You approved already. Can you approve again?" | Compare the new changes and current review requirements using section 3. Do not add duplicate approval when none is needed. | | "Can you mark this draft ready or remove the hold?" | Ask the service team or hold owner to confirm readiness and timing. An approved draft is still a draft. | -| "Why did TypeSpec migration introduce breaking changes?" | Point to the attribution and breaking-change guide. Ask the Python owner to resolve unintended generator/migration breaks; do not accept them merely because code is generated. | +| "Why did TypeSpec migration introduce breaking changes?" | Point to the attribution and [breaking-change guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/sdk-breaking-changes-guide.md). Ask the Python owner to resolve unintended generator/migration breaks; do not accept them merely because code is generated. | | "The first package has no pipeline / needs a different client name." | Ask the Python owner to coordinate onboarding or pipeline preparation. The service team fixes naming in `client.tsp` and regenerates. | -| "Playback tests fail after the API version changed." | Route recording updates to the service team with the existing failure analysis and [management test guide](tests.md). Do not hand-edit recordings or disable tests to obtain green CI. | +| "Playback tests fail after the API version changed." | Route recording updates to the service team with the existing failure analysis and [management test guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/tests.md). Do not hand-edit recordings or disable tests to obtain green CI. | | "The PR merged. Where is the package?" | Inspect the post-merge release run and PyPI using section 4. Escalate the exact failed stage, not a request for another PR approval. | Historical examples inspected on 2026-10-23 illustrate these distinctions: From f61029a17918fc24c8c9680df124b06bfb7f21ed Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 23 Oct 2026 18:02:00 +0800 Subject: [PATCH 04/11] docs: clarify maintainer release support and remove section six Direct release pipeline failures to Engineering System support and remove US-based coverage guidance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1be9a77a-185e-4a49-bb2e-9f1f4d952fcc --- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 37 ++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index 804c6e11f010..7d4f584305b0 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -1,8 +1,9 @@ # Python management SDK release rotation runbook -This runbook is for Go/JavaScript/Python rotation reviewers who are unfamiliar -with Python SDK releases. It explains how to find a release PR, use the bot's -review, decide the next action, and follow the release. You do not need a local +This runbook is for SDK maintainers covering the Go/JavaScript/Python release +rotation who are unfamiliar with Python SDK releases. It explains how to find a +release PR, use the bot's review, decide the next action, and follow the release. +You do not need a local Python environment or detailed knowledge of generated Python code for routine cases. The reviewer on duty is the directly responsible individual (**DRI**). Ask the Python owner to review handwritten SDK or code-generation tool changes. @@ -16,7 +17,8 @@ to the Python Package Index (**PyPI**). **Contacts:** the **service team** is the submitter/contact in the PR or linked release plan. The **Python owner** is a management SDK maintainer listed under `/**/*mgmt*/` in [CODEOWNERS](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/CODEOWNERS). -Ask the Python owner to coordinate code-generation, pipeline, or vendor support. +Ask the Python owner to coordinate code-generation or vendor support. For release +pipeline failures, seek engineering team support using section 4. Rotation assignment does not grant repository, CODEOWNER, or release permissions. ## 1. Find PRs needing attention @@ -47,7 +49,7 @@ package, SDK version, and API version. The service team owns the For routine review, focus on the bot comment and current CI: 1. Read the latest **Management SDK PR review** bot comment. If it is missing or - outdated, ask an SDK maintainer to add `mgmt-review-needed`. This label triggers + outdated, add `mgmt-review-needed`. This label triggers the [review workflow](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/workflows/mgmt-sdk-pr-review.md). If the label is already present, remove and reapply it only when no review is active. Wait for the updated comment; ask the Python owner if none appears. @@ -103,8 +105,11 @@ package's `Release: ` stage. Confirm the exact version is available on Release-plan completion can fail separately from publication; do not republish an existing version to repair tracking. -If publishing is absent, skipped, failed, or unclear, send the PR, package/version, -merged commit, pipeline link, and failed/skipped stage to the Python owner. +If publishing is absent, skipped, failed, or unclear, investigate the release run. +For release pipeline failures, seek engineering team support in the +[Engineering System Teams channel](https://teams.microsoft.com/l/channel/19%3A59dbfadafb5e41c4890e2cd3d74cc7ba%40thread.skype/Engineering%20System%20%F0%9F%9B%A0%EF%B8%8F?groupId=3e17dcb0-4257-4a30-b843-77f47f1d4121&tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47). +Include the PR, package/version, merged commit, pipeline link, failed/skipped stage, +and relevant error details. Ask the Python owner about Python-specific issues. A missing `auto-release` label or artifact mapping can leave no automatic release. Adding the label after the run is not a reliable recovery. Check for an active run or an already published version before any retry. @@ -133,21 +138,3 @@ Historical examples inspected on 2026-10-23 illustrate these distinctions: [a corrected first-release naming blocker](https://github.com/Azure/azure-sdk-for-python/pull/49457), and [approval followed by a service-team merge/release handoff](https://github.com/Azure/azure-sdk-for-python/pull/48686). These examples are not the live queue. - -## 6. US-based coverage and rotation handoff - -US-based colleagues can triage the queue, relay existing findings, request -service-team confirmation, and identify the next owner while Python maintainers -are offline. Authorized rotation reviewers can approve ordinary cases after the -checks above. Geography does not grant or restrict approval authority. - -Do not give courtesy approvals, grant breaking-change exceptions, clear holds, -or merge early to bridge a time-zone gap. Do not regenerate packages, patch -generated output, rerun every check, or repeat completed automated analysis merely -to show progress. Use the Python owner for unresolved Python-specific decisions; -keep the blocker visible rather than substituting an uninformed approval. - -Leave one clear next action in the existing PR thread. At handoff, give the next -DRI the PR link, package/version, current blocker, responsible owner, required -action, and release deadline or planned window. Include the release run if merged. -Never hand off "approved" as a substitute for verified release completion. From 519b04d204df68874af3e694cf8e0bcc25b3db2a Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Fri, 23 Oct 2026 18:06:12 +0800 Subject: [PATCH 05/11] docs: mark release workflow sections as ordered steps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1be9a77a-185e-4a49-bb2e-9f1f4d952fcc --- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index 7d4f584305b0..78f2151a800a 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -3,6 +3,7 @@ This runbook is for SDK maintainers covering the Go/JavaScript/Python release rotation who are unfamiliar with Python SDK releases. It explains how to find a release PR, use the bot's review, decide the next action, and follow the release. +Follow Steps 1-4 in order for each PR. You do not need a local Python environment or detailed knowledge of generated Python code for routine cases. The reviewer on duty is the directly responsible individual (**DRI**). Ask the @@ -18,10 +19,10 @@ to the Python Package Index (**PyPI**). release plan. The **Python owner** is a management SDK maintainer listed under `/**/*mgmt*/` in [CODEOWNERS](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/CODEOWNERS). Ask the Python owner to coordinate code-generation or vendor support. For release -pipeline failures, seek engineering team support using section 4. +pipeline failures, seek engineering team support using Step 4. Rotation assignment does not grant repository, CODEOWNER, or release permissions. -## 1. Find PRs needing attention +## Step 1. Find PRs needing attention Start from one [management AutoPR queue](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+sort%3Aupdated-desc). It lists open, non-draft PRs, most recently updated first. Open each PR and check @@ -29,8 +30,8 @@ for these three cases: | Needs attention | Next action | | --- | --- | -| No valid approval | Use the bot review and CI in section 2, then approve when ready. | -| New commits after approval | Inspect the changes and re-approve if required, using section 3. | +| No valid approval | Use the bot review and CI in Step 2, then approve when ready. | +| New commits after approval | Inspect the changes and re-approve if required, using Step 3. | | New PR-author comments after approval | Read the request and respond or route it to the Python owner. A comment alone does not require another approval. | GitHub search does not isolate commits or author comments after approval. Check @@ -40,7 +41,7 @@ Drafts are excluded; the service team must mark them ready when intended. Keep `HoldOn`, `blocking-release`, and explicit human holds until the owner clears them. If approval is current and no request needs a response, do not approve again. -## 2. Request and read the bot review +## Step 2. Request and read the bot review Confirm the service team's release intent in the PR or release plan, including the package, SDK version, and API version. The service team owns the @@ -60,13 +61,13 @@ For routine review, focus on the bot comment and current CI: 3. Check CI for the current revision. Wait for running checks. For failures, use the existing failure analysis and route the fix to the service team or Python owner. Do not restart review instead of fixing CI. -4. Once the findings and outstanding human comments are resolved, use section 3 +4. Once the findings and outstanding human comments are resolved, use Step 3 to approve. Do not repeat completed bot checks or inspect every generated file. Leaving `mgmt-review-needed` on the PR does not request another review after a push. Do not request another run while one is active. -## 3. Approve or re-approve +## Step 3. Approve or re-approve Approve only if you are an authorized reviewer, the service team has confirmed readiness, current required CI passes, review findings have justified @@ -83,7 +84,7 @@ event alone does not prove approval is current. Read new PR-author comments even when approval remains valid. Respond to the request or route it to the Python owner; do not add a duplicate approval. -## 4. Hand off merge and follow the release +## Step 4. Hand off merge and follow the release Tell the service team when review is complete and point to its release plan's next action. The service team normally merges; an authorized DRI may help only @@ -125,12 +126,12 @@ or upload manually with `twine`. | Question or symptom | Response / owner | | --- | --- | | "CI is green. Can you approve?" | Check readiness, the current automated result, breaking changes, and unresolved comments. Green CI alone is insufficient. | -| "You approved already. Can you approve again?" | Compare the new changes and current review requirements using section 3. Do not add duplicate approval when none is needed. | +| "You approved already. Can you approve again?" | Compare the new changes and current review requirements using Step 3. Do not add duplicate approval when none is needed. | | "Can you mark this draft ready or remove the hold?" | Ask the service team or hold owner to confirm readiness and timing. An approved draft is still a draft. | | "Why did TypeSpec migration introduce breaking changes?" | Point to the attribution and [breaking-change guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/sdk-breaking-changes-guide.md). Ask the Python owner to resolve unintended generator/migration breaks; do not accept them merely because code is generated. | | "The first package has no pipeline / needs a different client name." | Ask the Python owner to coordinate onboarding or pipeline preparation. The service team fixes naming in `client.tsp` and regenerates. | | "Playback tests fail after the API version changed." | Route recording updates to the service team with the existing failure analysis and [management test guide](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/mgmt/tests.md). Do not hand-edit recordings or disable tests to obtain green CI. | -| "The PR merged. Where is the package?" | Inspect the post-merge release run and PyPI using section 4. Escalate the exact failed stage, not a request for another PR approval. | +| "The PR merged. Where is the package?" | Inspect the post-merge release run and PyPI using Step 4. Escalate the exact failed stage, not a request for another PR approval. | Historical examples inspected on 2026-10-23 illustrate these distinctions: [an approved draft](https://github.com/Azure/azure-sdk-for-python/pull/48930), From beb4983f2df3db6fc5cb1de36ca4b552f871bce4 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 26 Oct 2026 17:22:15 +0800 Subject: [PATCH 06/11] Add read-only management SDK PR triage skill Link release rotators to workflow-aware PR queues with offline tests and standalone usage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azsdk-python-mgmt-pr-triage/SKILL.md | 102 ++++ .../scripts/triage_prs.py | 452 +++++++++++++++++ .../tests/test_triage_prs.py | 470 ++++++++++++++++++ AGENTS.md | 1 + doc/dev/mgmt/mgmt-release-rotation-runbook.md | 14 + 5 files changed, 1039 insertions(+) create mode 100644 .github/skills/azsdk-python-mgmt-pr-triage/SKILL.md create mode 100644 .github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py create mode 100644 .github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md new file mode 100644 index 000000000000..46ee298d393d --- /dev/null +++ b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md @@ -0,0 +1,102 @@ +--- +name: azsdk-python-mgmt-pr-triage +description: "Quickly list open Python management SDK AutoPRs for release rotators: automated review needs triggering, review already active, human review needed, changes after approval, and author follow-up after approval. Read-only; verify workflow activity before recommending a review trigger." +--- + +# Python management SDK PR triage + +Use this skill when a rotator asks for management SDK PR queues, PRs needing +attention, or whether an automated management review is already active. +Follow the [release rotation runbook](../../../doc/dev/mgmt/mgmt-release-rotation-runbook.md) +for review and release decisions; this skill only helps find PRs. + +## Prerequisites + +- Python 3.10+ and authenticated GitHub CLI (`gh`) on PATH. +- Read access to repository contents, pull requests, and Actions runs. +- Read access to CODEOWNERS team membership when team owners apply. + +No SDK installation or local repository checkout is needed to run the bundled +script. In Copilot, ask: **"Use azsdk-python-mgmt-pr-triage to show management SDK +PRs needing attention."** If skills are not available in the current client, +run the script directly. + +## Run + +```powershell +python \scripts\triage_prs.py +``` + +Optional filters: + +```powershell +python \scripts\triage_prs.py --package azure-mgmt-iothub +python \scripts\triage_prs.py --version 1.0.0b1 +python \scripts\triage_prs.py --repo Azure/azure-sdk-for-python --limit 500 +``` + +Defaults: `Azure/azure-sdk-for-python`, at most 500 initially listed PRs, open +non-draft titles starting with `[AutoPR azure-mgmt-`. Include first releases. +Package and version filters are exact matches. Unknown versions are shown as +`unknown`; a version-filtered scan warns when it cannot classify a version. + +## Report + +Return the script's Markdown report and any diagnostic warnings, without +reinterpreting evidence as permission to approve or release. Preserve these +section numbers, omit empty sections, and sort each queue by most recently updated: + +1. **Automated review needs triggering**: the bot review is missing or its + linked run targets an older head, and no active review was detected. + Recommend adding `mgmt-review-needed`, or removing/reapplying it when present. +2. **Automated review active**: link queued, requested, waiting, pending, or + in-progress management review runs. Wait; do not request a duplicate, + including when the active run targets an older revision. +3. **Human review needed**: no effective approval from applicable CODEOWNERS, + or GitHub's review decision still requires review/changes. +4. **Changes after approval**: the current head differs from the effective + approved commit. Inspect the changes and current review requirements; + re-approve only if required. +5. **Author follow-up after approval**: link newer PR-author issue comments, + inline review comments, or submitted review bodies. Read and respond or + route the request; a comment does not itself require re-approval. + +Entries include PR link, package/version, reason, and next action. A PR may +appear in multiple queues. CODEOWNERS use GitHub file precedence and the last +matching rule for changed package files at the PR's base commit. Resolve team +members and use each maintainer's latest opinionated review; a later ordinary +comment does not invalidate an approval. + +## Evidence and limitations + +- Query the management review workflow, not generic CI. Use its run's explicit + `pull_requests` associations, not timestamps, title guesses, or `head_sha` + alone (`pull_request_target` can use a base revision). +- Active runs without a PR association make activity verification incomplete. + Show **Review activity unknown** and the run links; never recommend + retriggering while activity cannot be verified. +- Identify the latest review comment by the GitHub Actions account, review + heading, and workflow marker/footer. Check the linked run's workflow identity + and associated PR head for freshness. An absent/unusable run link or head + makes freshness unknown, not outdated. Incomplete bot results remain for + human review; do not automatically rerun a current partial result. +- Activity is a point-in-time observation, not a lock. Follow the run link and + confirm no review is active immediately before changing a label. +- Unknown ownership, inaccessible APIs, scan limits, changed PRs during + collection, and missing version metadata produce explicit verification + warnings. A partial scan must not claim that no PRs need attention. +- Exit code `0` means collection completed without verification warnings; + `2` means the report is partial/unverified or collection failed. Read the + report and stderr rather than treating an incomplete scan as an empty queue. + +Never change labels, request reviews, post comments, approve, merge, or publish. +Holds, CI, release plans, release intent, and post-merge publication remain +manual runbook checks. Absence from a queue is not proof of release readiness. + +## Development + +Run fixture-based tests without GitHub access: + +```powershell +python -m unittest discover -s .github\skills\azsdk-python-mgmt-pr-triage\tests -v +``` diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py new file mode 100644 index 000000000000..c88e64dcae32 --- /dev/null +++ b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py @@ -0,0 +1,452 @@ +#!/usr/bin/env python3 +"""Read-only management SDK PR queues for release rotators (Python 3.10+).""" + +from __future__ import annotations + +import argparse +import base64 +from concurrent.futures import ThreadPoolExecutor +from datetime import datetime, timezone +import json +import re +import subprocess +import sys +from threading import RLock +from urllib.parse import quote + +WORKFLOW = "mgmt-sdk-pr-review.lock.yml" +ACTIVE_STATUSES = ("queued", "in_progress", "waiting", "pending", "requested") +PACKAGE = re.compile(r"^\[AutoPR (azure-mgmt-[^\]\s]+)\]") +SECTIONS = ( + "Automated review needs triggering", + "Automated review active", + "Human review needed", + "Changes after approval", + "Author follow-up after approval", +) + + +class CollectionError(RuntimeError): + """GitHub evidence could not be collected.""" + + +def gh(*args): + try: + result = subprocess.run( + ["gh", *args], capture_output=True, text=True, encoding="utf-8", timeout=90, check=False + ) + except (OSError, subprocess.TimeoutExpired) as error: + raise CollectionError(f"GitHub CLI unavailable or timed out: {error}") from error + if result.returncode: + raise CollectionError(f"gh {' '.join(args)}: {result.stderr.strip()}") + try: + return json.loads(result.stdout) + except ValueError as error: + raise CollectionError("GitHub CLI returned invalid JSON.") from error + + +class GitHub: + def __init__(self, repo): + self.repo = repo + self.cache = {} + self.lock = RLock() + + def get(self, endpoint): + return gh("api", endpoint) + + def cached(self, endpoint): + with self.lock: + if endpoint not in self.cache: + try: + self.cache[endpoint] = self.get(endpoint) + except CollectionError as error: + self.cache[endpoint] = error + value = self.cache[endpoint] + if isinstance(value, CollectionError): + raise value + return value + + def pages(self, endpoint, key=None): + pages = gh("api", "--paginate", "--slurp", endpoint) + items = [item for page in pages for item in (page[key] if key else page)] + if key and pages and len(items) < pages[0].get("total_count", len(items)): + raise CollectionError(f"Incomplete pagination for {endpoint}.") + return items + + def owners(self, base_sha, files, package): + with self.lock: + cache_key = ("owners", base_sha) + if cache_key not in self.cache: + self.cache[cache_key] = self.load_rules(base_sha) + rules = self.cache[cache_key] + owners = set() + paths = [] + for file in files: + path = file["filename"] + if package in path.split("/"): + paths.append(file.get("previous_filename", path) if file["status"] == "renamed" else path) + if not paths: + raise CollectionError("No changed package files; cannot resolve maintainers.") + for path in paths: + matched = () + for pattern, candidates in rules: + if pattern.search(path): + matched = candidates + if not matched: + raise CollectionError(f"No CODEOWNERS for {path}.") + owners.update(matched) + maintainers = set() + for owner in owners: + if not owner.startswith("@"): + raise CollectionError(f"Cannot resolve CODEOWNERS identity {owner}.") + if "/" in owner: + org, team = owner[1:].split("/", 1) + with self.lock: + key = ("team", owner.casefold()) + if key not in self.cache: + self.cache[key] = self.pages(f"orgs/{org}/teams/{team}/members?per_page=100") + members = self.cache[key] + maintainers.update(member["login"].casefold() for member in members) + else: + maintainers.add(owner[1:].casefold()) + return maintainers, sorted(owners, key=str.casefold) + + def load_rules(self, base_sha): + for path in (".github/CODEOWNERS", "CODEOWNERS", "docs/CODEOWNERS"): + try: + data = self.cached(f"repos/{self.repo}/contents/{path}?ref={quote(base_sha, safe='')}") + except CollectionError as error: + if "HTTP 404" in str(error): + continue + raise + if data.get("encoding") != "base64": + raise CollectionError(f"Cannot decode {path}.") + return parse_codeowners(base64.b64decode(data["content"]).decode("utf-8")) + raise CollectionError("No base-branch CODEOWNERS found.") + + +def parse_codeowners(text): + rules = [] + for line in text.splitlines(): + fields = line.split("#", 1)[0].split() + if not fields or fields[0].startswith("!") or any(char in fields[0] for char in "[]"): + continue + pattern = fields[0] + if "\\" in pattern: + raise CollectionError("Escaped CODEOWNERS patterns require manual ownership verification.") + rooted = pattern.startswith("/") or "/" in pattern.rstrip("/") + directory = pattern.endswith("/") + parts = pattern.strip("/").split("/") + expression = "^" if rooted else r"(?:^|/)" + for index, part in enumerate(parts): + if part == "**": + expression += ".*" if index == len(parts) - 1 else "(?:[^/]+/)*" + else: + expression += "".join( + "[^/]*" if char == "*" else "[^/]" if char == "?" else re.escape(char) for char in part + ) + if index < len(parts) - 1: + expression += "/" + expression += "/" if directory else r"(?:/|$)" + rules.append((re.compile(expression), tuple(fields[1:]))) + if not rules: + raise CollectionError("No usable CODEOWNERS rules.") + return rules + + +def timestamp(value): + return datetime.fromisoformat(value.replace("Z", "+00:00")) + + +def effective_approval(reviews, maintainers): + latest = {} + for review in sorted( + (item for item in reviews if item.get("submitted_at")), + key=lambda item: (timestamp(item["submitted_at"]), item["id"]), + ): + login = (review.get("user") or {}).get("login", "").casefold() + if login in maintainers and review["state"] in {"APPROVED", "CHANGES_REQUESTED", "DISMISSED"}: + latest[login] = review + approvals = [review for review in latest.values() if review["state"] == "APPROVED"] + return max(approvals, key=lambda review: timestamp(review["submitted_at"]), default=None) + + +def sdk_version(files): + for file in files: + if file["filename"].endswith("/_version.py"): + match = re.search(r"""^\+\s*VERSION\s*=\s*["']([^"']+)""", file.get("patch", ""), re.MULTILINE) + if match: + return match[1] + for file in files: + if file["filename"].endswith("/CHANGELOG.md"): + match = re.search(r"^\+##\s+([0-9][^\s(]*)", file.get("patch", ""), re.MULTILINE) + if match: + return match[1] + return "unknown" + + +def run_pr(run, number): + return next((pr for pr in run.get("pull_requests", []) if pr["number"] == number), None) + + +def active_reviews(client): + runs = {} + issues = [] + for status in ACTIVE_STATUSES: + try: + for run in client.pages( + f"repos/{client.repo}/actions/workflows/{WORKFLOW}/runs?status={status}&per_page=100", + "workflow_runs", + ): + # A run can finish between status-filtered requests. + if run["status"] != "completed": + runs[run["id"]] = run + except CollectionError as error: + issues.append(f"Review activity unknown: {error}") + for run in runs.values(): + if not run.get("pull_requests"): + issues.append(f"Review activity unknown: active run has no PR association: {run['html_url']}") + return list(runs.values()), issues + + +def bot_review(comments): + candidates = [] + for comment in comments: + body = comment.get("body") or "" + if ( + (comment.get("user") or {}).get("login", "").casefold() == "github-actions[bot]" + and "## Management SDK PR review" in body + and ("" in body or "workflow_id: mgmt-sdk-pr-review," in body) + ): + candidates.append(comment) + return max(candidates, key=lambda item: (timestamp(item["created_at"]), item["id"]), default=None) + + +def review_freshness(client, workflow_id, pr, comment): + if comment is None: + return "missing" + match = re.search( + rf"https://github\.com/{re.escape(client.repo)}/actions/runs/([0-9]+)\b", comment["body"], re.IGNORECASE + ) + if not match: + raise CollectionError(f"Automated review freshness unknown: no linked workflow run in {comment['html_url']}") + run = client.cached(f"repos/{client.repo}/actions/runs/{match[1]}") + association = run_pr(run, pr["number"]) + if run["workflow_id"] != workflow_id or association is None or not association.get("head", {}).get("sha"): + raise CollectionError(f"Automated review freshness unknown: cannot verify linked run {run['html_url']}") + return "current" if association["head"]["sha"] == pr["head"]["sha"] else "outdated" + + +def collect_pr(client, summary): + number = summary["number"] + endpoint = f"repos/{client.repo}/pulls/{number}" + pr = client.get(endpoint) + if pr["state"] != "open" or pr["draft"]: + return None + match = PACKAGE.match(pr["title"]) + if not match: + return None + name = match[1] + files = client.pages(f"{endpoint}/files?per_page=100") + package_files = [file for file in files if name in file["filename"].split("/")] + if len(files) < pr["changed_files"]: + raise CollectionError(f"Changed-file listing incomplete for PR #{number}.") + comments = client.pages(f"repos/{client.repo}/issues/{number}/comments?per_page=100") + reviews = client.pages(f"{endpoint}/reviews?per_page=100") + issues = [] + try: + maintainers, owners = client.owners(pr["base"]["sha"], files, name) + except CollectionError as error: + maintainers, owners = None, [] + issues.append(f"Approval verification incomplete: {error}") + approval = effective_approval(reviews, maintainers) if maintainers is not None else None + followups = [] + if approval: + inline = client.pages(f"{endpoint}/comments?per_page=100") + author = pr["user"]["login"].casefold() + for comment in comments + inline + reviews: + created = comment.get("submitted_at") if "state" in comment else comment.get("created_at") + if ( + created + and (comment.get("user") or {}).get("login", "").casefold() == author + and (comment.get("body") or "").strip() + and timestamp(created) > timestamp(approval["submitted_at"]) + ): + followups.append(comment) + version = sdk_version(package_files) + if version == "unknown": + issues.append("SDK version unknown: no added version metadata in the package diff.") + current = client.get(endpoint) + changed = ( + any(current[key]["sha"] != pr[key]["sha"] for key in ("head", "base")) + or current["updated_at"] != pr["updated_at"] + ) + if current["state"] != "open" or current["draft"]: + return None + if changed: + raise CollectionError(f"PR #{number} changed during collection; rerun before acting.") + return { + "pr": pr, + "name": name, + "version": version, + "approval": approval, + "owners": owners, + "ownership_known": maintainers is not None, + "review_decision": summary.get("reviewDecision"), + "bot_review": bot_review(comments), + "followups": followups, + "issues": issues, + "queues": {}, + } + + +def classify(row, client, workflow_id, runs, activity_issues): + pr = row["pr"] + queues = row["queues"] + active = [run for run in runs if run_pr(run, pr["number"])] + if active: + links = [] + for run in active: + head = run_pr(run, pr["number"]).get("head", {}).get("sha") + revision = "; older/different head" if head and head != pr["head"]["sha"] else "" + links.append(f"[{run['status']}{revision}]({run['html_url']})") + queues[2] = f"Review active: {', '.join(links)}. Wait; do not request a duplicate." + elif activity_issues: + row["issues"].append("Review activity unknown; inspect workflow runs before changing any review label.") + else: + try: + freshness = review_freshness(client, workflow_id, pr, row["bot_review"]) + except CollectionError as error: + row["issues"].append(str(error)) + else: + if freshness in {"missing", "outdated"}: + labeled = any(label["name"].casefold() == "mgmt-review-needed" for label in pr["labels"]) + action = "Remove and reapply" if labeled else "Add" + queues[1] = ( + f"Bot review {freshness}; no active review detected. " + f"{action} `mgmt-review-needed` after confirming activity at the run link." + ) + approval = row["approval"] + if row["ownership_known"] and approval is None: + queues[3] = f"No effective maintainer approval. Ask {', '.join(row['owners'])} to review." + elif row["review_decision"] in {"REVIEW_REQUIRED", "CHANGES_REQUESTED"}: + queues[3] = f"GitHub review decision: {row['review_decision']}. Inspect outstanding review requirements." + if approval: + if not approval.get("commit_id"): + row["issues"].append("Approved commit unknown; cannot verify changes after approval.") + elif approval["commit_id"] != pr["head"]["sha"]: + queues[4] = ( + f"Head changed since [@{approval['user']['login']}'s approval]({approval['html_url']}). " + "Inspect the changes and current review requirements; re-approve only if required." + ) + if row["followups"]: + links = ", ".join(f"[comment]({comment['html_url']})" for comment in row["followups"]) + queues[5] = ( + f"Author follow-up after approval: {links}. Read and respond or route; do not duplicate approval." + ) + + +def render_report(repo, rows, issues): + rows = sorted(rows, key=lambda row: timestamp(row["pr"]["updated_at"]), reverse=True) + warnings = list(issues) + for row in rows: + warnings.extend(f"[{repo}#{row['pr']['number']}]({row['pr']['html_url']}): {issue}" for issue in row["issues"]) + runs_url = f"https://github.com/{repo}/actions/workflows/{WORKFLOW}" + output = [ + f"Scanned {len(rows)} open, non-draft management AutoPRs" + ("; verification incomplete." if warnings else "."), + "", + f"[Management review runs]({runs_url}). Point-in-time report; confirm activity before changing labels.", + ] + for index, title in enumerate(SECTIONS, 1): + entries = [row for row in rows if index in row["queues"]] + if entries: + output.extend(["", f"## {index}. {title}", ""]) + for row in entries: + pr = row["pr"] + output.append( + f"- [{repo}#{pr['number']}]({pr['html_url']}) - `{row['name']}` `{row['version']}`" + f" - {row['queues'][index]}" + ) + if warnings: + output.extend(["", "## Verification warnings", ""]) + output.extend(f"- {' '.join(issue.split())}" for issue in dict.fromkeys(warnings)) + elif not any(row["queues"] for row in rows): + output.extend(["", "No PRs need action in these queues. Release readiness is not assessed."]) + return "\n".join(output), bool(warnings) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", default="Azure/azure-sdk-for-python") + parser.add_argument("--package", help="Exact package name") + parser.add_argument("--version", help="Exact SDK version (first releases included by default)") + parser.add_argument("--limit", type=int, default=500, help="Maximum initial PR listing size") + args = parser.parse_args(argv) + if args.limit < 1: + parser.error("--limit must be positive") + if not re.fullmatch(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", args.repo): + parser.error("--repo must be owner/name") + if args.package and not re.fullmatch(r"azure-mgmt-[A-Za-z0-9-]+", args.package): + parser.error("--package must be an azure-mgmt-* package name") + client = GitHub(args.repo) + issues = [] + try: + prs = gh( + "pr", + "list", + "--repo", + args.repo, + "--state", + "open", + "--search", + '"[AutoPR azure-mgmt-" in:title draft:false sort:updated-desc', + "--limit", + str(args.limit), + "--json", + "number,title,isDraft,reviewDecision", + ) + if len(prs) >= args.limit: + issues.append(f"PR listing reached --limit {args.limit}; increase it for a complete scan.") + selected = [] + for pr in prs: + match = PACKAGE.match(pr["title"]) + if match and not pr["isDraft"] and (not args.package or match[1] == args.package): + selected.append(pr) + rows = [] + + def collect(summary): + try: + return collect_pr(client, summary), None + except CollectionError as error: + return None, f"PR #{summary['number']}: collection incomplete: {error}" + + with ThreadPoolExecutor(max_workers=4) as pool: + for row, error in pool.map(collect, selected): + if error: + issues.append(error) + elif row: + if not args.version or row["version"] == args.version: + rows.append(row) + elif row["version"] == "unknown": + issues.append(f"PR #{row['pr']['number']}: unknown version; cannot apply --version filter.") + # Refresh activity after collecting PR evidence, close to report generation. + runs, activity_issues = active_reviews(client) if rows else ([], []) + issues.extend(activity_issues) + try: + workflow_id = client.cached(f"repos/{args.repo}/actions/workflows/{WORKFLOW}")["id"] if rows else None + except CollectionError as error: + workflow_id = None + issues.append(f"Automated review workflow identity unknown: {error}") + for row in rows: + classify(row, client, workflow_id, runs, activity_issues) + report, incomplete = render_report(args.repo, rows, issues) + print(f"Observed at {datetime.now(timezone.utc).isoformat()}.") + print(report) + return 2 if incomplete else 0 + except CollectionError as error: + print(f"Scan failed; no complete PR report is available: {error}", file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py new file mode 100644 index 000000000000..02f0a2965de2 --- /dev/null +++ b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py @@ -0,0 +1,470 @@ +"""Offline fixtures for release-rotation queues and conservative activity checks.""" + +import base64 +from copy import deepcopy +import importlib.util +import io +from pathlib import Path +import unittest +from unittest.mock import patch + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "triage_prs.py" +SPEC = importlib.util.spec_from_file_location("triage_prs", SCRIPT) +triage = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(triage) + +REPO = "Azure/azure-sdk-for-python" +PR_URL = f"https://github.com/{REPO}/pull/42" +RUN_URL = f"https://github.com/{REPO}/actions/runs/100" +HEAD = "a" * 40 +OLD = "b" * 40 +WHEN = "2026-10-26T09:00:00Z" +LATER = "2026-10-26T10:00:00Z" +PR = { + "number": 42, + "title": "[AutoPR azure-mgmt-example]-generated", + "state": "open", + "draft": False, + "html_url": PR_URL, + "updated_at": WHEN, + "head": {"sha": HEAD}, + "base": {"sha": OLD}, + "changed_files": 1, + "user": {"login": "author"}, + "labels": [], +} +FILE = { + "filename": "sdk/example/azure-mgmt-example/azure/mgmt/example/_version.py", + "status": "modified", + "patch": '+VERSION = "1.0.0b1"', +} + + +def review(state="APPROVED", login="maintainer", when=WHEN, identifier=1, commit=HEAD): + return { + "id": identifier, + "user": {"login": login}, + "state": state, + "submitted_at": when, + "commit_id": commit, + "html_url": PR_URL + f"#pullrequestreview-{identifier}", + "body": "", + } + + +def run(status="completed", head=HEAD, associated=True): + return { + "id": 100, + "workflow_id": 9, + "status": status, + "html_url": RUN_URL, + # Deliberately different from the associated PR head. + "head_sha": OLD, + "pull_requests": [{"number": 42, "head": {"sha": head}}] if associated else [], + } + + +def comment(body="Please check", login="author", when=LATER, identifier=2): + return { + "id": identifier, + "body": body, + "user": {"login": login}, + "created_at": when, + "html_url": PR_URL + f"#issuecomment-{identifier}", + } + + +def bot(body=None): + return comment( + body + or ( + "## Management SDK PR review\nPartial result; human review required.\n" + f"" + ), + "github-actions[bot]", + ) + + +def row(approval=None, bot_comment=None): + return { + "pr": deepcopy(PR), + "name": "azure-mgmt-example", + "version": "1.0.0b1", + "approval": approval, + "owners": ["@maintainer"], + "ownership_known": True, + "review_decision": "APPROVED" if approval else "REVIEW_REQUIRED", + "bot_review": bot_comment, + "followups": [], + "issues": [], + "queues": {}, + } + + +class FakeGitHub: + repo = REPO + + def __init__(self): + self.details = deepcopy(PR) + self.reviews = [review()] + self.comments = [bot()] + self.inline = [] + self.files = [deepcopy(FILE)] + self.linked_run = run() + self.workflow_runs = [] + self.ownership_error = False + self.activity_error = False + self.last_details = None + self.get_count = 0 + + def get(self, endpoint): + self.get_count += 1 + if self.get_count > 1 and self.last_details is not None: + return deepcopy(self.last_details) + return deepcopy(self.details) + + def cached(self, endpoint): + if "/actions/runs/" in endpoint: + return deepcopy(self.linked_run) + return {"id": 9} + + def pages(self, endpoint, key=None): + if "/actions/workflows/" in endpoint: + if self.activity_error: + raise triage.CollectionError("HTTP 403") + return [deepcopy(item) for item in self.workflow_runs if f"status={item['status']}" in endpoint] + if "/reviews?" in endpoint: + return deepcopy(self.reviews) + if "/issues/" in endpoint: + return deepcopy(self.comments) + if "/comments?" in endpoint: + return deepcopy(self.inline) + return deepcopy(self.files) + + def owners(self, base_sha, files, package): + if self.ownership_error: + raise triage.CollectionError("Team membership unreadable") + return {"maintainer"}, ["@maintainer"] + + +class ApprovalTests(unittest.TestCase): + def test_comment_does_not_invalidate_approval(self): + reviews = [review(), review("COMMENTED", when=LATER, identifier=2)] + self.assertEqual(triage.effective_approval(reviews, {"maintainer"})["id"], 1) + + def test_changes_requested_or_dismissal_invalidates_approval(self): + for state in ("CHANGES_REQUESTED", "DISMISSED"): + with self.subTest(state=state): + self.assertIsNone( + triage.effective_approval([review(), review(state, when=LATER, identifier=2)], {"maintainer"}) + ) + + def test_later_approval_and_login_case(self): + reviews = [review("CHANGES_REQUESTED"), review(login="Maintainer", when=LATER, identifier=2)] + self.assertEqual(triage.effective_approval(reviews, {"maintainer"})["id"], 2) + self.assertIsNone(triage.effective_approval(reviews, {"other"})) + + def test_pending_review_ignored(self): + pending = review("PENDING", identifier=2) + pending["submitted_at"] = None + self.assertEqual(triage.effective_approval([pending, review()], {"maintainer"})["id"], 1) + + +class OwnershipTests(unittest.TestCase): + def setUp(self): + self.client = triage.GitHub(REPO) + + def test_last_rule_and_nested_double_star(self): + text = "/sdk/ @general\n/**/*mgmt*/ @maintainer\n/sdk/example/azure-mgmt-example/private/ @special\n" + with patch.object(self.client, "load_rules", return_value=triage.parse_codeowners(text)) as load: + owners = self.client.owners(OLD, [FILE], "azure-mgmt-example") + self.assertEqual(owners, ({"maintainer"}, ["@maintainer"])) + self.client.owners(OLD, [FILE], "azure-mgmt-example") + load.assert_called_once() + special = dict(FILE, filename="sdk/example/azure-mgmt-example/private/client.py") + self.assertEqual(self.client.owners(OLD, [special], "azure-mgmt-example")[0], {"special"}) + + def test_rename_uses_previous_path_and_unrelated_files_ignored(self): + rules = triage.parse_codeowners("/sdk/ @general\n/sdk/example/azure-mgmt-example/old.py @old") + renamed = dict( + FILE, + status="renamed", + filename="sdk/example/azure-mgmt-example/new.py", + previous_filename="sdk/example/azure-mgmt-example/old.py", + ) + with patch.object(self.client, "load_rules", return_value=rules): + self.assertEqual( + self.client.owners(OLD, [renamed, {"filename": "eng/file.py"}], "azure-mgmt-example")[0], {"old"} + ) + + def test_missing_owner_is_unknown(self): + with patch.object(self.client, "load_rules", return_value=triage.parse_codeowners("/elsewhere/ @other")): + with self.assertRaises(triage.CollectionError): + self.client.owners(OLD, [FILE], "azure-mgmt-example") + + def test_team_members_cached_and_casefolded(self): + rules = triage.parse_codeowners("/sdk/ @Azure/owners") + with patch.object(self.client, "load_rules", return_value=rules), patch.object( + self.client, "pages", return_value=[{"login": "Maintainer"}] + ) as pages: + self.assertEqual(self.client.owners(OLD, [FILE], "azure-mgmt-example")[0], {"maintainer"}) + self.client.owners(OLD, [FILE], "azure-mgmt-example") + pages.assert_called_once() + + def test_codeowners_file_precedence(self): + content = base64.b64encode(b"/sdk/ @maintainer").decode() + with patch.object(self.client, "cached", return_value={"encoding": "base64", "content": content}) as cached: + self.client.load_rules(OLD) + self.assertIn("/contents/.github/CODEOWNERS?", cached.call_args.args[0]) + cached.assert_called_once() + + def test_empty_override_does_not_fall_back(self): + rules = triage.parse_codeowners("/sdk/ @maintainer\n/sdk/example/azure-mgmt-example/\n") + with patch.object(self.client, "load_rules", return_value=rules): + with self.assertRaises(triage.CollectionError): + self.client.owners(OLD, [FILE], "azure-mgmt-example") + + +class ActivityTests(unittest.TestCase): + def test_all_active_states_including_old_head(self): + for status in triage.ACTIVE_STATUSES: + with self.subTest(status=status): + item = row() + triage.classify(item, FakeGitHub(), 9, [run(status, OLD)], []) + self.assertIn(2, item["queues"]) + self.assertNotIn(1, item["queues"]) + self.assertIn("older/different head", item["queues"][2]) + + def test_unassociated_run_prevents_trigger_recommendation(self): + client = FakeGitHub() + client.workflow_runs = [run("queued", associated=False)] + runs, issues = triage.active_reviews(client) + item = row() + triage.classify(item, client, 9, runs, issues) + self.assertNotIn(1, item["queues"]) + self.assertIn("Review activity unknown", " ".join(item["issues"])) + self.assertIn(RUN_URL, " ".join(issues)) + + def test_activity_access_failure_is_unknown(self): + client = FakeGitHub() + client.activity_error = True + runs, issues = triage.active_reviews(client) + item = row() + triage.classify(item, client, 9, runs, issues) + self.assertNotIn(1, item["queues"]) + self.assertTrue(issues) + + def test_other_pr_active_run_does_not_block(self): + other = run("queued") + other["pull_requests"][0]["number"] = 43 + item = row() + triage.classify(item, FakeGitHub(), 9, [other], []) + self.assertIn(1, item["queues"]) + self.assertNotIn(2, item["queues"]) + + def test_label_presence_only_changes_trigger_instruction(self): + for labeled in (False, True): + with self.subTest(labeled=labeled): + item = row() + item["pr"]["labels"] = [{"name": "mgmt-review-needed"}] if labeled else [] + triage.classify(item, FakeGitHub(), 9, [], []) + self.assertIn("Remove and reapply" if labeled else "Add", item["queues"][1]) + + def test_status_requests_paginate_once_each(self): + client = FakeGitHub() + client.workflow_runs = [run("in_progress")] + runs, issues = triage.active_reviews(client) + self.assertEqual(len(runs), 1) + self.assertFalse(issues) + + +class FreshnessTests(unittest.TestCase): + def test_current_comment_with_different_run_head_sha(self): + self.assertEqual(triage.review_freshness(FakeGitHub(), 9, PR, bot()), "current") + item = row(review(), bot()) + triage.classify(item, FakeGitHub(), 9, [], []) + self.assertFalse(item["queues"]) + + def test_older_linked_pr_head_is_outdated(self): + client = FakeGitHub() + client.linked_run = run(head=OLD) + item = row(bot_comment=bot()) + triage.classify(item, client, 9, [], []) + self.assertIn("outdated", item["queues"][1]) + + def test_unverifiable_link_never_recommends_trigger(self): + for linked in (dict(run(), workflow_id=8), run(associated=False), run(head=None)): + with self.subTest(linked=linked): + client = FakeGitHub() + client.linked_run = linked + item = row(bot_comment=bot()) + triage.classify(item, client, 9, [], []) + self.assertNotIn(1, item["queues"]) + self.assertIn("freshness unknown", item["issues"][0]) + + def test_missing_run_link_is_unknown(self): + with self.assertRaises(triage.CollectionError): + triage.review_freshness(FakeGitHub(), 9, PR, bot("No linked run")) + + def test_heading_alone_does_not_identify_automated_review(self): + self.assertIsNone(triage.bot_review([comment("## Management SDK PR review")])) + fake = bot() + fake["user"]["login"] = "someone" + self.assertIsNone(triage.bot_review([fake])) + self.assertEqual(triage.bot_review([bot()])["id"], 2) + + def test_old_workflow_marker_supported(self): + item = bot("\n## Management SDK PR review") + self.assertEqual(triage.bot_review([item]), item) + + +class CollectionTests(unittest.TestCase): + def test_first_beta_and_author_followups_in_all_comment_channels(self): + client = FakeGitHub() + client.comments.append(comment()) + client.inline = [comment(identifier=3)] + author_review = review("COMMENTED", "author", LATER, 4) + author_review["body"] = "Ready now?" + client.reviews.append(author_review) + item = triage.collect_pr(client, {"number": 42, "reviewDecision": "APPROVED"}) + self.assertEqual(item["version"], "1.0.0b1") + self.assertEqual(len(item["followups"]), 3) + triage.classify(item, client, 9, [], []) + self.assertIn(5, item["queues"]) + self.assertNotIn(4, item["queues"]) + + def test_comments_before_approval_or_by_other_people_not_followups(self): + client = FakeGitHub() + client.comments += [comment(when=WHEN), comment(login="other")] + item = triage.collect_pr(client, {"number": 42}) + self.assertFalse(item["followups"]) + + def test_old_commit_timestamps_cannot_hide_changed_head(self): + item = row(review(commit=OLD), bot()) + triage.classify(item, FakeGitHub(), 9, [], []) + self.assertIn(4, item["queues"]) + + def test_missing_commit_is_verification_warning(self): + item = row(review(commit=None), bot()) + triage.classify(item, FakeGitHub(), 9, [], []) + self.assertTrue(item["issues"]) + self.assertNotIn(4, item["queues"]) + + def test_ownership_failure_is_not_missing_approval(self): + client = FakeGitHub() + client.ownership_error = True + item = triage.collect_pr(client, {"number": 42}) + triage.classify(item, client, 9, [], []) + self.assertNotIn(3, item["queues"]) + self.assertIn("Approval verification incomplete", item["issues"][0]) + + def test_github_review_requirement_even_with_approval(self): + item = row(review(), bot()) + item["review_decision"] = "CHANGES_REQUESTED" + triage.classify(item, FakeGitHub(), 9, [], []) + self.assertIn(3, item["queues"]) + + def test_changed_pr_requires_rescan(self): + client = FakeGitHub() + client.last_details = deepcopy(PR) + client.last_details["head"]["sha"] = OLD + with self.assertRaises(triage.CollectionError): + triage.collect_pr(client, {"number": 42}) + + def test_newly_closed_or_draft_pr_skipped(self): + for changes in ({"state": "closed"}, {"draft": True}): + client = FakeGitHub() + client.details.update(changes) + self.assertIsNone(triage.collect_pr(client, {"number": 42})) + + def test_truncated_files_not_treated_as_complete(self): + client = FakeGitHub() + client.details["changed_files"] = 3001 + with self.assertRaises(triage.CollectionError): + triage.collect_pr(client, {"number": 42}) + + def test_version_changelog_fallback(self): + file = dict(FILE, filename="sdk/example/azure-mgmt-example/CHANGELOG.md", patch="+## 2.0.0 (Unreleased)") + self.assertEqual(triage.sdk_version([file]), "2.0.0") + + +class ReportAndCliTests(unittest.TestCase): + def test_partial_scan_never_claims_no_action(self): + report, incomplete = triage.render_report(REPO, [], ["HTTP 403"]) + self.assertTrue(incomplete) + self.assertNotIn("No PRs need action", report) + self.assertIn("verification incomplete", report) + + def test_queue_numbers_preserved_and_rows_sorted(self): + first, second = row(), row() + second["pr"]["number"] = 43 + second["pr"]["html_url"] = PR_URL.replace("42", "43") + second["pr"]["updated_at"] = LATER + first["queues"][4] = second["queues"][4] = "Inspect changes." + report, incomplete = triage.render_report(REPO, [first, second], []) + self.assertFalse(incomplete) + self.assertIn("## 4. Changes after approval", report) + self.assertNotIn("## 1.", report) + self.assertLess(report.index(f"{REPO}#43"), report.index(f"{REPO}#42")) + + def test_paginated_lists_and_incomplete_objects(self): + client = triage.GitHub(REPO) + with patch.object(triage, "gh", return_value=[[{"id": 1}], [{"id": 2}]]): + self.assertEqual(len(client.pages("endpoint")), 2) + with patch.object(triage, "gh", return_value=[{"total_count": 2, "workflow_runs": [run()]}]): + with self.assertRaises(triage.CollectionError): + client.pages("endpoint", "workflow_runs") + + def test_cli_default_includes_first_beta(self): + listing = [{"number": 42, "title": PR["title"], "isDraft": False, "reviewDecision": "APPROVED"}] + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "GitHub", return_value=FakeGitHub() + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main([]), 0) + self.assertIn("Scanned 1", output.getvalue()) + self.assertIn("No PRs need action", output.getvalue()) + + def test_cli_limit_marks_partial(self): + listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "GitHub", return_value=FakeGitHub() + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main(["--limit", "1"]), 2) + self.assertIn("reached --limit", output.getvalue()) + self.assertNotIn("No PRs need action", output.getvalue()) + + def test_cli_exact_package_filter_and_drafts(self): + listing = [ + {"number": 42, "title": PR["title"], "isDraft": False}, + {"number": 43, "title": "[AutoPR azure-mgmt-other]-generated", "isDraft": False}, + {"number": 44, "title": PR["title"], "isDraft": True}, + ] + client = FakeGitHub() + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "GitHub", return_value=client + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main(["--package", "azure-mgmt-example"]), 0) + self.assertIn("Scanned 1", output.getvalue()) + + def test_cli_unknown_version_cannot_silently_match_filter(self): + listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + client = FakeGitHub() + client.files[0]["patch"] = "" + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "GitHub", return_value=client + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main(["--version", "1.0.0b1"]), 2) + self.assertIn("cannot apply --version", output.getvalue()) + + def test_cli_collection_failure_reports_partial(self): + listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "collect_pr", side_effect=triage.CollectionError("HTTP 403") + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main([]), 2) + self.assertIn("collection incomplete", output.getvalue()) + self.assertNotIn("No PRs need action", output.getvalue()) + + +if __name__ == "__main__": + unittest.main() diff --git a/AGENTS.md b/AGENTS.md index 613eeec05ba0..e95d2afba477 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,6 +53,7 @@ Always: | Add a feature or fix a bug in a package under `sdk/` | [`.github/skills/find-package-skill/SKILL.md`](.github/skills/find-package-skill/SKILL.md) (check FIRST for package-specific skills) | | Generate or regenerate SDK code from TypeSpec, build, and validate | [`.github/skills/azsdk-common-generate-sdk-locally/SKILL.md`](.github/skills/azsdk-common-generate-sdk-locally/SKILL.md) | | Release / publish a package | [`.github/skills/azsdk-common-sdk-release/SKILL.md`](.github/skills/azsdk-common-sdk-release/SKILL.md) | +| Find management SDK PRs needing attention or check active automated reviews | [`.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md`](.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md) | | Prepare a release plan | [`.github/skills/azsdk-common-prepare-release-plan/SKILL.md`](.github/skills/azsdk-common-prepare-release-plan/SKILL.md) | | Resolve APIView feedback | [`.github/skills/azsdk-common-apiview-feedback-resolution/SKILL.md`](.github/skills/azsdk-common-apiview-feedback-resolution/SKILL.md) | | Analyze a CI / pipeline failure | [`.github/skills/azsdk-common-pipeline-analysis/SKILL.md`](.github/skills/azsdk-common-pipeline-analysis/SKILL.md) | diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index 78f2151a800a..8356c960942f 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -24,6 +24,14 @@ Rotation assignment does not grant repository, CODEOWNER, or release permissions ## Step 1. Find PRs needing attention +For a quick grouped list, ask Copilot: **"Use azsdk-python-mgmt-pr-triage to show +management SDK PRs needing attention."** The read-only +[PR triage skill](../../../.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md) +lists automated reviews to trigger, active reviews, missing human approvals, +changes after approval, and author follow-ups. Its instructions also include a +standalone command for clients without skill support. Check any verification +warnings; the report does not establish release readiness. + Start from one [management AutoPR queue](https://github.com/Azure/azure-sdk-for-python/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse+%22%5BAutoPR+azure-mgmt-%22+in%3Atitle+sort%3Aupdated-desc). It lists open, non-draft PRs, most recently updated first. Open each PR and check for these three cases: @@ -66,6 +74,12 @@ For routine review, focus on the bot comment and current CI: Leaving `mgmt-review-needed` on the PR does not request another review after a push. Do not request another run while one is active. +To check quickly, use the PR triage skill's **Automated review active** queue, +which links the management review workflow runs associated with each PR. +Queued or waiting runs also count as active. If it reports **Review activity +unknown**, inspect the linked workflow runs or ask the Python owner; do not +assume it is safe to retrigger. Activity can change after a scan, so confirm +the linked run status immediately before removing and reapplying the label. ## Step 3. Approve or re-approve From e190d3c0ac3919b9804b4b5b964f76ad045f8c69 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 26 Oct 2026 17:28:03 +0800 Subject: [PATCH 07/11] Remove management triage skill routing from AGENTS.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 1 - 1 file changed, 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index e95d2afba477..613eeec05ba0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,7 +53,6 @@ Always: | Add a feature or fix a bug in a package under `sdk/` | [`.github/skills/find-package-skill/SKILL.md`](.github/skills/find-package-skill/SKILL.md) (check FIRST for package-specific skills) | | Generate or regenerate SDK code from TypeSpec, build, and validate | [`.github/skills/azsdk-common-generate-sdk-locally/SKILL.md`](.github/skills/azsdk-common-generate-sdk-locally/SKILL.md) | | Release / publish a package | [`.github/skills/azsdk-common-sdk-release/SKILL.md`](.github/skills/azsdk-common-sdk-release/SKILL.md) | -| Find management SDK PRs needing attention or check active automated reviews | [`.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md`](.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md) | | Prepare a release plan | [`.github/skills/azsdk-common-prepare-release-plan/SKILL.md`](.github/skills/azsdk-common-prepare-release-plan/SKILL.md) | | Resolve APIView feedback | [`.github/skills/azsdk-common-apiview-feedback-resolution/SKILL.md`](.github/skills/azsdk-common-apiview-feedback-resolution/SKILL.md) | | Analyze a CI / pipeline failure | [`.github/skills/azsdk-common-pipeline-analysis/SKILL.md`](.github/skills/azsdk-common-pipeline-analysis/SKILL.md) | From 5a2c980fb643a21c0ce22ba9642ffd80c25e166f Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Mon, 26 Oct 2026 17:36:16 +0800 Subject: [PATCH 08/11] Ignore expected management reviewer team membership 403 Keep named CODEOWNER approvals and use GitHub review decisions when the optional reviewer team cannot be read. Cache the expected denial and retain warnings for other errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azsdk-python-mgmt-pr-triage/SKILL.md | 12 +++- .../scripts/triage_prs.py | 30 ++++++-- .../tests/test_triage_prs.py | 68 ++++++++++++++++++- 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md index 46ee298d393d..0bc29ab48bb0 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md +++ b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md @@ -14,7 +14,8 @@ for review and release decisions; this skill only helps find PRs. - Python 3.10+ and authenticated GitHub CLI (`gh`) on PATH. - Read access to repository contents, pull requests, and Actions runs. -- Read access to CODEOWNERS team membership when team owners apply. +- Read access to CODEOWNERS team membership when team owners apply, except + the optional `Azure/azure-arm-sdk-reviewers` lookup described below. No SDK installation or local repository checkout is needed to run the bundled script. In Copilot, ask: **"Use azsdk-python-mgmt-pr-triage to show management SDK @@ -67,6 +68,15 @@ matching rule for changed package files at the PR's base commit. Resolve team members and use each maintainer's latest opinionated review; a later ordinary comment does not invalidate an approval. +If the `Azure/azure-arm-sdk-reviewers` membership lookup returns HTTP 403, +silently skip that team lookup and cache the failure for the scan. Keep +approvals from individually named CODEOWNERS and other readable teams. Use +GitHub's review decision for the human-review queue when a team-only approval +cannot be verified; do not claim an approval is missing solely because this +team is unreadable. Changes and author follow-ups after a team-only approval +cannot be classified unless the approver is also a verified maintainer. +Other team errors and API failures still produce verification warnings. + ## Evidence and limitations - Query the management review workflow, not generic CI. Use its run's explicit diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py index c88e64dcae32..43ac87a15888 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py +++ b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py @@ -96,6 +96,7 @@ def owners(self, base_sha, files, package): raise CollectionError(f"No CODEOWNERS for {path}.") owners.update(matched) maintainers = set() + skipped_team = False for owner in owners: if not owner.startswith("@"): raise CollectionError(f"Cannot resolve CODEOWNERS identity {owner}.") @@ -104,12 +105,20 @@ def owners(self, base_sha, files, package): with self.lock: key = ("team", owner.casefold()) if key not in self.cache: - self.cache[key] = self.pages(f"orgs/{org}/teams/{team}/members?per_page=100") + try: + self.cache[key] = self.pages(f"orgs/{org}/teams/{team}/members?per_page=100") + except CollectionError as error: + self.cache[key] = error members = self.cache[key] + if isinstance(members, CollectionError): + if owner.casefold() == "@azure/azure-arm-sdk-reviewers" and "HTTP 403" in str(members): + skipped_team = True + continue + raise members maintainers.update(member["login"].casefold() for member in members) else: maintainers.add(owner[1:].casefold()) - return maintainers, sorted(owners, key=str.casefold) + return maintainers, sorted(owners, key=str.casefold), skipped_team def load_rules(self, base_sha): for path in (".github/CODEOWNERS", "CODEOWNERS", "docs/CODEOWNERS"): @@ -255,11 +264,22 @@ def collect_pr(client, summary): reviews = client.pages(f"{endpoint}/reviews?per_page=100") issues = [] try: - maintainers, owners = client.owners(pr["base"]["sha"], files, name) + maintainers, owners, skipped_team = client.owners(pr["base"]["sha"], files, name) except CollectionError as error: - maintainers, owners = None, [] + maintainers, owners, skipped_team = None, [], False issues.append(f"Approval verification incomplete: {error}") approval = effective_approval(reviews, maintainers) if maintainers is not None else None + if ( + skipped_team + and approval is None + and summary.get("reviewDecision") + not in { + "APPROVED", + "REVIEW_REQUIRED", + "CHANGES_REQUESTED", + } + ): + issues.append("GitHub review decision unavailable; cannot verify team-only approval.") followups = [] if approval: inline = client.pages(f"{endpoint}/comments?per_page=100") @@ -291,7 +311,7 @@ def collect_pr(client, summary): "version": version, "approval": approval, "owners": owners, - "ownership_known": maintainers is not None, + "ownership_known": maintainers is not None and not skipped_team, "review_decision": summary.get("reviewDecision"), "bot_review": bot_review(comments), "followups": followups, diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py index 02f0a2965de2..82afbda0b82e 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py +++ b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py @@ -113,6 +113,7 @@ def __init__(self): self.linked_run = run() self.workflow_runs = [] self.ownership_error = False + self.skipped_team = False self.activity_error = False self.last_details = None self.get_count = 0 @@ -144,7 +145,7 @@ def pages(self, endpoint, key=None): def owners(self, base_sha, files, package): if self.ownership_error: raise triage.CollectionError("Team membership unreadable") - return {"maintainer"}, ["@maintainer"] + return {"maintainer"}, ["@maintainer"], self.skipped_team class ApprovalTests(unittest.TestCase): @@ -178,7 +179,7 @@ def test_last_rule_and_nested_double_star(self): text = "/sdk/ @general\n/**/*mgmt*/ @maintainer\n/sdk/example/azure-mgmt-example/private/ @special\n" with patch.object(self.client, "load_rules", return_value=triage.parse_codeowners(text)) as load: owners = self.client.owners(OLD, [FILE], "azure-mgmt-example") - self.assertEqual(owners, ({"maintainer"}, ["@maintainer"])) + self.assertEqual(owners, ({"maintainer"}, ["@maintainer"], False)) self.client.owners(OLD, [FILE], "azure-mgmt-example") load.assert_called_once() special = dict(FILE, filename="sdk/example/azure-mgmt-example/private/client.py") @@ -218,6 +219,35 @@ def test_codeowners_file_precedence(self): self.assertIn("/contents/.github/CODEOWNERS?", cached.call_args.args[0]) cached.assert_called_once() + def test_expected_team_403_skipped_and_cached_without_losing_named_owners(self): + rules = triage.parse_codeowners("/sdk/ @Azure/azure-arm-sdk-reviewers @maintainer") + with patch.object(self.client, "load_rules", return_value=rules), patch.object( + self.client, + "pages", + side_effect=triage.CollectionError("Resource not accessible by integration (HTTP 403)"), + ) as pages: + for _ in range(2): + maintainers, owners, skipped = self.client.owners(OLD, [FILE], "azure-mgmt-example") + self.assertEqual(maintainers, {"maintainer"}) + self.assertIn("@Azure/azure-arm-sdk-reviewers", owners) + self.assertTrue(skipped) + pages.assert_called_once() + + def test_other_team_403_or_other_error_still_reported(self): + for owner, error in ( + ("@Azure/other-team", "HTTP 403"), + ("@Azure/azure-arm-sdk-reviewers", "HTTP 401"), + ("@Azure/azure-arm-sdk-reviewers", "HTTP 500"), + ): + with self.subTest(owner=owner, error=error): + client = triage.GitHub(REPO) + rules = triage.parse_codeowners(f"/sdk/ {owner} @maintainer") + with patch.object(client, "load_rules", return_value=rules), patch.object( + client, "pages", side_effect=triage.CollectionError(error) + ): + with self.assertRaises(triage.CollectionError): + client.owners(OLD, [FILE], "azure-mgmt-example") + def test_empty_override_does_not_fall_back(self): rules = triage.parse_codeowners("/sdk/ @maintainer\n/sdk/example/azure-mgmt-example/\n") with patch.object(self.client, "load_rules", return_value=rules): @@ -319,6 +349,40 @@ def test_old_workflow_marker_supported(self): class CollectionTests(unittest.TestCase): + def test_skipped_team_preserves_named_approval_changes_and_followups(self): + client = FakeGitHub() + client.skipped_team = True + client.reviews = [review(commit=OLD)] + client.comments.append(comment()) + item = triage.collect_pr(client, {"number": 42, "reviewDecision": "APPROVED"}) + triage.classify(item, client, 9, [], []) + self.assertIsNotNone(item["approval"]) + self.assertFalse(item["issues"]) + self.assertNotIn(3, item["queues"]) + self.assertIn(4, item["queues"]) + self.assertIn(5, item["queues"]) + + def test_skipped_team_uses_github_decision_without_missing_approval_claim(self): + for decision in ("APPROVED", "REVIEW_REQUIRED", "CHANGES_REQUESTED"): + with self.subTest(decision=decision): + client = FakeGitHub() + client.skipped_team = True + client.reviews = [review(login="team-only-reviewer")] + item = triage.collect_pr(client, {"number": 42, "reviewDecision": decision}) + triage.classify(item, client, 9, [], []) + self.assertFalse(item["issues"]) + if decision == "APPROVED": + self.assertNotIn(3, item["queues"]) + else: + self.assertIn(f"GitHub review decision: {decision}", item["queues"][3]) + + def test_skipped_team_without_github_decision_is_still_unverified(self): + client = FakeGitHub() + client.skipped_team = True + client.reviews = [] + item = triage.collect_pr(client, {"number": 42}) + self.assertIn("GitHub review decision unavailable", item["issues"][0]) + def test_first_beta_and_author_followups_in_all_comment_channels(self): client = FakeGitHub() client.comments.append(comment()) From 1bb6e77bf3db76d325c732e2f92fdce7089eb64b Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Tue, 27 Oct 2026 10:01:23 +0800 Subject: [PATCH 09/11] Fix management PR triage snapshot races Scan workflow runs without status filters and reject stale initial PR snapshots. Add race regressions and clarify service-team ownership after review handoff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azsdk-python-mgmt-pr-triage/SKILL.md | 5 + .../scripts/triage_prs.py | 24 ++-- .../tests/test_triage_prs.py | 106 ++++++++++++++---- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 17 ++- 4 files changed, 115 insertions(+), 37 deletions(-) diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md index 0bc29ab48bb0..884df24aa46e 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md +++ b/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md @@ -82,6 +82,11 @@ Other team errors and API failures still produce verification warnings. - Query the management review workflow, not generic CI. Use its run's explicit `pull_requests` associations, not timestamps, title guesses, or `head_sha` alone (`pull_request_target` can use a base revision). +- Fetch workflow runs with status-independent pagination and filter completed + runs locally, so transitions between active statuses do not hide a run. +- Compare the initial listing's `updatedAt` with the first PR read, then check + for changes again after collecting evidence. Reject changed snapshots rather + than combine an old review decision with newer reviews. - Active runs without a PR association make activity verification incomplete. Show **Review activity unknown** and the run links; never recommend retriggering while activity cannot be verified. diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py index 43ac87a15888..35c67d5d1e06 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py +++ b/.github/skills/azsdk-python-mgmt-pr-triage/scripts/triage_prs.py @@ -201,17 +201,15 @@ def run_pr(run, number): def active_reviews(client): runs = {} issues = [] - for status in ACTIVE_STATUSES: - try: - for run in client.pages( - f"repos/{client.repo}/actions/workflows/{WORKFLOW}/runs?status={status}&per_page=100", - "workflow_runs", - ): - # A run can finish between status-filtered requests. - if run["status"] != "completed": - runs[run["id"]] = run - except CollectionError as error: - issues.append(f"Review activity unknown: {error}") + try: + for run in client.pages( + f"repos/{client.repo}/actions/workflows/{WORKFLOW}/runs?per_page=100", + "workflow_runs", + ): + if run["status"] != "completed": + runs[run["id"]] = run + except CollectionError as error: + issues.append(f"Review activity unknown: {error}") for run in runs.values(): if not run.get("pull_requests"): issues.append(f"Review activity unknown: active run has no PR association: {run['html_url']}") @@ -252,6 +250,8 @@ def collect_pr(client, summary): pr = client.get(endpoint) if pr["state"] != "open" or pr["draft"]: return None + if summary.get("updatedAt") != pr["updated_at"]: + raise CollectionError(f"PR #{number} changed since initial listing; rerun before acting.") match = PACKAGE.match(pr["title"]) if not match: return None @@ -423,7 +423,7 @@ def main(argv=None): "--limit", str(args.limit), "--json", - "number,title,isDraft,reviewDecision", + "number,title,isDraft,reviewDecision,updatedAt", ) if len(prs) >= args.limit: issues.append(f"PR listing reached --limit {args.limit}; increase it for a complete scan.") diff --git a/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py index 82afbda0b82e..f6a932dda5fb 100644 --- a/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py +++ b/.github/skills/azsdk-python-mgmt-pr-triage/tests/test_triage_prs.py @@ -33,6 +33,7 @@ "user": {"login": "author"}, "labels": [], } +SUMMARY = {"number": 42, "title": PR["title"], "isDraft": False, "updatedAt": WHEN} FILE = { "filename": "sdk/example/azure-mgmt-example/azure/mgmt/example/_version.py", "status": "modified", @@ -133,7 +134,7 @@ def pages(self, endpoint, key=None): if "/actions/workflows/" in endpoint: if self.activity_error: raise triage.CollectionError("HTTP 403") - return [deepcopy(item) for item in self.workflow_runs if f"status={item['status']}" in endpoint] + return deepcopy(self.workflow_runs) if "/reviews?" in endpoint: return deepcopy(self.reviews) if "/issues/" in endpoint: @@ -300,13 +301,48 @@ def test_label_presence_only_changes_trigger_instruction(self): triage.classify(item, FakeGitHub(), 9, [], []) self.assertIn("Remove and reapply" if labeled else "Add", item["queues"][1]) - def test_status_requests_paginate_once_each(self): + def test_status_independent_request_filters_completed_runs(self): client = FakeGitHub() - client.workflow_runs = [run("in_progress")] - runs, issues = triage.active_reviews(client) + client.workflow_runs = [run("in_progress"), dict(run(), id=101)] + with patch.object(client, "pages", wraps=client.pages) as pages: + runs, issues = triage.active_reviews(client) + pages.assert_called_once_with( + f"repos/{REPO}/actions/workflows/{triage.WORKFLOW}/runs?per_page=100", "workflow_runs" + ) self.assertEqual(len(runs), 1) self.assertFalse(issues) + def test_waiting_to_in_progress_transition_cannot_hide_active_run(self): + client = FakeGitHub() + active = run("waiting") + + def transition(endpoint, key=None): + if "?status=waiting&" in endpoint or "?status=" not in endpoint: + active["status"] = "in_progress" + if "?status=" not in endpoint or f"status={active['status']}&" in endpoint: + return [deepcopy(active)] + return [] + + with patch.object(client, "pages", side_effect=transition): + runs, issues = triage.active_reviews(client) + item = row() + triage.classify(item, client, 9, runs, issues) + self.assertFalse(issues) + self.assertEqual(runs[0]["status"], "in_progress") + self.assertIn(2, item["queues"]) + self.assertNotIn(1, item["queues"]) + + def test_active_run_on_later_page_is_found(self): + client = triage.GitHub(REPO) + pages = [ + {"total_count": 2, "workflow_runs": [run()]}, + {"total_count": 2, "workflow_runs": [run("queued")]}, + ] + with patch.object(triage, "gh", return_value=pages): + runs, issues = triage.active_reviews(client) + self.assertFalse(issues) + self.assertEqual([item["status"] for item in runs], ["queued"]) + class FreshnessTests(unittest.TestCase): def test_current_comment_with_different_run_head_sha(self): @@ -354,7 +390,7 @@ def test_skipped_team_preserves_named_approval_changes_and_followups(self): client.skipped_team = True client.reviews = [review(commit=OLD)] client.comments.append(comment()) - item = triage.collect_pr(client, {"number": 42, "reviewDecision": "APPROVED"}) + item = triage.collect_pr(client, dict(SUMMARY, reviewDecision="APPROVED")) triage.classify(item, client, 9, [], []) self.assertIsNotNone(item["approval"]) self.assertFalse(item["issues"]) @@ -368,7 +404,7 @@ def test_skipped_team_uses_github_decision_without_missing_approval_claim(self): client = FakeGitHub() client.skipped_team = True client.reviews = [review(login="team-only-reviewer")] - item = triage.collect_pr(client, {"number": 42, "reviewDecision": decision}) + item = triage.collect_pr(client, dict(SUMMARY, reviewDecision=decision)) triage.classify(item, client, 9, [], []) self.assertFalse(item["issues"]) if decision == "APPROVED": @@ -380,7 +416,7 @@ def test_skipped_team_without_github_decision_is_still_unverified(self): client = FakeGitHub() client.skipped_team = True client.reviews = [] - item = triage.collect_pr(client, {"number": 42}) + item = triage.collect_pr(client, SUMMARY) self.assertIn("GitHub review decision unavailable", item["issues"][0]) def test_first_beta_and_author_followups_in_all_comment_channels(self): @@ -390,7 +426,7 @@ def test_first_beta_and_author_followups_in_all_comment_channels(self): author_review = review("COMMENTED", "author", LATER, 4) author_review["body"] = "Ready now?" client.reviews.append(author_review) - item = triage.collect_pr(client, {"number": 42, "reviewDecision": "APPROVED"}) + item = triage.collect_pr(client, dict(SUMMARY, reviewDecision="APPROVED")) self.assertEqual(item["version"], "1.0.0b1") self.assertEqual(len(item["followups"]), 3) triage.classify(item, client, 9, [], []) @@ -400,7 +436,7 @@ def test_first_beta_and_author_followups_in_all_comment_channels(self): def test_comments_before_approval_or_by_other_people_not_followups(self): client = FakeGitHub() client.comments += [comment(when=WHEN), comment(login="other")] - item = triage.collect_pr(client, {"number": 42}) + item = triage.collect_pr(client, SUMMARY) self.assertFalse(item["followups"]) def test_old_commit_timestamps_cannot_hide_changed_head(self): @@ -417,7 +453,7 @@ def test_missing_commit_is_verification_warning(self): def test_ownership_failure_is_not_missing_approval(self): client = FakeGitHub() client.ownership_error = True - item = triage.collect_pr(client, {"number": 42}) + item = triage.collect_pr(client, SUMMARY) triage.classify(item, client, 9, [], []) self.assertNotIn(3, item["queues"]) self.assertIn("Approval verification incomplete", item["issues"][0]) @@ -433,19 +469,37 @@ def test_changed_pr_requires_rescan(self): client.last_details = deepcopy(PR) client.last_details["head"]["sha"] = OLD with self.assertRaises(triage.CollectionError): - triage.collect_pr(client, {"number": 42}) + triage.collect_pr(client, SUMMARY) + + def test_changes_requested_since_listing_rejects_stale_approval(self): + client = FakeGitHub() + client.details["updated_at"] = LATER + client.reviews = [review(), review("CHANGES_REQUESTED", "second", LATER, 2)] + with self.assertRaisesRegex(triage.CollectionError, "changed since initial listing"): + triage.collect_pr(client, dict(SUMMARY, reviewDecision="APPROVED")) + self.assertEqual(client.get_count, 1) + + def test_missing_listing_timestamp_cannot_bypass_snapshot_guard(self): + with self.assertRaisesRegex(triage.CollectionError, "changed since initial listing"): + triage.collect_pr(FakeGitHub(), {"number": 42, "reviewDecision": "APPROVED"}) + + def test_timestamp_change_during_collection_rejects_snapshot(self): + client = FakeGitHub() + client.last_details = dict(PR, updated_at=LATER) + with self.assertRaisesRegex(triage.CollectionError, "changed during collection"): + triage.collect_pr(client, SUMMARY) def test_newly_closed_or_draft_pr_skipped(self): for changes in ({"state": "closed"}, {"draft": True}): client = FakeGitHub() client.details.update(changes) - self.assertIsNone(triage.collect_pr(client, {"number": 42})) + self.assertIsNone(triage.collect_pr(client, SUMMARY)) def test_truncated_files_not_treated_as_complete(self): client = FakeGitHub() client.details["changed_files"] = 3001 with self.assertRaises(triage.CollectionError): - triage.collect_pr(client, {"number": 42}) + triage.collect_pr(client, SUMMARY) def test_version_changelog_fallback(self): file = dict(FILE, filename="sdk/example/azure-mgmt-example/CHANGELOG.md", patch="+## 2.0.0 (Unreleased)") @@ -480,7 +534,7 @@ def test_paginated_lists_and_incomplete_objects(self): client.pages("endpoint", "workflow_runs") def test_cli_default_includes_first_beta(self): - listing = [{"number": 42, "title": PR["title"], "isDraft": False, "reviewDecision": "APPROVED"}] + listing = [dict(SUMMARY, reviewDecision="APPROVED")] with patch.object(triage, "gh", return_value=listing), patch.object( triage, "GitHub", return_value=FakeGitHub() ), patch("sys.stdout", new_callable=io.StringIO) as output: @@ -488,8 +542,20 @@ def test_cli_default_includes_first_beta(self): self.assertIn("Scanned 1", output.getvalue()) self.assertIn("No PRs need action", output.getvalue()) + def test_cli_stale_listing_never_claims_no_action(self): + listing = [dict(SUMMARY, reviewDecision="APPROVED")] + client = FakeGitHub() + client.details["updated_at"] = LATER + client.reviews = [review(), review("CHANGES_REQUESTED", "second", LATER, 2)] + with patch.object(triage, "gh", return_value=listing), patch.object( + triage, "GitHub", return_value=client + ), patch("sys.stdout", new_callable=io.StringIO) as output: + self.assertEqual(triage.main([]), 2) + self.assertIn("changed since initial listing", output.getvalue()) + self.assertNotIn("No PRs need action", output.getvalue()) + def test_cli_limit_marks_partial(self): - listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + listing = [dict(SUMMARY)] with patch.object(triage, "gh", return_value=listing), patch.object( triage, "GitHub", return_value=FakeGitHub() ), patch("sys.stdout", new_callable=io.StringIO) as output: @@ -499,9 +565,9 @@ def test_cli_limit_marks_partial(self): def test_cli_exact_package_filter_and_drafts(self): listing = [ - {"number": 42, "title": PR["title"], "isDraft": False}, - {"number": 43, "title": "[AutoPR azure-mgmt-other]-generated", "isDraft": False}, - {"number": 44, "title": PR["title"], "isDraft": True}, + dict(SUMMARY), + dict(SUMMARY, number=43, title="[AutoPR azure-mgmt-other]-generated"), + dict(SUMMARY, number=44, isDraft=True), ] client = FakeGitHub() with patch.object(triage, "gh", return_value=listing), patch.object( @@ -511,7 +577,7 @@ def test_cli_exact_package_filter_and_drafts(self): self.assertIn("Scanned 1", output.getvalue()) def test_cli_unknown_version_cannot_silently_match_filter(self): - listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + listing = [dict(SUMMARY)] client = FakeGitHub() client.files[0]["patch"] = "" with patch.object(triage, "gh", return_value=listing), patch.object( @@ -521,7 +587,7 @@ def test_cli_unknown_version_cannot_silently_match_filter(self): self.assertIn("cannot apply --version", output.getvalue()) def test_cli_collection_failure_reports_partial(self): - listing = [{"number": 42, "title": PR["title"], "isDraft": False}] + listing = [dict(SUMMARY)] with patch.object(triage, "gh", return_value=listing), patch.object( triage, "collect_pr", side_effect=triage.CollectionError("HTTP 403") ), patch("sys.stdout", new_callable=io.StringIO) as output: diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index 8356c960942f..bc7e77942747 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -3,7 +3,7 @@ This runbook is for SDK maintainers covering the Go/JavaScript/Python release rotation who are unfamiliar with Python SDK releases. It explains how to find a release PR, use the bot's review, decide the next action, and follow the release. -Follow Steps 1-4 in order for each PR. +Follow Steps 1-3 in order for each PR; Step 4 explains the handoff. You do not need a local Python environment or detailed knowledge of generated Python code for routine cases. The reviewer on duty is the directly responsible individual (**DRI**). Ask the @@ -98,7 +98,12 @@ event alone does not prove approval is current. Read new PR-author comments even when approval remains valid. Respond to the request or route it to the Python owner; do not add a duplicate approval. -## Step 4. Hand off merge and follow the release +## Step 4. Hand off merge and release follow-up + +After approval and handoff, the service team owns merge and release follow-up. +The guidance below explains what happens next; it is not an additional mandatory +rotator step. Rotators assist when the service team requests help or a release +problem is escalated to them. Tell the service team when review is complete and point to its release plan's next action. The service team normally merges; an authorized DRI may help only @@ -112,15 +117,17 @@ uses internal post-merge CI on `main` to select changed, declared package artifa This automatic path does **not** wait for the manual `package-publish` approval gate. Do not merge early assuming another approval will stop publication. -After merge, locate `python - ` in the +For post-merge follow-up, the service team (or a rotator helping with an +escalation) locates `python - ` in the [internal Python pipelines](https://dev.azure.com/azure-sdk/internal/_build?view=folders) -and find the run for the merged commit. Check `AutoReleasePrepare` and the target +and finds the run for the merged commit. Check `AutoReleasePrepare` and the target package's `Release: ` stage. Confirm the exact version is available on [PyPI](https://pypi.org/) and check the linked release plan's status. Release-plan completion can fail separately from publication; do not republish an existing version to repair tracking. -If publishing is absent, skipped, failed, or unclear, investigate the release run. +If publishing is absent, skipped, failed, or unclear, the service team +investigates the release run and requests help when needed. For release pipeline failures, seek engineering team support in the [Engineering System Teams channel](https://teams.microsoft.com/l/channel/19%3A59dbfadafb5e41c4890e2cd3d74cc7ba%40thread.skype/Engineering%20System%20%F0%9F%9B%A0%EF%B8%8F?groupId=3e17dcb0-4257-4a30-b843-77f47f1d4121&tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47). Include the PR, package/version, merged commit, pipeline link, failed/skipped stage, From f6ec720e209652d2ba25129cd2d3042b548019f8 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Tue, 27 Oct 2026 11:04:48 +0800 Subject: [PATCH 10/11] Use absolute skill link in management release runbook Comply with Verify Links guidance for documentation outside relative-link exemptions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index bc7e77942747..cf76fe5c68ec 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -26,7 +26,7 @@ Rotation assignment does not grant repository, CODEOWNER, or release permissions For a quick grouped list, ask Copilot: **"Use azsdk-python-mgmt-pr-triage to show management SDK PRs needing attention."** The read-only -[PR triage skill](../../../.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md) +[PR triage skill](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/skills/azsdk-python-mgmt-pr-triage/SKILL.md) lists automated reviews to trigger, active reviews, missing human approvals, changes after approval, and author follow-ups. Its instructions also include a standalone command for clients without skill support. Check any verification From cef3d34efd30a991fcbadda0da1a2b262606a836 Mon Sep 17 00:00:00 2001 From: Yuchao Yan Date: Tue, 27 Oct 2026 14:49:37 +0800 Subject: [PATCH 11/11] docs: clarify management release rotation guidance Clarify reviewer roles, active workflow checks, approval comparisons, and explicit manual-release intent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8560e756-c30b-4ed8-b15e-f1b8ddeec83e --- doc/dev/mgmt/mgmt-release-rotation-runbook.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/dev/mgmt/mgmt-release-rotation-runbook.md b/doc/dev/mgmt/mgmt-release-rotation-runbook.md index cf76fe5c68ec..618696512876 100644 --- a/doc/dev/mgmt/mgmt-release-rotation-runbook.md +++ b/doc/dev/mgmt/mgmt-release-rotation-runbook.md @@ -6,8 +6,10 @@ release PR, use the bot's review, decide the next action, and follow the release Follow Steps 1-3 in order for each PR; Step 4 explains the handoff. You do not need a local Python environment or detailed knowledge of generated Python code for routine cases. -The reviewer on duty is the directly responsible individual (**DRI**). Ask the -Python owner to review handwritten SDK or code-generation tool changes. +The reviewer on duty is the directly responsible individual (**DRI**) for routine +review and handoff. The DRI routes Python-specific decisions, including handwritten +SDK or code-generation tool changes, to the Python owner. The DRI and Python owner +can be the same person. Python management SDKs manage Azure resources and use package names such as `azure-mgmt-network`. An **AutoPR** contains automatically generated SDK changes. @@ -60,8 +62,9 @@ For routine review, focus on the bot comment and current CI: 1. Read the latest **Management SDK PR review** bot comment. If it is missing or outdated, add `mgmt-review-needed`. This label triggers the [review workflow](https://github.com/Azure/azure-sdk-for-python/blob/main/.github/workflows/mgmt-sdk-pr-review.md). - If the label is already present, remove and reapply it only when no review is - active. Wait for the updated comment; ask the Python owner if none appears. + If the label is already present, remove and reapply it only when no management + SDK review workflow run is active. Wait for the updated comment; ask the Python + owner if none appears. 2. Follow the comment's findings and next actions. Ask the service team to resolve reported source/configuration issues. Send incomplete or unverified results, unclear breaking changes, and Python-specific decisions to the Python owner. @@ -88,7 +91,8 @@ readiness, current required CI passes, review findings have justified dispositions, and no unresolved review, evidence gap, or hold remains. The automated comment is advisory; it never grants human approval. -After new commits, inspect the changes since approval and request an updated bot +After new commits, inspect the changes since the latest applicable approval, +using that approval's commit as the comparison baseline. Request an updated bot result when the previous result is outdated. Wait for current CI and resolve any new findings. Ask the Python owner about API-version changes or unclear generated changes. Re-approve only after reviewing the new changes and only if required by @@ -136,7 +140,10 @@ A missing `auto-release` label or artifact mapping can leave no automatic releas Adding the label after the run is not a reliable recovery. Check for an active run or an already published version before any retry. -For an intentional manual release, let the Python owner coordinate the +The service team must confirm intentional manual-release plans in the PR or +release plan. A missing `auto-release` label does not prove manual-release intent; +the bot cannot infer that intent from the label's absence. For a confirmed manual +release, let the Python owner coordinate the [unified release process](https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/release.md#releasing-through-unified-pipelines). Only authorized release approvers validate and approve the intended package's artifacts. Do not queue a second release by default, approve unrelated packages,