Add management SDK breaking-change attribution - #48992
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The workflow currently has fatal Python indentation errors, redundant collection, a toolchain downgrade, and attribution scope inconsistent with its description.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Extends the management SDK reviewer with breaking-change attribution and immutable provenance collection.
Changes:
- Adds changelog and provenance collection.
- Adds focused unit tests.
- Adds an advisory attribution table to review comments.
File summaries
| File | Description |
|---|---|
.github/workflows/scripts/mgmt_sdk_review_context.py |
Collects changelog and provenance evidence. |
.github/workflows/tests/test_mgmt_sdk_review_context.py |
Tests parsing and failure handling. |
.github/workflows/mgmt-sdk-pr-review.md |
Adds collection and attribution instructions. |
.github/workflows/mgmt-sdk-pr-review.lock.yml |
Contains the generated workflow. |
Review details
Suppressed comments (2)
.github/workflows/mgmt-sdk-pr-review.md:291
- The inline Python cannot execute because these lines have inconsistent top-level indentation (
with,encoded_content, andcontentare unexpectedly indented, whilepayloadis outside thewithblock). This makes every workflow run fail before the trusted collector is launched; fix the indentation and regenerate the lock file.
with urllib.request.urlopen(request, timeout=30) as response:
payload = json.load(response)
encoded_content = re.sub(r"\s+", "", payload["content"])
content = base64.b64decode(encoded_content, validate=True)
.github/workflows/mgmt-sdk-pr-review.lock.yml:474
- The generated workflow embeds the same malformed indentation as the source bootstrap, so the executable lock file raises
IndentationErrorbefore creatingreview-context.json. After correcting the markdown source, regenerate this lock file so the deployed workflow contains valid Python.
run: "python - <<'PY'\nimport base64\nimport json\nimport os\nimport pathlib\nimport re\nimport urllib.parse\nimport urllib.request\n\nrepository = os.environ[\"GH_REPOSITORY\"]\nrevision = os.environ[\"TRUSTED_BASE_SHA\"]\nif not re.fullmatch(r\"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\", repository):\n raise SystemExit(\"Invalid repository reference\")\nif not re.fullmatch(r\"[0-9a-f]{40}\", revision):\n raise SystemExit(\"Invalid trusted base revision\")\npath = \".github/workflows/scripts/mgmt_sdk_review_context.py\"\nurl = (\n f\"https://api.github.com/repos/{repository}/contents/\"\n f\"{urllib.parse.quote(path, safe='/')}?ref={revision}\"\n)\nrequest = urllib.request.Request(\n url,\n headers={\n \"Accept\": \"application/vnd.github+json\",\n \"Authorization\": f\"Bearer {os.environ['GH_TOKEN']}\",\n \"User-Agent\": \"azure-sdk-python-mgmt-review\",\n \"X-GitHub-Api-Version\": \"2022-11-28\",\n },\n)\n with urllib.request.urlopen(request, timeout=30) as response:\n payload = json.load(response)\n encoded_content = re.sub(r\"\\s+\", \"\", payload[\"content\"])\n content = base64.b64decode(encoded_content, validate=True)\nif len(content) > 128 * 1024:\n raise SystemExit(\"Trusted collector exceeded the size limit\")\nscript = pathlib.Path(\"mgmt_sdk_review_context.py\")\nscript.write_bytes(content)\nPY\npython mgmt_sdk_review_context.py\n"
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The collector can misclassify changelog history and fail to report incomplete evidence accurately.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The lock pins an incompatible setup action, and versioned Unreleased headings select the wrong release baseline.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Malformed PR-controlled repository metadata can crash the collector before it produces a review context.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/scripts/mgmt_sdk_review_context.py:372
repository_urlcomes from PR-controlled_metadata.json. When it is a truthy non-string (for example, an object),re.fullmatchraisesTypeError, aborting the collector before it writesreview-context.json. Validate the value and record invalid provenance instead of executing the regex.
This issue also appears on line 459 of the same file.
.github/workflows/scripts/mgmt_sdk_review_context.py:463
- This second regex path also accepts the PR-controlled
repository_urlwithout checking its type. Iftsp-location.yamlis absent, the earlier conflict comparison does not run, so a truthy non-string reaches this call and crashes attribution collection. Returnunverifiedfor a non-string value before matching it.
if repository_url:
match = re.fullmatch(
r"https://github\.com/([A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+?)(?:\.git)?/?", repository_url
)
repository = match.group(1) if match else None
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
Extend the management SDK PR-review agentic workflow to explain newly added or modified changelog breaking-change entries using direct TypeSpec/API evidence, leaving entries without clear evidence to human reviewers. Emitter/toolchain attribution is out of scope.
Validation Performed
compile mgmt-sdk-pr-review --action-mode action --action-tag v0.87.1 --validate, passed.git diff --checkpassed.Known Issues and Limitations
pull_request_target.PR Source
Replacement for fork-based draft #48991, with the same implementation published from
Azure/azure-sdk-for-python:copilot/mgmt-breaking-change-attribution. The earlier PR remains open.