Skip to content

Make the branch-freeze required status self-healing - #14548

Open
AlesProkop wants to merge 2 commits into
mainfrom
alesprokop/branch-freeze-status-reliability
Open

Make the branch-freeze required status self-healing#14548
AlesProkop wants to merge 2 commits into
mainfrom
alesprokop/branch-freeze-status-reliability

Conversation

@AlesProkop

Copy link
Copy Markdown
Member

Context

The branch-freeze commit status is a required check, but nothing posts it for pull requests opened by our own workflows. branch-freeze-pr-status.yml reacts to pull_request_target, and GitHub suppresses workflow runs for pull request events created with the built-in GITHUB_TOKEN, so those pull requests wait forever on a status nobody writes and stay unmergeable — for example #14542, which has 10 passing checks, zero commit statuses, and mergeStateStatus: BLOCKED.

Changes Made

  • Added a workflow_run trigger to branch-freeze-refresh.yml for the six workflows that can open a pull request from Actions (Backport PR to branch, Dreaming (learning atoms curation), Flaky Test Auto-Fixer, Flaky Test Triage, Inter-branch merge workflow, Sync Microsoft.Build version in analyzer template with Version.props).
  • Added a twice-daily schedule (23 5,17 * * *) as a safety net for anything the other entry points miss.
  • Skipped workflow_run events whose conclusion is skipped, so Backport PR to branch — which is requested on every issue comment — does not trigger repository-wide sweeps.
  • Replaced the per-base-branch concurrency group with a single branch-freeze-write group plus queue: max in both branch-freeze-pr-status.yml and branch-freeze-refresh.yml. The old group put nearly every pull request in one queue, and cancel-in-progress: false only protects the running job — a newer run evicted an already-pending one, silently dropping a required status.
  • Changed set-pr-status.ps1 to accept -PullRequestNumber and resolve the pull request's current head and base via the new Get-GitHubPullRequest after acquiring the concurrency lock, instead of trusting the event payload. Queued runs have no guaranteed start order, so a stale retarget event could otherwise report a frozen branch as open.
  • Added Get-GitHubCommitStatus and Test-GitHubCommitStatusMatches to GitHubStatusChecksClient.psm1 so Set-GitHubCommitStatus skips writes that would not change the status, keeping long-lived pull requests away from the 1000-statuses-per-context API limit.
  • Made that read best-effort: a failed status read emits a ::warning:: and still posts the status, since a redundant status is harmless but a missing one blocks merging.
  • Treated a status whose creator is missing or is not github-actions[bot] as a mismatch, so a status from another integration is replaced rather than trusted — the ruleset is only satisfied by the GitHub Actions integration.
  • Added the six source workflow paths to branch-freeze-tests.yml so the workflow-name contract tests run on the pull requests that can break them.

