docs(ci): improve Branch Protection section per review feedback by danielmeppiel · Pull Request #874 · microsoft/apm

danielmeppiel · 2026-04-23T11:24:42Z

What changed

Most of the original PR #874 content was absorbed when stacked-PR #875 was merged (workflow renumbering, merge-gate.yml entry, original Branch Protection section, CHANGELOG bump all landed via #875).

This PR is now scoped down to just the reviewer-suggested wording improvements for the Branch Protection section in .github/instructions/cicd.instructions.md.

Why

The version that landed via #875 is correct but terse. The reviewer (Copilot reviewer on the original #874) suggested a 4-bullet structure with a new "How the name is derived" bullet that documents the job name: vs job-id fallback rule. This makes the diagnostic process explicit for any future contributor who hits the same "Expected - Waiting for status to be reported" symptom we hit on PR #860 today.

Also tightens the gotcha bullet: previous wording said the context must "match" the check-run name (room for ambiguity). New wording says it must BE the literal check-run name and explicitly calls out that Merge Gate / gate is a UI rendering, not a value to store.

Diff

One file, 4 insertions / 3 deletions in cicd.instructions.md.

Copilot

Pull request overview

Updates internal CI/CD documentation to reflect the post-merge-gate architecture and to capture a branch-protection ruleset configuration pitfall encountered during the v0.9.2 release process.

Changes:

Document merge-gate.yml as the single PR-time required-check aggregator and mark ci-integration-pr-stub.yml as deprecated.
Add a new "Branch Protection & Required Checks" section describing the required-check context matching behavior and the ruleset gotcha.
Add a [Unreleased] changelog entry describing the CI docs update.

Show a summary per file

File	Description
`CHANGELOG.md`	Adds an `[Unreleased]` entry capturing the CI docs clarification.
`.github/instructions/cicd.instructions.md`	Updates CI workflow architecture docs; adds branch-protection required-check guidance and gotcha.

Copilot's findings

Files reviewed: 2/2 changed files
Comments generated: 2

Copilot · 2026-04-23T11:27:49Z


+### Changed
+
+- CI docs: clarify that branch-protection ruleset must store the check-run name (`gate`), not the workflow display string (`Merge Gate / gate`); document the merge-gate aggregator in `cicd.instructions.md` and mark the legacy stub workflow as deprecated.


The new Unreleased changelog entry does not follow this repo's standard format of ending each bullet with the PR number in parentheses (e.g. "(#867)"). Please append this PR number to the end of the line so it matches the rest of the file and Keep a Changelog guidance used elsewhere in this repo.

Suggested change

- CI docs: clarify that branch-protection ruleset must store the check-run name (`gate`), not the workflow display string (`Merge Gate / gate`); document the merge-gate aggregator in `cicd.instructions.md` and mark the legacy stub workflow as deprecated.

