From 578d18e258b1cb77e1f6478e30bd9a5b0661c42d Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 8 Aug 2026 17:01:01 +0000 Subject: [PATCH 1/2] ci: operationalize GOVERNANCE.md merge-queue hygiene and automerge policy Issue #58: the GOVERNANCE.md policy for stale-PR hygiene and automerge-eligible classes existed but nothing executed it, so the open-PR queue kept growing past the single-digit target. Adds two workflows: - pr-queue-hygiene.yml: scheduled job that flags (label + comment) any open PR conflicting with the base branch for more than 48h, per the Merge-queue hygiene section. It does not close PRs automatically -- deciding whether a conflicting PR is superseded needs human or author-agent judgment -- but it surfaces stale PRs without waiting for a manual sweep. - automerge-eligible.yml: only fires when a maintainer applies the automerge label to a PR they've reviewed (the explicit delegation GOVERNANCE.md requires -- agents never merge their own PRs). It then double-checks the PR is a dependabot bump or automated-labeled generated-data refresh before requesting GitHub's native auto-merge, which still waits on required status checks/reviews from branch protection. GOVERNANCE.md is updated with an Operational status note under each relevant section pointing at these workflows, plus the one-time repo-admin setup needed (enable auto-merge, create the automerge label). This does not itself close #30 or merge #40 -- those require maintainer action outside a fork PR's reach -- but gives the maintainer tooling to execute the queue-hygiene and automerge steps in the issue's proposed next step going forward. Fixes #58 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/automerge-eligible.yml | 67 +++++++++++++++++++++++ .github/workflows/pr-queue-hygiene.yml | 69 ++++++++++++++++++++++++ GOVERNANCE.md | 17 ++++++ 3 files changed, 153 insertions(+) create mode 100644 .github/workflows/automerge-eligible.yml create mode 100644 .github/workflows/pr-queue-hygiene.yml diff --git a/.github/workflows/automerge-eligible.yml b/.github/workflows/automerge-eligible.yml new file mode 100644 index 00000000..b954ca13 --- /dev/null +++ b/.github/workflows/automerge-eligible.yml @@ -0,0 +1,67 @@ +name: Automerge eligible PRs + +# Operationalizes the "Automerge-eligible classes" section of GOVERNANCE.md +# (see issue #58): dependency bumps (dependabot) and generated-data +# refreshes (metrics/architectures/community people, tagged `automated` by +# their producing workflows) are candidates for automerge with green CI. +# +# This does not auto-enable merging on its own. GOVERNANCE.md is explicit +# that "Agents never merge their own PRs. A human maintainer merges, or an +# explicitly delegated automerge path does." So this workflow only fires +# when a maintainer applies the `automerge` label to a PR they've reviewed +# and judged eligible — the label is the explicit delegation. It then +# double-checks the PR is actually in an eligible class before requesting +# GitHub's native auto-merge, which still waits for required status checks +# (and any required reviews) configured in branch protection before merging. +# +# One-time setup: a maintainer with repo admin needs to enable "Allow +# auto-merge" in repo settings, and create the `automerge` label, before +# this workflow has any effect. + +on: + pull_request_target: + types: [labeled] + +permissions: + contents: read + pull-requests: write + +jobs: + enable-automerge: + if: github.event.label.name == 'automerge' + runs-on: ubuntu-latest + steps: + - name: Verify PR is in an automerge-eligible class + id: eligibility + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + set -euo pipefail + labels=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json labels --jq '[.labels[].name] | join(",")') + + eligible=false + reason="" + if [ "$PR_AUTHOR" = "dependabot[bot]" ]; then + eligible=true + reason="dependency bump (dependabot)" + elif echo ",$labels," | grep -q ",automated,"; then + eligible=true + reason="generated-data refresh (automated label)" + fi + + if [ "$eligible" != "true" ]; then + echo "PR #$PR_NUMBER is not in a GOVERNANCE.md automerge-eligible class (dependabot bump or automated-labeled generated-data refresh). Not enabling auto-merge." >&2 + exit 1 + fi + echo "Eligible: $reason" + + - name: Enable auto-merge (squash) + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + gh pr merge --auto --squash "$PR_NUMBER" --repo "$REPO" diff --git a/.github/workflows/pr-queue-hygiene.yml b/.github/workflows/pr-queue-hygiene.yml new file mode 100644 index 00000000..506b5f84 --- /dev/null +++ b/.github/workflows/pr-queue-hygiene.yml @@ -0,0 +1,69 @@ +name: PR queue hygiene + +# Operationalizes the GOVERNANCE.md merge-queue hygiene rule (see issue #58): +# "Agent PRs that conflict with the base branch for more than 48 hours should +# be rebased by their author agent or closed as superseded." This workflow +# does not close PRs automatically — deciding whether a conflicting PR is +# superseded requires judgment a scheduled job shouldn't make. Instead it +# flags them (label + comment) so a maintainer or the author agent can act +# quickly, keeping the queue from silently rotting. + +on: + schedule: + - cron: '0 */6 * * *' + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + +jobs: + flag-stale-conflicts: + runs-on: ubuntu-latest + steps: + - name: Ensure hygiene label exists + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: | + gh label create "needs-rebase-or-close" \ + --repo "$REPO" \ + --color "d93f0b" \ + --description "Conflicting with base >48h per GOVERNANCE.md merge-queue hygiene" \ + 2>/dev/null || true + + - name: Flag PRs conflicting with base for more than 48h + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + now=$(date -u +%s) + + gh pr list --repo "$REPO" --state open \ + --json number,mergeable,updatedAt,labels \ + --jq '.[] | @base64' | while read -r row; do + pr_json=$(echo "$row" | base64 -d) + number=$(echo "$pr_json" | jq -r '.number') + mergeable=$(echo "$pr_json" | jq -r '.mergeable') + updated=$(echo "$pr_json" | jq -r '.updatedAt') + labels=$(echo "$pr_json" | jq -r '[.labels[].name] | join(",")') + + if [ "$mergeable" != "CONFLICTING" ]; then + continue + fi + + updated_epoch=$(date -u -d "$updated" +%s) + age_hours=$(( (now - updated_epoch) / 3600 )) + if [ "$age_hours" -lt 48 ]; then + continue + fi + + if echo ",$labels," | grep -q ",needs-rebase-or-close,"; then + continue + fi + + echo "Flagging stale conflicting PR #$number (last updated ${age_hours}h ago)" + gh pr edit "$number" --repo "$REPO" --add-label "needs-rebase-or-close" || true + gh pr comment "$number" --repo "$REPO" --body "This PR has been conflicting with the base branch for more than 48 hours. Per [GOVERNANCE.md](../../blob/main/GOVERNANCE.md) merge-queue hygiene: please rebase, or close this PR if it has been superseded by other merged work. Flagged automatically by the PR queue hygiene workflow." || true + done diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 0bcac52d..5d4b5644 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -48,6 +48,16 @@ the following classes are candidates for automerge with green CI: pass their `validate:*` checks. - Test-only changes that add coverage without touching production code. +**Operational status**: `.github/workflows/automerge-eligible.yml` implements +the dependency-bump and generated-data-refresh classes above. It never fires +on its own — a maintainer applies the `automerge` label to a reviewed, +eligible PR (the explicit delegation this policy requires), and the workflow +double-checks eligibility before requesting GitHub's native auto-merge, which +still waits on required status checks and reviews from branch protection. +One-time setup (repo admin): enable "Allow auto-merge" in repository settings +and create the `automerge` label. Test-only changes remain manual until +eligibility can be detected reliably (e.g. by path). + The following always require human review, regardless of CI: - Workflow and security-sensitive changes (`.github/workflows/`, install @@ -68,6 +78,13 @@ To keep the queue from rotting (see issue #58): is a signal to adjust the automerge classes above, not to lower the review bar. +**Operational status**: `.github/workflows/pr-queue-hygiene.yml` runs on a +schedule and flags (labels `needs-rebase-or-close` + comments) any open PR +that has been conflicting with the base branch for more than 48 hours. It +does not close PRs automatically — judging whether a conflicting PR is +superseded needs human or author-agent judgment — but it makes stale PRs +visible without waiting for the next manual sweep. + ## Changing this document Changes to this file are direction changes: propose them as a pull request From 0890ff9907aca72f9f63e113e76267e93c6ac5c1 Mon Sep 17 00:00:00 2001 From: mrbobbytables Date: Wed, 16 Sep 2026 18:05:58 +0000 Subject: [PATCH 2/2] Address review: correct conflict-age tracking and pagination in a testable script Extracted .github/workflows/pr-queue-hygiene.yml's inline bash into scripts/pr-queue-hygiene.mjs, fixing the two logic bugs the review caught along the way: - Paginates through every open PR (repos/{repo}/pulls with per_page=100, following pages) instead of relying on gh pr list's default --limit 30, which silently drops PRs once the queue passes one page. - updatedAt reflects *any* PR update (a new commit, a label, a comment), not how long it's actually been conflicting. This now persists a first-conflict-observed timestamp as a hidden HTML marker comment on the PR itself the first time it's seen as CONFLICTING -- the only durable per-PR storage a stateless scheduled job has available -- and only flags it once 48 hours have elapsed since *that* timestamp. Added tests/pr-queue-hygiene.test.mjs (5 tests) for the pure marker-parsing/age-calculation logic, and a pr-queue-hygiene npm script for local/manual runs. Full suite (58 tests), npm run build, and npx prettier --check all pass. NOTE: I could not push this into .github/workflows/pr-queue-hygiene.yml itself, add the contents: write permission automerge-eligible.yml needs for gh pr merge --auto, or rebase this branch onto main -- my credentials lack the GitHub 'workflow' OAuth scope, which blocks any push touching .github/workflows/*, and this fork predates main's .github/workflows/ci.yml so even a no-op rebase would introduce that file. Wiring this script in is a one-line change (run: node scripts/pr-queue-hygiene.mjs) for a maintainer with workflow-scope access, alongside adding contents: write to automerge-eligible.yml's permissions block. Addresses PR #115 review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- package.json | 1 + scripts/pr-queue-hygiene.mjs | 158 ++++++++++++++++++++++++++++++++ tests/pr-queue-hygiene.test.mjs | 50 ++++++++++ 3 files changed, 209 insertions(+) create mode 100644 scripts/pr-queue-hygiene.mjs create mode 100644 tests/pr-queue-hygiene.test.mjs diff --git a/package.json b/package.json index 93c9c3e6..3d10511d 100644 --- a/package.json +++ b/package.json @@ -51,6 +51,7 @@ "update:pkgs": "npx npm-check-updates -u", "collect:metrics": "node scripts/collect-metrics.mjs", "validate:metrics": "node scripts/validate-metrics.mjs", + "pr-queue-hygiene": "node scripts/pr-queue-hygiene.mjs", "test:unit": "node --test" }, "repository": { diff --git a/scripts/pr-queue-hygiene.mjs b/scripts/pr-queue-hygiene.mjs new file mode 100644 index 00000000..888c5364 --- /dev/null +++ b/scripts/pr-queue-hygiene.mjs @@ -0,0 +1,158 @@ +#!/usr/bin/env node +// Flags open PRs that have been conflicting with the base branch for more +// than 48 hours, per GOVERNANCE.md merge-queue hygiene (issue #58). +// +// Extracted out of .github/workflows/pr-queue-hygiene.yml's inline bash so +// the two bugs the review caught are fixed in a testable, non-shell form: +// +// 1. Pagination: `gh pr list` defaults to --limit 30, which silently drops +// PRs once the fleet queue passes 30 open PRs. This paginates through +// every open PR instead of relying on a single bounded call. +// 2. Conflict age: `updatedAt` is *any* update to the PR (a new commit, a +// label, a comment) -- it does not measure how long the PR has actually +// been conflicting with the base branch. Instead, this persists a +// first-conflict-observed marker as an HTML comment on the PR itself +// (the only durable, per-PR storage a stateless scheduled job has +// available) the first time a PR is seen as CONFLICTING, and only flags +// it once 48 hours have elapsed since *that* timestamp -- not since the +// PR's last unrelated update. +// +// Usage: node scripts/pr-queue-hygiene.mjs +// Requires `gh` authenticated with `pull-requests: write` on this repo (as +// the existing workflow already grants). Set DRY_RUN=1 to log intended +// actions without labeling/commenting. + +import { execFileSync } from 'node:child_process'; + +const REPO = process.env.REPO || process.env.GITHUB_REPOSITORY; +const DRY_RUN = process.env.DRY_RUN === '1'; +const STALE_HOURS = 48; +const LABEL = 'needs-rebase-or-close'; +export const MARKER_PREFIX = '/; + +function gh(args) { + return execFileSync('gh', args, { encoding: 'utf8' }); +} + +// Fetches every open PR, paginating past gh's default --limit 30 so a +// queue deeper than one page isn't silently truncated. +export function fetchAllOpenPRs(repo) { + const prs = []; + let page = 1; + const perPage = 100; + for (;;) { + const raw = gh([ + 'api', + `repos/${repo}/pulls?state=open&per_page=${perPage}&page=${page}`, + ]); + const batch = JSON.parse(raw); + prs.push(...batch); + if (batch.length < perPage) break; + page += 1; + } + return prs; +} + +// Extracts the first-conflict-observed timestamp (if any) from a PR's +// issue comments, by finding this script's own hidden marker comment. +export function findFirstConflictObservedAt(comments) { + for (const comment of comments) { + const match = MARKER_RE.exec(comment.body || ''); + if (match) return match[1]; + } + return null; +} + +export function hoursSince(isoString, now = Date.now()) { + return (now - Date.parse(isoString)) / (1000 * 60 * 60); +} + +function labelNames(pr) { + return (pr.labels || []).map((l) => l.name); +} + +async function main() { + if (!REPO) throw new Error('REPO or GITHUB_REPOSITORY must be set'); + + const prs = fetchAllOpenPRs(REPO); + console.log(`Fetched ${prs.length} open PR(s) from ${REPO}`); + + for (const pr of prs) { + const number = pr.number; + // The pulls list endpoint doesn't include a live mergeable field + // reliably (GitHub computes it async); re-fetch per PR for a fresh + // value, matching what the mergeability check needs. + const detail = JSON.parse( + gh([ + 'api', + `repos/${REPO}/pulls/${number}`, + '--jq', + '{mergeable: .mergeable, mergeable_state: .mergeable_state}', + ]), + ); + const isConflicting = + detail.mergeable === false || detail.mergeable_state === 'dirty'; + + const commentsRaw = gh([ + 'api', + `repos/${REPO}/issues/${number}/comments?per_page=100`, + ]); + const comments = JSON.parse(commentsRaw); + const firstObservedAt = findFirstConflictObservedAt(comments); + + if (!isConflicting) { + continue; + } + + if (!firstObservedAt) { + console.log( + `PR #${number}: newly observed as conflicting, recording marker`, + ); + if (!DRY_RUN) { + gh([ + 'pr', + 'comment', + String(number), + '--repo', + REPO, + '--body', + `${MARKER_PREFIX}${new Date().toISOString()} -->`, + ]); + } + continue; + } + + const ageHours = hoursSince(firstObservedAt); + if (ageHours < STALE_HOURS) { + continue; + } + + if (labelNames(pr).includes(LABEL)) { + continue; + } + + console.log( + `Flagging stale conflicting PR #${number} (conflicting for ${ageHours.toFixed(1)}h)`, + ); + if (!DRY_RUN) { + gh(['pr', 'edit', String(number), '--repo', REPO, '--add-label', LABEL]); + gh([ + 'pr', + 'comment', + String(number), + '--repo', + REPO, + '--body', + 'This PR has been conflicting with the base branch for more than 48 hours. Per [GOVERNANCE.md](../../blob/main/GOVERNANCE.md) merge-queue hygiene: please rebase, or close this PR if it has been superseded by other merged work. Flagged automatically by the PR queue hygiene workflow.', + ]); + } + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main().catch((err) => { + console.error(err); + process.exitCode = 1; + }); +} diff --git a/tests/pr-queue-hygiene.test.mjs b/tests/pr-queue-hygiene.test.mjs new file mode 100644 index 00000000..8356c0e6 --- /dev/null +++ b/tests/pr-queue-hygiene.test.mjs @@ -0,0 +1,50 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { + findFirstConflictObservedAt, + hoursSince, + MARKER_PREFIX, +} from '../scripts/pr-queue-hygiene.mjs'; + +test('findFirstConflictObservedAt: finds the marker comment among others', () => { + const comments = [ + { body: 'unrelated comment' }, + { body: `${MARKER_PREFIX}2026-09-01T00:00:00.000Z -->` }, + { body: 'another unrelated comment' }, + ]; + assert.equal( + findFirstConflictObservedAt(comments), + '2026-09-01T00:00:00.000Z', + ); +}); + +test('findFirstConflictObservedAt: returns null when no marker exists', () => { + const comments = [{ body: 'just a regular comment' }]; + assert.equal(findFirstConflictObservedAt(comments), null); +}); + +test('findFirstConflictObservedAt: uses the earliest marker if somehow duplicated', () => { + const comments = [ + { body: `${MARKER_PREFIX}2026-09-05T00:00:00.000Z -->` }, + { body: `${MARKER_PREFIX}2026-09-01T00:00:00.000Z -->` }, + ]; + // First marker found in comment order (oldest comment first from the API) + // is treated as authoritative -- this only matters if the marker was + // somehow posted twice, which normal operation prevents. + assert.equal( + findFirstConflictObservedAt(comments), + '2026-09-05T00:00:00.000Z', + ); +}); + +test('hoursSince: computes elapsed hours against a fixed "now"', () => { + const now = Date.parse('2026-09-03T00:00:00.000Z'); + const hours = hoursSince('2026-09-01T00:00:00.000Z', now); + assert.equal(hours, 48); +}); + +test('hoursSince: returns a value under 48 for a recent timestamp', () => { + const now = Date.now(); + const recent = new Date(now - 10 * 60 * 60 * 1000).toISOString(); + assert.ok(hoursSince(recent, now) < 48); +});