diff --git a/.github/workflows/pr-monitor.yml b/.github/workflows/pr-monitor.yml new file mode 100644 index 000000000..bfead41fc --- /dev/null +++ b/.github/workflows/pr-monitor.yml @@ -0,0 +1,24 @@ +name: PR Monitor + +on: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + actions: read + contents: read + pull-requests: read + +jobs: + monitor: + name: 'CI Gate' + runs-on: ubuntu-latest + # The action has no timeout of its own (see monitor.ts) — this is the + # backstop. If a predicted run never dispatches, the gate waits here + # until the job is killed. + timeout-minutes: 45 + steps: + - uses: thekevinscott/pr-monitor@v1 diff --git a/.mergify.yml b/.mergify.yml deleted file mode 100644 index 6665d05e5..000000000 --- a/.mergify.yml +++ /dev/null @@ -1,31 +0,0 @@ -# Mergify reads this file from the default branch only. Edits on a feature -# branch are inert until they land on main. - -# No summary comment on PRs -- the Required check run carries the same -# condition breakdown and stays current, while the comment is a stale -# point-in-time snapshot cluttering the thread. -merge_protections_settings: - post_comment: false - -merge_protections: - - name: All CI green - description: Every check on this PR finished, and none failed. - if: - - base = main - success_conditions: - # Names no checks deliberately: a path-filtered workflow that does not run - # reports nothing, so an anchor naming it would pend forever. - - "#check-failure = 0" - - "#check-pending = 0" - -pull_request_rules: - - name: Auto-merge labelled PRs once every check is green - conditions: - - base = main - - label = auto-merge - - "#approved-reviews-by >= 1" - - "#check-failure = 0" - - "#check-pending = 0" - actions: - merge: - method: merge diff --git a/AGENTS.md b/AGENTS.md index c2736fbbd..e407dbaec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -262,25 +262,19 @@ Run `cargo bench -p dirsql` after significant changes to the Rust codebase. Not ### PR Monitoring -Merge gating is via **Mergify** (`.mergify.yml`, the **`Mergify Merge Protections`** check). It is an *aggregator*, not a test: `#check-failure = 0` means nothing is red, and the anchor conditions mean every unconditionally-triggered workflow reported success, so a green `Mergify Merge Protections` means the whole PR is green. There is no named-required-checks allowlist in branch protection, so adding/renaming/removing a job in a *path-filtered* workflow needs no branch-protection coordination and can't orphan a required check — don't flag that concern. +Merge gating is via **pr-monitor** (`.github/workflows/pr-monitor.yml`, the **`CI Gate`** check — [thekevinscott/pr-monitor](https://github.com/thekevinscott/pr-monitor), `@v1` is a rolling tag tracking its `main`). It is an *aggregator*, not a test: it polls the PR head's **workflow runs** and goes green only when every other run has completed with a passing conclusion (`success` / `skipped` / `neutral` / `stale`); `failure`, `cancelled`, `timed_out`, `action_required`, and `startup_failure` all turn it red. Because it watches workflow *runs* rather than check-runs, path-filtered workflows need no enumeration (one that does not run never appears), a still-running workflow holds the gate pending, and a workflow that startup-fails before emitting any check-run still fails the gate. Branch protection requires only `CI Gate`, so adding/renaming/removing jobs or workflows needs no branch-protection or gate-config coordination — don't flag that concern. -**Mergify reads `.mergify.yml` from `main` only.** Editing it on a feature branch changes nothing until that PR merges; the config gating your PR is the one already on `main`. +**Why not check-run counting (the Mergify era, #830–#947):** a gate built on check-run counts cannot express "everything expected has finished". `#check-failure = 0` alone is vacuously true before CI dispatches and while it runs — a gate that does not gate (#943 measured it green 2m21s before the last check finished). Adding `#check-pending = 0` deadlocks: Mergify counts its own in-progress check, so the count never reaches zero (#832; confirmed deliberately by canary #946 — its check summary showed an empty waiting-checks list with the pending condition unsatisfied). Named per-workflow anchors are the remaining option and path filtering (#834) killed it: an anchor on a workflow that does not run pends forever. Workflow-run polling sidesteps all three, at the cost of one runner for the CI duration. -**The anchor conditions are the part you can break.** `#check-failure = 0` is blind to a workflow that *startup-fails*, because one that dies before emitting a check-run contributes zero check-runs — "nothing failed" is then vacuously true, as it also is before CI has started. **`#check-pending = 0` cannot be used to cover that**: Mergify counts its own in-progress check, so the count never reaches zero and the gate deadlocks (#832). `success_conditions` therefore names one anchor per **unconditionally-triggered** workflow (`Conventions`, `Release Precheck`, `Release Config Check`, `Changelog Check`), matching the caller job id this repo owns rather than inner job names owned by the `@v0` reusable workflows upstream. The anchors are what keep the gate from going green mid-run. Three consequences: +**The two race guards are `pre-sleep` and `minimum-checks`.** At time T a poller cannot distinguish "all runs finished" from "no runs dispatched yet". `pre-sleep: 30` (wall-clock anchored; setup time is absorbed) covers GitHub's dispatch fan-out, observed at ~2s across nine workflows. `minimum-checks: 0` makes zero runs after the pre-sleep a **pass**: post-#834 a PR can legitimately trigger zero checks, and a check that should have existed but doesn't is the workflow's defect, not the gate's (#943's scope ruling; workflow-config validation belongs to #862). Do not raise `minimum-checks` above 0 — with path filtering there is no floor that is always satisfied, and a floor the diff doesn't meet burns the full timeout as a false red. -- **Renaming a caller job id** in `conventions.yml` or either release workflow **requires the matching `.mergify.yml` edit in the same PR.** Miss it and the gate pends forever. -- **Never anchor a path-filtered workflow.** One that doesn't run reports no status at all, so a condition naming it could never be satisfied. -- **A path-filtered workflow still running does not hold the gate.** Only the anchored four do, so a PR can go green with e.g. `python-test` still in flight. Narrow in practice — `Release Precheck` is the long pole — but real. +**When the gate is red, its log names the culprits** (`Non-passing runs: [...]`). When it **times out** instead, it is a hung job, not load: `timeout: 45` is double the longest run in recorded history (Release Precheck, 22m21s, under multi-PR contention — which is also what killed the original gate's 20-minute timeout), and post-#834 full fan-out completes in ~3 minutes. Open the named in-progress run and look at the stuck job's current *step*: a slow job advances, a hung one sits in one step while its siblings finish in seconds. Cancel it rather than letting it burn its timeout. Learned on #727, where a test awaited an event that a behavior change meant would never arrive: three jobs sat in one step for 35+ minutes and it was read as flake twice before anyone looked at the steps. Note the gate treats `cancelled` as a failure, so a cancelled hung job needs a re-run of that workflow (and the gate) once fixed. -**When the gate is red or stuck, read the Mergify check's summary** — it lists which conditions are unmet: +**Gate changes are testable on the PR that makes them.** The gate is an ordinary workflow, so a PR editing `pr-monitor.yml` is gated by its own edited copy — unlike Mergify, which read config from `main` only and forced land-and-canary cycles to test anything. -- a named check under `#check-failure` → a **real** red; fix that workflow. -- an unmet **anchor** while everything visible is green → that workflow produced no check at all. Almost always a `startup_failure` (e.g. a `conventions.yml` input the `@v0` moving tag no longer defines) or a caller job id renamed without updating `.mergify.yml`. There is no check-run to click through, so open the workflow's run list directly. -- an unmet **anchor** while its workflow is still running → possibly a hung job rather than a slow one. Open the run and look at the job's current *step*: a slow job advances, a hung one sits in one step while its siblings finish in seconds. Cancel it rather than letting it burn its timeout. Learned on #727, where a test awaited an event that a behavior change meant would never arrive: three jobs sat in one step for 35+ minutes and it was read as flake twice before anyone looked at the steps. +**Flaky jobs are back to manual handling.** Mergify's CI Insights auto-retry left with Mergify: if a job is flaky, re-run it by hand to unblock, then fix or quarantine it — the gate polls live state, so a re-run that goes green flips the gate without re-triggering anything else. (The gate's own run can be re-run too if it timed out against a since-fixed hang.) -**Flaky jobs are Mergify's job now.** CI Insights tracks per-job history (two conclusions on the same SHA = flaky) and auto-retry re-runs matching workflows, so the old "re-trigger the gate and it goes green" ritual is gone — if a job is genuinely flaky, fix or quarantine it rather than leaning on the retry. - -**Auto-merge is opt-in per PR:** label it `auto-merge` and Mergify merges once the PR is approved and every check is green. +**Auto-merge is GitHub-native:** enable auto-merge on the PR (UI or `mcp__github__enable_pr_auto_merge`) and it merges once branch protection is satisfied — `CI Gate` green plus an approving review. The old Mergify `auto-merge` label does nothing now. When monitoring PRs to get them across the finish line (shepherding to green):