Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@ README.md @danielmeppiel

# Workflow files - Lead Maintainer review required for any change to CI
# definitions, especially anything touching `pull_request_target` or secrets.
# See .github/workflows/ci-integration-pr-stub.yml for the security rationale.
.github/workflows/** @danielmeppiel
40 changes: 23 additions & 17 deletions .github/instructions/cicd.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: "CI/CD Pipeline configuration for PyInstaller binary packaging and
# CI/CD Pipeline Instructions

## Workflow Architecture (Tiered + Merge Queue)
Four workflows split by trigger and tier. PRs get fast feedback; the heavy
Five workflows split by trigger and tier. PRs get fast feedback; the heavy
integration suite runs only at merge time via GitHub Merge Queue
(microsoft/apm#770).

Comment on lines 8 to 12
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

This doc was updated, but there are other copies in the repo that still reference the deleted ci-integration-pr-stub.yml (e.g. .apm/instructions/cicd.instructions.md and custom-instructions/repo/.github/instructions/cicd.instructions.md). If those files are consumed anywhere (or checked for consistency), they should be updated/removed in the same PR or documented as generated artifacts to avoid drift.

Copilot uses AI. Check for mistakes.
Expand All @@ -24,27 +24,28 @@ integration suite runs only at merge time via GitHub Merge Queue
cross-workflow artifact plumbing across triggers.
- **Never add a `pull_request` or `pull_request_target` trigger here.**
This file holds production secrets (`GH_CLI_PAT`, `ADO_APM_PAT`).
Required-check satisfaction at PR time is handled by the inert stub
`ci-integration-pr-stub.yml` instead.
3. **`ci-integration-pr-stub.yml`** - inert PR-time stub for required checks
- Triggers on `pull_request_target` so the YAML is read from `main`
(admin-controlled) regardless of PR head contents - applies retroactively
to existing fork PRs without rebase.
- `permissions: {}`, no secrets, no checkout, four no-op `echo` jobs whose
names match the four Tier 2 required checks. Reports success in seconds.
- Concurrency group keyed on PR number cancels in-flight stub runs on
subsequent pushes.
- Activity types include `labeled/unlabeled/edited` so maintainers can
re-trigger the stub without forcing contributors to push commits.
Required-check satisfaction at PR time is handled by `merge-gate.yml`,
which aggregates all required signals into a single `gate` check.
3. **`merge-gate.yml`** - single-authority PR-time aggregator
- Triggers on `pull_request` only (single trigger - dual-trigger with
`pull_request_target` produces SUCCESS+CANCELLED check-run twins via
`cancel-in-progress` and poisons branch protection's rollup).
- One job named `gate`. Polls the Checks API for all entries in the
workflow's `EXPECTED_CHECKS` env var; aggregates pass/fail into a
single check-run.
- Branch protection requires ONLY this one check (`gate`). Adding,
renaming, or removing an underlying check is a `merge-gate.yml` edit,
never a ruleset edit. Tide / bors single-authority pattern.
- Recovery if the `pull_request` webhook is dropped: empty commit,
`gh workflow run merge-gate.yml -f pr_number=NNN`, or close+reopen.
- `.github/CODEOWNERS` requires Lead Maintainer review for any change
to `.github/workflows/**` to prevent inadvertent additions of secrets,
checkout, or PR-data interpolation to this file.
3. **`build-release.yml`** - `push` to main, tags, schedule, `workflow_dispatch`
to `.github/workflows/**`.
4. **`build-release.yml`** - `push` to main, tags, schedule, `workflow_dispatch`
- **Linux + Windows** run combined `build-and-test` (unit tests + binary build in one job).
- **macOS Intel** uses `build-and-validate-macos-intel` (root node, runs own unit tests - no dependency on `build-and-test`). Builds the binary on every push for early regression feedback; integration + release-validation phases conditional on tag/schedule/dispatch.
- **macOS ARM** uses `build-and-validate-macos-arm` (root node, tag/schedule/dispatch only - ARM runners are extremely scarce with 2-4h+ queue waits). Only requested when the binary is actually needed for a release.
- Secrets always available. Full 5-platform binary output (linux x86_64/arm64, darwin x86_64/arm64, windows x86_64).
4. **`ci-runtime.yml`** - nightly schedule, manual dispatch, path-filtered push
5. **`ci-runtime.yml`** - nightly schedule, manual dispatch, path-filtered push
- **Linux x86_64 only**. Live inference smoke tests (`apm run`) isolated from release pipeline.
- Uses `GH_MODELS_PAT` for GitHub Models API access.
- Failures do not block releases - annotated as warnings.
Expand Down Expand Up @@ -86,6 +87,11 @@ integration suite runs only at merge time via GitHub Merge Queue
- **Artifact Retention**: 30 days for debugging failed releases
- **Cross-workflow artifacts**: ci-integration.yml builds the binary inline (no cross-workflow artifact transfer); build-release.yml jobs share artifacts within the same workflow run.

## Branch Protection & Required Checks
- **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.

## Trust Model
- **PR push (any contributor, including forks)**: Runs Tier 1 only. No CI secrets exposed. PR code is checked out and tested in an unprivileged context.
- **merge_group (write access required)**: Runs Tier 1 + Tier 2. Tier 2 sees secrets. The `gh-readonly-queue/main/*` ref is created by GitHub from the PR merged into main; only users with write access can trigger this by enqueueing a PR.
Expand Down
74 changes: 0 additions & 74 deletions .github/workflows/ci-integration-pr-stub.yml

This file was deleted.

12 changes: 6 additions & 6 deletions .github/workflows/ci-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
# - Tier 2 (this workflow) runs smoke + integration + release-validation
# against the tentative merge commit that the merge queue creates.
#
# Required-check satisfaction on PR commits:
# - The four required check names produced here (Build (Linux), Smoke Test
# (Linux), Integration Tests (Linux), Release Validation (Linux)) are also
# produced by `ci-integration-pr-stub.yml` on `pull_request_target` events
# so branch protection's required-check gate is satisfied at PR time
# without burning runner minutes.
# Required-check satisfaction at PR time:
# - Branch protection requires only `gate` (from `merge-gate.yml`), which
# aggregates all PR-time signals. The four check names produced by THIS
# workflow (Build/Smoke/Integration/Release Validation - all Linux) are
# not required at PR time; they only run on the `gh-readonly-queue/main/*`
# SHA the merge queue creates, against the tentative merge commit.
# - This workflow intentionally does NOT trigger on pull_request events.
# Doing so would let a fork PR check out and run code with the secrets
# declared below, which is a supply-chain attack vector. Keep this file
Expand Down
23 changes: 11 additions & 12 deletions .github/workflows/merge-gate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@
#
# Why this file exists:
# GitHub's required-status-checks model is name-based, not workflow-based.
# Our CI is tiered: ci.yml emits 'Build & Test (Linux)' and
# ci-integration-pr-stub.yml emits four stubs that hold positions for
# merge-queue jobs in ci-integration.yml. Without this gate, branch
# protection had to require all 5 checks individually -- adding or
# renaming a check meant a ruleset edit. With this gate, branch
# protection requires only 'Merge Gate / gate' and the gate aggregates.
# Tide / bors pattern.
# Without this gate, branch protection had to require each PR-time check
# individually -- adding or renaming a check meant a ruleset edit. With
# this gate, branch protection requires only 'gate' (the check-run name
# of the job below) and the gate aggregates whatever underlying checks
# we declare in EXPECTED_CHECKS. Tide / bors single-authority pattern.
#
# Why a single trigger (not dual pull_request + pull_request_target):
# We tried dual-trigger redundancy in PR #865 to harden against rare
Expand Down Expand Up @@ -91,11 +89,12 @@ jobs:
REPO: ${{ github.repository }}
SHA: ${{ steps.sha.outputs.sha }}
# All PR-time checks the gate aggregates. Keep this in sync with
# the underlying workflows: ci.yml emits Build & Test (Linux),
# ci-integration-pr-stub.yml emits the other four.
# NOTE: 'Merge Gate / gate' itself MUST NOT appear here -- it
# would deadlock waiting for itself.
EXPECTED_CHECKS: 'Build & Test (Linux),Build (Linux),Smoke Test (Linux),Integration Tests (Linux),Release Validation (Linux)'
# the underlying workflows. Currently only ci.yml emits PR-time
# checks ('Build & Test (Linux)'); ci-integration.yml is
# merge_group-only and is NOT polled here.
# NOTE: 'gate' (this job) MUST NOT appear here -- it would
# deadlock waiting for itself.
EXPECTED_CHECKS: 'Build & Test (Linux)'
TIMEOUT_MIN: '30'
POLL_SEC: '30'
run: |
Expand Down
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### 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.

### Removed

- CI: deleted `ci-integration-pr-stub.yml`. The four stubs 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 `gate`, so the stubs are dead weight. Reduced `EXPECTED_CHECKS` in `merge-gate.yml` to just `Build & Test (Linux)`.
Comment on lines +13 to +17
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

CHANGELOG entries under [Unreleased] do not follow the repo's changelog rules: each PR should be a single concise bullet ending with a PR reference like "(#123)", but these bullets are multi-sentence and have no PR number. Also, the "Changed" bullet claims the stub workflow was "deprecated", but in this PR it is deleted (so that wording is inaccurate and should be updated or removed).

Copilot generated this review using guidance from repository custom instructions.

## [0.9.2] - 2026-04-23

### Added
Expand Down
Loading