fix(ci): compare registry against the PR merge result - #9595
Conversation
The extension registry check compared the latest base registry against the PR branch's own snapshot, so a release merged by a concurrent PR looked like a removal and wrongly demanded core-team review. Resolve the comparison refs as a pair from GitHub's merge preview (`refs/pull/<n>/merge`): its first parent is the exact base the preview was built from, and the merge commit is the state the registry would have once the PR lands. Verify the preview's second parent matches the head being evaluated, retry briefly while GitHub recomputes it, and fail closed with recovery guidance otherwise. Fixes #9593
|
Azure Pipelines: 21 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.
Pull request overview
Fixes false extension-registry failures caused by concurrent registry merges by comparing GitHub’s synthetic merge result with its matching base.
Changes:
- Resolves and validates merge-preview refs with retries.
- Reads both registry snapshots from the base repository.
- Adds regression, stale-preview, retry, and removal tests.
Show a summary per file
| File | Description |
|---|---|
.github/scripts/src/ext-registry-check.js |
Compares matched merge-preview states. |
.github/scripts/test/ext-registry-check.test.js |
Covers merge-preview behavior and regressions. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
|
Azure Pipelines: 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
A pull request whose merge preview can't be resolved is almost always conflicting, which is the contributor's to fix, but the failure surfaced as "Internal failure in script" and read like the check itself was broken. Raise a distinct error for an unavailable or permanently stale merge preview and report its message directly, leaving the internal-failure wrapper for genuine script bugs.
There was a problem hiding this comment.
Review details
Suppressed comments (1)
.github/scripts/src/ext-registry-check.js:382
- [azd-code-reviewer] This catch treats every
getCommitfailure—including permission, rate-limit, and GitHub 5xx responses—as an unavailable merge preview. After three retries, contributors are incorrectly told to resolve merge conflicts even when the workflow token or GitHub API is the actual problem. Only classify the expected not-found response (plus the explicit stale/malformed cases above) asMergePreviewUnavailableError; rethrow unexpected API failures sorunreports an internal failure, and cover a non-404 response in the tests.
} catch (err) {
lastError = err instanceof Error ? err : new Error(String(err));
}
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Fixes #9593
Summary
This PR makes the extension registry approval check evaluate the registry as it would exist after the pull request merges, instead of comparing the latest base registry against whatever snapshot happens to be on the PR branch. Concurrent registry PRs no longer produce a false "published release was removed" failure, while genuine removals and modifications still require core-team review.
Issue
The check loaded the base registry at the current base branch tip, then loaded the PR's registry from the head branch and diffed the two whole files. Those two commits are unrelated whenever the base branch moves after the PR branch is created.
mainwas required purely to clear a check that should never have failed. This was hit in [azure.ai.agents] Registry update for 1.0.0-beta.10 #9592 after [azure.ai.projects] Registry update for 1.0.0-beta.6 #9590 merged.Fix
The comparison now uses GitHub's synthetic merge commit (
refs/pull/<number>/merge) and takes both sides of the diff from it as a matched pair.pull_request_targetworkflow.GitHub recomputes the merge ref asynchronously, so it can briefly be missing or point at an older head right after a push. The check retries a few times, then fails closed with guidance instead of an opaque internal error. A pull request whose preview cannot be produced is unmergeable anyway, so this matches what GitHub already reports on the PR.
Testing
Unit coverage was added for the concurrent-merge regression, genuine published-release removal, a permanently stale merge preview, a preview that refreshes on retry, and a preview that is never available, with fake timers so the retry paths incur no real delay. Existing live scenarios against historical pull requests now supply explicit comparison refs, since the merge ref disappears once a PR closes.
The end-to-end behaviour was also validated in a fork by recreating this issue's exact shape - branch a PR before a concurrent registry PR merges, then open it afterwards. The old check reproduced the reported immutable-release failure, the new check passed on the identical scenario, a genuine release removal still required core review, and a conflicting PR failed closed with the recovery message.