From 2a2de5be6362a5fb1fd039821591a6da359067c7 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:06:40 -0400 Subject: [PATCH 1/3] ci: remove Mergify, restore pr-monitor as the merge gate Merge gating returns to pr-monitor's CI Gate check. Mergify's check-run condition vocabulary cannot express "everything expected has finished": `#check-failure = 0` alone is vacuously green before and during CI (#943), adding `#check-pending = 0` deadlocks on Mergify's own in-progress check (#832, confirmed by canary #946), and named anchors died with path filtering (#834). pr-monitor polls workflow runs instead, so path-filtered workflows need no enumeration, in-flight runs hold the gate, and startup_failure is visible. The restored workflow differs from the pre-#830 one: minimum-checks: 0 (zero-check PRs are legitimate post-#834 and the old floor of 1 burned the timeout as a false red), pre-sleep: 30 (sole dispatch-race guard once the floor is 0; observed fan-out is ~2s), timeout: 20 (full fan-out completes in ~3 min, so a timeout means a hang), and the deprecated job-name input is dropped in favor of the action's run-ID self-exclusion. Requires maintainer follow-up outside the repo: require CI Gate in branch protection instead of Mergify Merge Protections, enable native auto-merge, uninstall the Mergify app. Fixes #948 --- .github/workflows/pr-monitor.yml | 30 ++++++++++++++++++++++++++++++ .mergify.yml | 31 ------------------------------- AGENTS.md | 20 +++++++------------- 3 files changed, 37 insertions(+), 44 deletions(-) create mode 100644 .github/workflows/pr-monitor.yml delete mode 100644 .mergify.yml diff --git a/.github/workflows/pr-monitor.yml b/.github/workflows/pr-monitor.yml new file mode 100644 index 000000000..7dcbd42c1 --- /dev/null +++ b/.github/workflows/pr-monitor.yml @@ -0,0 +1,30 @@ +name: PR Monitor + +on: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + actions: read + contents: read + +jobs: + monitor: + name: 'CI Gate' + runs-on: ubuntu-latest + steps: + - uses: thekevinscott/pr-monitor@v1 + with: + # Path filtering (#834) means a PR can legitimately trigger zero + # checks; zero runs after pre-sleep is a pass, not a wait (#943). + minimum-checks: 0 + # With minimum-checks at 0, pre-sleep is the only guard against + # polling before other workflows register. Observed dispatch fan-out + # is ~2s across all nine workflows (#946); 30s is ~15x margin. + pre-sleep: 30 + # Full fan-out completes in under 3 minutes (#946); a timeout at 20 + # is a hung job or a real slowdown, not flake. + timeout: 20 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..fc22123fc 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, suspect a hung job before a slow one: full fan-out completes in ~3 minutes, so at `timeout: 20` a genuine hang is far likelier than load. 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): From dfb356159fcce10eccfb45c773054126ed09eaac Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:06:40 -0400 Subject: [PATCH 2/3] ci: raise the gate timeout to double the longest observed run The accessible Actions history (the API serves the most recent 1,000 PR runs) tops out at Release Precheck taking 22m21s under multi-PR contention on 2026-08-12 -- past the old gate's 20-minute timeout, which is what its "regular" timeout firings actually were: CI legitimately outrunning the gate, not hangs. 45 minutes is double the observed maximum, so a firing now means a hung job. Fixes #948 --- .github/workflows/pr-monitor.yml | 8 +++++--- AGENTS.md | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-monitor.yml b/.github/workflows/pr-monitor.yml index 7dcbd42c1..724a30f10 100644 --- a/.github/workflows/pr-monitor.yml +++ b/.github/workflows/pr-monitor.yml @@ -25,6 +25,8 @@ jobs: # polling before other workflows register. Observed dispatch fan-out # is ~2s across all nine workflows (#946); 30s is ~15x margin. pre-sleep: 30 - # Full fan-out completes in under 3 minutes (#946); a timeout at 20 - # is a hung job or a real slowdown, not flake. - timeout: 20 + # Double the longest run observed in the accessible Actions history + # (Release Precheck, 22m21s, under multi-PR contention on 2026-08-12 + # — which also outran the old gate's 20-minute timeout). Post-#834 + # fan-out completes in ~3 minutes, so a firing means a hung job. + timeout: 45 diff --git a/AGENTS.md b/AGENTS.md index fc22123fc..e407dbaec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -268,7 +268,7 @@ Merge gating is via **pr-monitor** (`.github/workflows/pr-monitor.yml`, the **`C **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. -**When the gate is red, its log names the culprits** (`Non-passing runs: [...]`). When it **times out** instead, suspect a hung job before a slow one: full fan-out completes in ~3 minutes, so at `timeout: 20` a genuine hang is far likelier than load. 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, 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. **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. From 3d248a0fa63b80dcbbaba5a4c39ae80f7782a5f1 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:34:18 -0400 Subject: [PATCH 3/3] ci: drop inert pr-monitor inputs, use a real job timeout --- .github/workflows/pr-monitor.yml | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pr-monitor.yml b/.github/workflows/pr-monitor.yml index 724a30f10..bfead41fc 100644 --- a/.github/workflows/pr-monitor.yml +++ b/.github/workflows/pr-monitor.yml @@ -10,23 +10,15 @@ concurrency: 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 - with: - # Path filtering (#834) means a PR can legitimately trigger zero - # checks; zero runs after pre-sleep is a pass, not a wait (#943). - minimum-checks: 0 - # With minimum-checks at 0, pre-sleep is the only guard against - # polling before other workflows register. Observed dispatch fan-out - # is ~2s across all nine workflows (#946); 30s is ~15x margin. - pre-sleep: 30 - # Double the longest run observed in the accessible Actions history - # (Release Precheck, 22m21s, under multi-PR contention on 2026-08-12 - # — which also outran the old gate's 20-minute timeout). Post-#834 - # fan-out completes in ~3 minutes, so a firing means a hung job. - timeout: 45