- CI docs: clarify that branch-protection ruleset must store the check-run name (`gate`), not the workflow display string (`Merge Gate / gate`); document the merge-gate aggregator in `cicd.instructions.md` and mark the legacy stub workflow as deprecated. (#867, #868)

Copilot · 2026-04-23T11:27:49Z

+- **Single required check**: branch protection (`main-protection` ruleset id 9294522) requires only one status check: `gate` from `merge-gate.yml`. All other PR-time signals are aggregated by that workflow's poll loop.
+- **CRITICAL ruleset gotcha**: the `context` field stored in the ruleset must match the **check-run name** (the job key, e.g. `gate`), NOT the GitHub UI's display string (`Merge Gate / gate`). Storing the display string causes permanent "Expected - Waiting for status to be reported" because no check-run with that literal name ever posts. The integration_id (`15368` = github-actions) plus the check-run name is what GitHub matches.
+- **Adding a new required check**: add it to `EXPECTED_CHECKS` in `merge-gate.yml`. Do NOT touch the ruleset.


This section states the ruleset "context" must be the check-run name gate (and that Merge Gate / gate never posts), but .github/workflows/merge-gate.yml currently documents the opposite (it says branch protection requires Merge Gate / gate). Please reconcile these docs and clarify the exact string that must be configured in the ruleset, including how it relates to the UI display name vs the check-run name (job name:).

Suggested change

- **Single required check**: branch protection (`main-protection` ruleset id 9294522) requires only one status check: `gate` from `merge-gate.yml`. All other PR-time signals are aggregated by that workflow's poll loop.

- **CRITICAL ruleset gotcha**: the `context` field stored in the ruleset must match the **check-run name** (the job key, e.g. `gate`), NOT the GitHub UI's display string (`Merge Gate / gate`). Storing the display string causes permanent "Expected - Waiting for status to be reported" because no check-run with that literal name ever posts. The integration_id (`15368` = github-actions) plus the check-run name is what GitHub matches.

- **Adding a new required check**: add it to `EXPECTED_CHECKS` in `merge-gate.yml`. Do NOT touch the ruleset.

- **Single required check**: branch protection (`main-protection` ruleset id 9294522) requires exactly one status check context: `gate` from `merge-gate.yml`. All other PR-time signals are aggregated by that workflow's poll loop.

- **CRITICAL ruleset gotcha**: the ruleset `context` must be the literal check-run name `gate`. `Merge Gate / gate` is only how GitHub may render the workflow and job together in the UI; it is not the context value to store in the ruleset. If the ruleset stores `Merge Gate / gate`, GitHub waits forever with "Expected - Waiting for status to be reported" because no check-run with that literal name is posted.

- **How the name is derived**: GitHub matches the check by `integration_id` (`15368` = github-actions) plus the emitted check-run name. That emitted name comes from the job `name:` if one is set; otherwise it falls back to the job id. In the current merge gate workflow, the emitted check-run name is `gate`, so that is the exact string the ruleset must require.

- **Adding a new aggregated check**: add it to `EXPECTED_CHECKS` in `merge-gate.yml`. Do not change the ruleset unless you intentionally rename the merge gate job's emitted check-run name, in which case the ruleset `context` must be updated to the new exact name.

* docs(ci): document merge-gate architecture and ruleset context gotcha - Document merge-gate.yml as the single-authority PR-time aggregator - Mark ci-integration-pr-stub.yml as DEPRECATED (slated for deletion) - Renumber workflow list (now 6 entries, was misnumbered with two #3s) - New section: Branch Protection & Required Checks - Ruleset 'context' field MUST match check-run name ('gate'), not the UI display string ('Merge Gate / gate'). Storing the display string causes permanent 'Expected - Waiting for status to be reported' that blocked PR #860 today - Adding new required checks goes through EXPECTED_CHECKS in merge-gate.yml, not the ruleset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(ci): remove deprecated PR-time stub workflow The four PR stubs (Build/Smoke/Integration/Release Validation - Linux) were a holdover from the pre-merge-gate model where branch protection required each Tier 2 check name directly. After #867, branch protection requires only the single 'gate' check from merge-gate.yml, so the stubs are dead weight that fire on every PR for no reason. Changes: - Delete .github/workflows/ci-integration-pr-stub.yml - Reduce EXPECTED_CHECKS in merge-gate.yml to just 'Build & Test (Linux)' (the only PR-time check we still emit) - Update merge-gate.yml + ci-integration.yml header comments - Update cicd.instructions.md (drop DEPRECATED entry, renumber to 5 workflows) - Drop stale CODEOWNERS reference to the deleted file - CHANGELOG entry under [Unreleased] > Removed Stacked on #874 (docs). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- CHANGELOG: append (#874) PR number per Keep a Changelog convention - merge-gate.yml header: fix self-contradiction -- said branch protection requires 'Merge Gate / gate' (the broken display string), now correctly says 'gate' (the literal check-run name) - cicd.instructions.md: adopt reviewer's clearer wording for the Branch Protection section, including a 'How the name is derived' bullet that explains job name: vs job id fallback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

danielmeppiel · 2026-04-23T11:42:14Z

Addressed both review comments in fef49e5:

CHANGELOG: appended (#874) per Keep a Changelog convention
merge-gate.yml self-contradiction: the header comment said branch protection requires Merge Gate / gate (the broken display string). Now correctly says gate (the literal check-run name) with an inline reminder that the UI string is NOT the value to store

Also adopted the reviewer's clearer wording for the "Branch Protection & Required Checks" section, including the new "How the name is derived" bullet that documents the job name: vs job-id fallback rule.

#875 (stub removal) has been rebased on top.

The original wording was correct but terse. Reviewer (#874) suggested a clearer 4-bullet structure that adds a 'How the name is derived' bullet explaining the job 'name:' vs job-id fallback rule. This makes the diagnostic process explicit for future contributors who hit the same 'Expected - Waiting for status to be reported' symptom. Also tightens the gotcha bullet: the previous wording said the context must 'match' the check-run name, which left room for ambiguity. The new wording says it must BE the literal check-run name and explicitly explains that 'Merge Gate / gate' is a UI rendering, not a value. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address PR #878 review: 1. Sync .apm/instructions/cicd.instructions.md (canonical source per #823) with .github/instructions/cicd.instructions.md so future apm install --target copilot regenerations don't revert the build-release smoke-gating doc note (and to bring along the stub-removal changes from #875 + branch-protection refinement from #874 that had also drifted). 2. Append (#878) suffix to the new CHANGELOG entry, matching the established Keep-a-Changelog convention used by neighbouring entries. No workflow behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci(build-release): gate smoke to tag/schedule/dispatch only Push-time smoke in build-release.yml's build-and-test job (Linux x86_64, Linux arm64, Windows) duplicated the merge-time smoke gate already enforced by ci-integration.yml on the same SHA content, while burning ~15 redundant codex-binary downloads per active day and amplifying network-flake exposure. Smoke now runs only at promotion boundaries: - tags (pre-ship release gate; only validation tag-cut releases receive) - schedule (nightly drift catch for upstream openai/codex URL changes) - workflow_dispatch (manual safety net) Push-to-main retains unit tests on all build-and-test platforms for platform-regression signal; smoke coverage on Linux at merge_group time (ci-integration.yml) and on Linux x86_64 nightly (ci-runtime.yml) is unchanged. Multi-platform smoke (arm64 + Windows) shifts from per-push to per-tag, narrowing the time-to-detection window for platform-specific regressions in scripts/runtime/setup-codex.sh by hours-to-days but trading that for a meaningful reduction in network noise. The gating expression matches the existing canonical pattern used by the macOS Intel/ARM jobs and integration-tests/release-validation jobs in this same workflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci(build-release): sync .apm canonical + add PR ref to changelog Address PR #878 review: 1. Sync .apm/instructions/cicd.instructions.md (canonical source per #823) with .github/instructions/cicd.instructions.md so future apm install --target copilot regenerations don't revert the build-release smoke-gating doc note (and to bring along the stub-removal changes from #875 + branch-protection refinement from #874 that had also drifted). 2. Append (#878) suffix to the new CHANGELOG entry, matching the established Keep-a-Changelog convention used by neighbouring entries. No workflow behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 23, 2026 11:24

danielmeppiel requested a review from sergio-sisternes-epam as a code owner April 23, 2026 11:24

Copilot started reviewing on behalf of danielmeppiel April 23, 2026 11:25 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

danielmeppiel mentioned this pull request Apr 23, 2026

chore(ci): remove deprecated PR-time stub workflow #875

Merged

danielmeppiel force-pushed the docs/cicd-instructions-merge-gate branch from fef49e5 to f7ed978 Compare April 23, 2026 11:43

danielmeppiel changed the title ~~docs(ci): document merge-gate architecture and ruleset context gotcha~~ docs(ci): improve Branch Protection section per review feedback Apr 23, 2026

danielmeppiel merged commit aa26b31 into main Apr 23, 2026
12 checks passed

danielmeppiel deleted the docs/cicd-instructions-merge-gate branch April 23, 2026 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(ci): improve Branch Protection section per review feedback#874

docs(ci): improve Branch Protection section per review feedback#874
danielmeppiel merged 1 commit intomainfrom
docs/cicd-instructions-merge-gate

danielmeppiel commented Apr 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

danielmeppiel commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		### Changed

		- CI docs: clarify that branch-protection ruleset must store the check-run name (`gate`), not the workflow display string (`Merge Gate / gate`); document the merge-gate aggregator in `cicd.instructions.md` and mark the legacy stub workflow as deprecated.

-- **Single required check**: branch protection (`main-protection` ruleset id 9294522) requires only one status check: `gate` from `merge-gate.yml`. All other PR-time signals are aggregated by that workflow's poll loop.
-- **CRITICAL ruleset gotcha**: the `context` field stored in the ruleset must match the **check-run name** (the job key, e.g. `gate`), NOT the GitHub UI's display string (`Merge Gate / gate`). Storing the display string causes permanent "Expected - Waiting for status to be reported" because no check-run with that literal name ever posts. The integration_id (`15368` = github-actions) plus the check-run name is what GitHub matches.
-- **Adding a new required check**: add it to `EXPECTED_CHECKS` in `merge-gate.yml`. Do NOT touch the ruleset.
+- **Single required check**: branch protection (`main-protection` ruleset id 9294522) requires exactly one status check context: `gate` from `merge-gate.yml`. All other PR-time signals are aggregated by that workflow's poll loop.
+- **CRITICAL ruleset gotcha**: the ruleset `context` must be the literal check-run name `gate`. `Merge Gate / gate` is only how GitHub may render the workflow and job together in the UI; it is not the context value to store in the ruleset. If the ruleset stores `Merge Gate / gate`, GitHub waits forever with "Expected - Waiting for status to be reported" because no check-run with that literal name is posted.
+- **How the name is derived**: GitHub matches the check by `integration_id` (`15368` = github-actions) plus the emitted check-run name. That emitted name comes from the job `name:` if one is set; otherwise it falls back to the job id. In the current merge gate workflow, the emitted check-run name is `gate`, so that is the exact string the ruleset must require.
+- **Adding a new aggregated check**: add it to `EXPECTED_CHECKS` in `merge-gate.yml`. Do not change the ruleset unless you intentionally rename the merge gate job's emitted check-run name, in which case the ruleset `context` must be updated to the new exact name.

Conversation

danielmeppiel commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

Diff

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

danielmeppiel commented Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

danielmeppiel commented Apr 23, 2026 •

edited

Loading