Testing

  • Ran .github/branch-freeze/tests/run-tests.ps1112 passed, 0 failed (was 73 before this change).
  • New coverage in run-tests.ps1:
    • current frozen base wins over stale event data — asserts a stale retarget event still reports the current frozen base.
    • unreadable current status still posts the required status — asserts a failed status read does not suppress the write.
    • status of unknown origin is replaced rather than trusted — asserts a status with no creator is reposted.
    • GitHub Actions reposts a status created by another integration — asserts a foreign creator is not treated as satisfying the check.
    • matching open/frozen branch status is not reposted — asserts identical statuses are skipped.
    • single-PR workflow does not trust an event-captured base branch — asserts the workflow no longer passes github.event.pull_request.base.ref.
    • <workflow> declares the expected workflow name / these tests run when <workflow> changes — pins the six workflow_run names and their test-trigger paths.
  • Validated all four branch-freeze workflow YAML files parse with js-yaml, and all .ps1/.psm1 files parse with [System.Management.Automation.Language.Parser] (the same check CI runs).
  • Verified Get-GitHubPullRequest against the live API (dotnet/msbuild#14542) and confirmed /commits/{sha}/statuses returns creator while the combined /commits/{sha}/status endpoint does not.
  • Did not run the MSBuild build or unit tests; no product code is touched.

Notes

  • This does not back-fill already-stuck pull requests such as [Flaky Test] Quarantine 1 flaky test #14542. After merge they are fixed by the first scheduled sweep, or immediately by running Refresh branch freeze PR statuses via workflow_dispatch with a blank base_ref.
  • queue: max requires the concurrency queue support announced 2026-05-07; it cannot be combined with cancel-in-progress: true, which this change does not use.
  • The bulk sweep still stamps each pull request from the gh pr list snapshot taken at job start rather than re-resolving per pull request. A retarget mid-sweep is corrected by the retarget's own pull_request_target run, which queues on the same lock and executes afterwards.

The `branch-freeze` commit status is a required check, but nothing posted
it for pull requests opened by our own workflows. `branch-freeze-pr-status.yml`
reacts to `pull_request_target`, and GitHub suppresses workflow runs for pull
request events created with the built-in `GITHUB_TOKEN`, so those pull requests
waited forever on a status nobody wrote (for example #14542).

Add `workflow_run` triggers for the six workflows that open pull requests from
Actions, plus a twice-daily scheduled sweep as a safety net.

Fix three related correctness problems this exposed:

* All status writers now share one `branch-freeze-write` concurrency group with
  `queue: max`. The previous per-base-branch group put nearly every pull request
  in one queue, and `cancel-in-progress: false` only protects the running job:
  a newer run evicted an already-pending one, silently dropping a required
  status.
* `set-pr-status.ps1` resolves the pull request's current head and base from
  GitHub after acquiring that lock instead of trusting the event payload.
  Queued runs have no guaranteed start order, so a stale retarget event could
  otherwise report a frozen branch as open.
* `Set-GitHubCommitStatus` skips writes that would not change the status,
  keeping long-lived pull requests away from the 1000-statuses-per-context API
  limit. The read is best-effort: if it fails, the status is still posted.

Skipped `workflow_run` conclusions are ignored so that `Backport PR to branch`,
which is requested on every issue comment, does not trigger repository-wide
sweeps.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0c42f84f-0e98-4d6b-8df9-4c75335ed330
@AlesProkop
AlesProkop requested a review from a team as a code owner July 28, 2026 10:50
Copilot AI review requested due to automatic review settings July 28, 2026 10:50
@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 10:50 — with GitHub Actions Inactive
@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 10:51 — with GitHub Actions Inactive
@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 10:52 — with GitHub Actions Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the branch-freeze required commit status so it can’t get “stuck missing” (and therefore block merging) when PRs are opened by repository automation that triggers GitHub’s workflow-recursion suppression. It adds additional refresh entry points, makes the concurrency strategy truly queue-based, and ensures status writes are based on the PR’s current head/base after acquiring the global lock, while also avoiding redundant status writes to reduce the risk of hitting GitHub’s per-context status limits.

Changes:

  • Added workflow_run + twice-daily schedule triggers to proactively refresh branch-freeze statuses for PRs created by automation workflows, while ignoring workflow_run events with conclusion: skipped.
  • Reworked concurrency to a single shared branch-freeze-write group with queue: max so pending runs are not silently replaced/dropped.
  • Enhanced the status writer scripts/modules to (a) resolve PR head/base under the lock via gh pr view, and (b) skip no-op status updates by best-effort reading the current status first; expanded the local contract/unit tests accordingly.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
.github/workflows/branch-freeze-tests.yml Ensures contract tests run when any workflow name/path that workflow_run depends on changes.
.github/workflows/branch-freeze-refresh.yml Adds workflow_run + schedule triggers; switches to shared queued concurrency group; skips skipped conclusions.
.github/workflows/branch-freeze-pr-status.yml Switches to shared queued concurrency group; passes only PR number so head/base are resolved after lock acquisition.
.github/workflows/branch-freeze-command.yml Updates documentation comment to reflect shared queued lock semantics for all status writers.
.github/branch-freeze/workflows/set-pr-status.ps1 Adds -PullRequestNumber parameter set and resolves current PR head/base via GitHub API after the lock is held.
.github/branch-freeze/tests/run-tests.ps1 Adds workflow-contract tests + new behavioral tests for stale event data, read-failure fallback, creator handling, and no-op updates.
.github/branch-freeze/tests/mock-gh.ps1 Extends the gh mock to support pr view and commit-status list reads (including simulated read failures).
.github/branch-freeze/components/github/GitHubStatusChecksClient.psm1 Adds best-effort “read current status then skip no-op write” logic; treats non-actions creators as mismatches.
.github/branch-freeze/components/github/GitHubPullRequestsClient.psm1 Adds Get-GitHubPullRequest to fetch current headRefOid / baseRefName for accurate status stamping.

@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 11:05 — with GitHub Actions Inactive
@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 11:07 — with GitHub Actions Inactive

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This PR correctly addresses a real reliability gap: automation-created PRs (via GITHUB_TOKEN) never received a branch-freeze commit status because GitHub suppresses pull_request_target runs for those. The three-part fix is well-designed:

  1. Status deduplication (Get-GitHubCommitStatus + Test-GitHubCommitStatusMatches) avoids hitting the 1,000-statuses-per-context API budget on no-op writes.
  2. Stale-event-data fix: The branch-freeze-pr-status.yml workflow now passes only the PR number into the concurrency lock, then resolves the live head SHA and base branch inside the lock — eliminating the race where a queued event payload could carry stale branch data from before a retarget.
  3. workflow_run trigger: The refresh workflow now fires after every automation workflow that can create PRs, plus a twice-daily cron safety net.

Strengths

  • The test suite grows proportionally with the new behavior, including the explicit "workflow contracts" tests that fail if the YAML is later edited in a way that breaks the invariants.
  • Error handling in Set-GitHubCommitStatus is explicitly fail-open: a read failure emits a warning but always proceeds to write, so a stale or missing read can never leave a required status unanswered.
  • The github-actions[bot] creator check ensures statuses posted by other integrations are replaced rather than silently accepted.

Minor observations (no blocking issues)

See inline comments for details:

  • per_page=100 pagination assumption in Get-GitHubCommitStatus
  • workflow_run triggers on failure/cancelled runs (intentionally broad but worth documenting)
  • Global branch-freeze-write concurrency group serializes all PR updates (intentional trade-off, mitigated by deduplication)

Overall this is a solid, well-tested reliability improvement. The design decisions are justified and documented.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Expert Code Review (on open) for #14548 · sonnet46 · 164.5 AIC · ⌖ 5.35 AIC · ⊞ 4.9K

Comment thread .github/workflows/branch-freeze-refresh.yml
Comment thread .github/workflows/branch-freeze-pr-status.yml
@AlesProkop
AlesProkop temporarily deployed to copilot-pat-pool July 28, 2026 11:08 — with GitHub Actions Inactive
Two gaps in the previous commit, both instances of a rule it already
established elsewhere: a pending run must not be silently evicted, and a
read must never be able to suppress the required status write.

branch-freeze-command.yml kept an unqueued concurrency group. That group
is held for the whole run, including the refresh job, which now waits on
the repository-wide branch-freeze-write queue rather than a per-branch
one -- so the window in which a pending command can be evicted got
longer. An evicted /freeze or /unfreeze never toggles the branch and its
author is never told, so give that group queue: max too.

set-pr-status.ps1 resolved the head and base through gh pr view under
$ErrorActionPreference = 'Stop', so a failed read aborted the job and
posted nothing -- the exact outcome Set-GitHubCommitStatus goes out of
its way to avoid for its own status read, and something the event
payload path could not do before. The live read stays authoritative and
is still tried first; a failure or an incomplete response now warns and
falls back to the payload head and base supplied by the workflow. Stale
data is corrected by the next event or sweep, whereas a missing status
leaves the pull request unmergeable. With no fallback available the
script still throws rather than stamping the wrong commit.

The workflow contract test that asserted the event base ref was absent
is replaced by a tighter one: it may appear exactly once, and only on
the FALLBACK_BASE_REF line, so it cannot be trusted as the primary
source again.

Tests: 118 passed, 0 failed (was 112).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ddcc43f9-f40f-441b-8b7d-e216868aca6d
@dotnet dotnet deleted a comment from ly6093909-ops Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants