From 28653080f728f12bb0da63a84f44f2be65bd00c2 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 11:47:28 -0500 Subject: [PATCH 01/17] CI: priority lanes for rebase-cascade pushes, escalate on stack promotion gh stack rebase cascades a push to every PR above the one actually changed, at the SAME priority as real new commits - a stacked PR's own runner budget gets crowded out by rebase noise from pushes that introduce no new content. bin/patch_id_hash tells the two apart via a base-independent content hash; a synchronize push whose hash matches the last one observed for this PR routes to a small shared low-priority lane instead of the per-PR high-priority one, still running the full matrix either way (a rebase can break things, and merges wait on a clean run regardless) - just possibly later. A base retarget (edited, gh stack promoting this PR to the bottom of its stack) or a draft PR marked ready escalates to an immediate high-priority run bypassing whatever lane its last push landed in, unless a full run already exists for that exact SHA. ci.yml's pull_request trigger previously had no edited/ready_for_review in its types, so neither of those could fire at all before now. claude-code-review.yml gets the equivalent skip on pure rebase pushes (nothing new to review), restricted to synchronize so it never suppresses the review on ready_for_review/opened/reopened. Reapplied onto the current ci.yml/claude-code-review.yml structure (docs-only gate, PG-matrix derivation, pg-upgrade-test/pg-tle-test draft gating) rather than the stale branch point this originally carried. --- .github/workflows/ci.yml | 213 ++++++++++++++++++++--- .github/workflows/claude-code-review.yml | 82 ++++++++- bin/patch_id_hash | 13 ++ 3 files changed, 285 insertions(+), 23 deletions(-) create mode 100755 bin/patch_id_hash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a302c94..7a45376 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,19 +51,42 @@ # in full; `test`'s matrix drops to just the newest supported PostgreSQL # major (see its own comment) instead of running full or being skipped # outright, since it's cheap per-leg and a draft author still wants signal -# on every push; every other heavy job (`pg-tle-test`, and `pg-upgrade-test` -# etc. from later phases) is skipped entirely via an added -# `&& github.event.pull_request.draft != true` on its existing `if:`. None -# of this applies to a `push` event (e.g. the post-merge run on master) or -# a non-draft PR, both of which always run the full suite exactly as -# before. `github.event.pull_request.draft` reflects the PR's CURRENT -# draft status at the time each event fires, so once a PR is marked -# ready-for-review, its next actual trigger (a `synchronize` push - this -# workflow's `pull_request:` has no `types:` override, so it only runs on -# the GitHub default of opened/synchronize/reopened, NOT the -# `ready_for_review` action by itself) correctly sees draft=false and runs -# the full suite; the reduced draft-time result on prior commits is not -# retroactively re-run. +# on every push; every other heavy job (`pg-tle-test`, `pg-upgrade-test`) +# is skipped entirely via an added `&& github.event.pull_request.draft != +# true` on its existing `if:`. None of this applies to a `push` event (e.g. +# the post-merge run on master) or a non-draft PR, both of which always run +# the full suite exactly as before. `pull_request.types` below includes +# `ready_for_review` specifically so marking a draft PR ready retriggers +# immediately at full scope (draft is already false by the time that event +# fires) instead of leaving the reduced draft-time result on the PR's last +# commit as its current status until some later real push. +# +# CI PRIORITY: a `gh stack rebase` cascade pushes to every PR above the one +# actually being changed, purely to move it onto a new base - the PR's own +# diff (what it actually contributes) is unchanged. Running that at the +# same priority as real new commits means a burst of cascade pushes can +# crowd out the runner capacity a genuinely active push needs for fast +# feedback. `changes` tells the two apart via a base-independent content +# hash (bin/patch_id_hash, robust to rebase-induced context shifts - see +# its own comment - persisted per-PR across pushes via actions/cache since +# GitHub Actions has no other cross-run memory): a synchronize push whose +# hash matches the last one observed for this PR is routed to a +# LOW-priority lane (`ci--lowprio-<0..2>`, a small fixed set shared +# across ALL PRs, capping rebase noise to at most 3 concurrent runs +# repo-wide instead of letting it consume the account's whole concurrency +# budget) rather than the per-PR high-priority lane real pushes use. It +# still gets the FULL matrix either way, just possibly later - a rebase CAN +# break something the diff itself didn't touch, and this repo won't merge +# without a clean run regardless. `edited` is in `pull_request.types` so a +# base retarget (the signal gh stack sends when this PR is promoted to the +# bottom of its stack - the next one due to merge) can be caught by the +# `escalate` step and bumped to an immediate high-priority run bypassing +# whatever lane its last push landed in, UNLESS a full run already exists +# for that exact head SHA (checked via the Checks API), in which case +# there's nothing to gain by re-running it. `ready_for_review` gets the +# same escalation unconditionally (no existing-run check): its prior run, +# if any, was necessarily the reduced draft-time one above, which doesn't +# count as "already had one". # =========================================================================== name: CI on: @@ -71,6 +94,11 @@ on: branches: - master pull_request: + # Explicit (not the GitHub default of [opened, synchronize, reopened]): + # `ready_for_review` retriggers immediately when a draft PR is marked + # ready (see the top-of-file comment); `edited` catches a base retarget + # for the priority-escalation check in the `changes` job below. + types: [opened, synchronize, reopened, edited, ready_for_review] jobs: # Cheap gate that lets the heavy jobs below skip themselves on commits that # touch only docs. Must run on every push/pull_request (no paths-ignore on @@ -94,11 +122,17 @@ jobs: changes: name: ๐Ÿ” Detect docs-only changes & derive PG matrix runs-on: ubuntu-latest + permissions: + contents: read + checks: read # escalate step: has this SHA already got a full run? + actions: write # escalate step: cancel a superseded low-priority run outputs: docs_only: ${{ steps.diff.outputs.docs_only }} supported_pg: ${{ steps.pg.outputs.supported_pg }} newest_pg: ${{ steps.pg.outputs.newest_pg }} floor_pg: ${{ steps.pg.outputs.floor_pg }} + priority: ${{ steps.diff.outputs.priority }} + lane: ${{ steps.lane.outputs.lane }} steps: - name: Check out the repo uses: actions/checkout@v4 @@ -106,16 +140,93 @@ jobs: # Full history needed so BASE and HEAD below are both reachable # for `git diff`. fetch-depth: 0 - - name: Compute per-push changed files + + - name: Base retargeted onto a new branch? Check whether this SHA already has a full run + # Fires only for `edited` WITH a base change - the gh-stack-promotion + # signal described at the top of this file. A plain title/body edit + # also fires `edited` but leaves `github.event.changes.base` absent, + # so it's excluded here and falls through to the diff step's + # edited-with-nothing-to-do branch below. + id: escalate + if: >- + github.event_name == 'pull_request' && + github.event.action == 'edited' && + github.event.changes.base != null + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + existing=$(gh api "repos/$REPO/commits/$SHA/check-runs" --paginate \ + --jq '[.check_runs[] | select(.name == "all-checks-passed")]' 2>/dev/null || echo '[]') + count=$(echo "$existing" | jq 'length') + if [ "$count" -gt 0 ]; then + echo "escalate=false" >> "$GITHUB_OUTPUT" + echo "SHA $SHA already has an all-checks-passed run - nothing to escalate" + else + echo "escalate=true" >> "$GITHUB_OUTPUT" + fi + + - name: Cancel a superseded low-priority run for this SHA + # An escalation (base retarget, or draft->ready below) is about to + # trigger a fresh high-priority run for this exact SHA; a + # low-priority run still queued/running for the SAME SHA would just + # duplicate that work, so kill it first. ready_for_review always + # qualifies (unconditionally, no escalate check) since its prior + # run, if any, was necessarily the reduced draft-time one - never + # worth keeping once the PR is no longer a draft. + if: >- + steps.escalate.outputs.escalate == 'true' || + github.event.action == 'ready_for_review' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + gh api "repos/$REPO/actions/workflows/ci.yml/runs" --paginate \ + --jq ".workflow_runs[] | select(.head_sha == \"$SHA\" and .status != \"completed\" and .id != ${{ github.run_id }}) | .id" \ + | while read -r run_id; do + echo "Cancelling superseded run $run_id for $SHA" + gh api -X POST "repos/$REPO/actions/runs/$run_id/cancel" || true + done + + - name: Restore this PR's previously observed content patch-id + id: patchid-restore + if: github.event_name == 'pull_request' && github.event.action != 'edited' + uses: actions/cache/restore@v4 + with: + path: /tmp/prev_patch_id + # A key guaranteed not to already exist, so this always falls + # through to restore-keys (prefix match, picks the MOST RECENT + # save) instead of ever hitting its own key directly. + key: unused-${{ github.run_id }} + restore-keys: | + ci-patchid-pr-${{ github.event.pull_request.number }}- + + - name: Compute per-push changed files, docs-only status, and CI priority id: diff run: | - # Fail safe to running the full matrix: default docs_only to false - # immediately, before anything below has a chance to compute or - # fail. Writing the same GITHUB_OUTPUT key twice is fine (the last - # write wins), so the only way this step ends with docs_only=true - # is by genuinely proving it further down - never by skipping past - # an edge case with a default. + # Fail safe to running the full matrix at high priority: default + # both immediately, before anything below has a chance to compute + # or fail. Writing the same GITHUB_OUTPUT key twice is fine (the + # last write wins), so the only way this step ends with + # docs_only=true or priority=low is by genuinely proving it + # further down - never by skipping past an edge case with a + # default. echo "docs_only=false" >> "$GITHUB_OUTPUT" + echo "priority=high" >> "$GITHUB_OUTPUT" + + if [ "${{ github.event.action }}" = "edited" ]; then + # Either a base retarget with nothing outstanding to run + # (escalate above said false - an all-checks-passed run already + # covers this SHA), or a plain title/description edit (no base + # change at all, so escalate didn't even run). Either way, + # there's no code to test right now. + if [ "${{ steps.escalate.outputs.escalate }}" != "true" ]; then + echo "docs_only=true" >> "$GITHUB_OUTPUT" + fi + exit 0 + fi if [ "${{ github.event_name }}" = "pull_request" ] && \ [ "${{ github.event.action }}" = "synchronize" ] && \ @@ -164,6 +275,42 @@ jobs: echo "$CHANGED" echo "docs_only=$DOCS_ONLY" >> "$GITHUB_OUTPUT" + # Priority: a synchronize push whose own patch-id (the PR's + # actual contribution, independent of what it's based on) matches + # the last one observed for this PR is a pure gh-stack rebase + # cascade, not new work - safe to route to the low-priority lane. + # Real new commits, and this PR's first appearance (nothing to + # compare against yet), keep the high priority set above. + if [ "${{ github.event_name }}" = "pull_request" ]; then + NEW_PATCH_ID=$(bin/patch_id_hash "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}") + echo "content patch-id: $NEW_PATCH_ID" + if [ "${{ github.event.action }}" = "synchronize" ] && \ + [ -f /tmp/prev_patch_id ] && \ + [ "$(cat /tmp/prev_patch_id)" = "$NEW_PATCH_ID" ]; then + echo "priority=low" >> "$GITHUB_OUTPUT" + fi + echo "$NEW_PATCH_ID" > /tmp/prev_patch_id + fi + + - name: Save this push's content patch-id for the next push to compare against + if: github.event_name == 'pull_request' && github.event.action != 'edited' + uses: actions/cache/save@v4 + with: + path: /tmp/prev_patch_id + key: ci-patchid-pr-${{ github.event.pull_request.number }}-${{ github.run_id }} + + - name: Assign a low-priority lane number + id: lane + # A fixed small number of shared lanes (not one lane per PR) bounds + # how much of the account's concurrent-runner budget rebase-cascade + # noise can ever occupy at once, so it can't crowd out real-work + # runs waiting for a runner - at the cost of not deduping a stale + # rebase run against a newer one for the SAME PR if they land in the + # same lane as an unrelated PR's job (acceptable: a stale completed + # check on an old SHA doesn't block merging the new SHA). + if: github.event_name == 'pull_request' + run: echo "lane=$(( ${{ github.event.pull_request.number }} % 3 ))" >> "$GITHUB_OUTPUT" + - name: Derive the supported-PostgreSQL-major list id: pg run: | @@ -255,6 +402,18 @@ jobs: test: needs: [changes] if: needs.changes.outputs.docs_only != 'true' + # "ci-test-" prefix, distinct from pg-upgrade-test's/pg-tle-test's below: + # those run CONCURRENTLY with this job within the same PR's workflow run, + # so sharing a group name across job types would make them cancel/queue + # behind EACH OTHER instead of behind their own prior runs for this job. + # See the top-of-file CI PRIORITY comment for the low-priority lane + # scheme this implements. + concurrency: + group: >- + ${{ needs.changes.outputs.priority == 'low' + && format('ci-test-lowprio-{0}', needs.changes.outputs.lane) + || format('ci-test-{0}', github.event.pull_request.number || github.sha) }} + cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: # From the single source in the changes job. On a draft PR, reduced @@ -347,6 +506,13 @@ jobs: # this is a heavy job, and a draft author doesn't need a real binary # pg_upgrade re-proven on every push while still iterating. if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + # "ci-pgupgrade-" prefix - see the `test` job's concurrency comment above. + concurrency: + group: >- + ${{ needs.changes.outputs.priority == 'low' + && format('ci-pgupgrade-lowprio-{0}', needs.changes.outputs.lane) + || format('ci-pgupgrade-{0}', github.event.pull_request.number || github.sha) }} + cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: # Single-element lists built from the changes job's floor_pg/ @@ -490,6 +656,13 @@ jobs: # draft PR: this is a heavy job, and a draft author doesn't need the # pg_tle deployment path re-proven on every push while still iterating. if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + # "ci-pgtle-" prefix - see the `test` job's concurrency comment above. + concurrency: + group: >- + ${{ needs.changes.outputs.priority == 'low' + && format('ci-pgtle-lowprio-{0}', needs.changes.outputs.lane) + || format('ci-pgtle-{0}', github.event.pull_request.number || github.sha) }} + cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: # Intersection of count_nulls' own supported range (10-18, see the diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 51f5c39..d8a8479 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -32,12 +32,88 @@ concurrency: cancel-in-progress: true jobs: - claude-review: - # jnasbyupgrade's own PRs only, and skip drafts (don't spend API/CI on - # unfinished PRs). + # Cheap gate that skips the paid review when this push's PATCH is + # identical to the last one reviewed for this PR - a pure gh-stack rebase + # cascade changes only the base commit, not the PR's own diff, so there's + # nothing new to review. Restricted to `synchronize`: `opened`/`reopened`/ + # `ready_for_review` always run the review regardless of patch-id, since + # draft->ready is itself a meaningful trigger even if nothing about the + # diff needed to change, and comparing against a stale cached patch-id + # there would otherwise wrongly suppress it. Runs before the (slow) "wait + # for CI" gate in claude-review below, so a rebase-only push never enters + # that wait at all. + content-check: + runs-on: ubuntu-latest if: >- github.event.pull_request.draft == false && github.event.pull_request.user.login == 'jnasbyupgrade' + outputs: + content_changed: ${{ steps.diff.outputs.changed }} + steps: + # Default (non-overridden) checkout for a pull_request_target event + # is the BASE branch - deliberate here: bin/patch_id_hash must be on + # disk to run, and an older PR predating this script (checked out at + # its own head, as claude-review's later step does) wouldn't have it. + # The script itself diffs base...head via git plumbing on commit + # objects, not the working tree, so which ref is checked out doesn't + # affect the computed hash - only whether the SCRIPT is present. + - name: Check out the base branch + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Fetch the PR head commit + # fetch-depth 0 above pulls this repo's full history but not a + # fork's (or an as-yet-unmerged) ref - fetch head.sha explicitly so + # it's reachable for the diff below. + run: git fetch origin ${{ github.event.pull_request.head.sha }} + + - name: Restore this PR's previously observed content patch-id + id: patchid-restore + if: github.event.action == 'synchronize' + uses: actions/cache/restore@v4 + with: + path: /tmp/prev_patch_id + # A key guaranteed not to already exist, so this always falls + # through to restore-keys (prefix match, picks the MOST RECENT + # save) instead of ever hitting its own key directly. Same cache + # namespace as ci.yml's `changes` job - both are answering the + # identical "did this PR's patch change" question, so whichever + # workflow runs first primes the cache for the other. + key: unused-${{ github.run_id }} + restore-keys: | + ci-patchid-pr-${{ github.event.pull_request.number }}- + + - name: Compare against the last observed patch-id + id: diff + run: | + NEW=$(bin/patch_id_hash "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}") + echo "content patch-id: $NEW" + if [ "${{ github.event.action }}" = "synchronize" ] && \ + [ -f /tmp/prev_patch_id ] && \ + [ "$(cat /tmp/prev_patch_id)" = "$NEW" ]; then + echo "changed=false" >> "$GITHUB_OUTPUT" + else + echo "changed=true" >> "$GITHUB_OUTPUT" + fi + echo "$NEW" > /tmp/prev_patch_id + + - name: Save this push's content patch-id for the next push to compare against + if: always() + uses: actions/cache/save@v4 + with: + path: /tmp/prev_patch_id + key: ci-patchid-pr-${{ github.event.pull_request.number }}-${{ github.run_id }} + + claude-review: + needs: [content-check] + # jnasbyupgrade's own PRs only, skip drafts (don't spend API/CI on + # unfinished PRs), and skip a pure rebase-cascade push (see + # content-check above - nothing new to review). + if: >- + github.event.pull_request.draft == false && + github.event.pull_request.user.login == 'jnasbyupgrade' && + needs.content-check.outputs.content_changed == 'true' runs-on: ubuntu-latest timeout-minutes: 60 permissions: diff --git a/bin/patch_id_hash b/bin/patch_id_hash new file mode 100755 index 0000000..d91493b --- /dev/null +++ b/bin/patch_id_hash @@ -0,0 +1,13 @@ +#!/bin/sh +# Emits, on stdout, a single hash representing the CONTENT of the diff +# between $1 (base) and $2 (head) - via `git patch-id --stable`, not a plain +# `git diff | sha256sum`. patch-id normalizes away the context-line shifts a +# rebase causes when the base commit itself changes, so it comes out +# IDENTICAL across a pure rebase that introduces no new content. That's the +# signal CI uses to tell "just a gh-stack rebase cascade" apart from "real +# new commits", and route the former to a low-priority runner lane instead +# of treating it like fresh work. +set -eu +base="$1" +head="$2" +git diff "$base...$head" | git patch-id --stable | awk '{print $1}' From 14e4cb1c609f5a2f06f9f7dd62fc3167bd820d04 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 11:52:34 -0500 Subject: [PATCH 02/17] ci.yml: scope high-priority concurrency groups per matrix leg The high-priority concurrency group for test/pg-upgrade-test/pg-tle-test was keyed only on PR number (or SHA), shared across every leg of that job's own matrix - with cancel-in-progress true, a job's second leg to start cancelled its own first leg instead of only ever cancelling a PRIOR push's run of the SAME leg. Confirmed live on this PR's own first push: most PostgreSQL-version/pg_tle legs came back cancelled, directly contradicting the "still runs the full matrix either way" design intent. Now includes matrix.pg (test, pg-tle-test) / matrix.old_pg+matrix.new_pg (pg-upgrade-test) in the high-priority group name, so sibling legs of one push get distinct groups while a same-leg rerun on a later push still supersedes the earlier one. The low-priority lane is deliberately left without this split: cancel-in-progress is false there, so a shared name only serializes legs through the lane instead of cancelling them. --- .github/workflows/ci.yml | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a45376..a73a0f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -408,11 +408,19 @@ jobs: # behind EACH OTHER instead of behind their own prior runs for this job. # See the top-of-file CI PRIORITY comment for the low-priority lane # scheme this implements. + # + # The high-priority group includes ${{ matrix.pg }}: without it, every + # leg of THIS SAME push's own matrix would collide on one shared group + # name and cancel-in-progress would cancel siblings, not just a prior + # push's run - the exact opposite of "always runs the full matrix + # either way". The low-priority group deliberately has NO such + # per-leg split: cancel-in-progress is false there, so a shared name + # only serializes legs through the lane instead of cancelling them. concurrency: group: >- ${{ needs.changes.outputs.priority == 'low' && format('ci-test-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-test-{0}', github.event.pull_request.number || github.sha) }} + || format('ci-test-{0}-{1}', github.event.pull_request.number || github.sha, matrix.pg) }} cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: @@ -506,12 +514,14 @@ jobs: # this is a heavy job, and a draft author doesn't need a real binary # pg_upgrade re-proven on every push while still iterating. if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true - # "ci-pgupgrade-" prefix - see the `test` job's concurrency comment above. + # "ci-pgupgrade-" prefix, matrix.old_pg/new_pg split - see the `test` + # job's concurrency comment above (currently a single-combination + # matrix, but split the same way regardless in case that ever changes). concurrency: group: >- ${{ needs.changes.outputs.priority == 'low' && format('ci-pgupgrade-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-pgupgrade-{0}', github.event.pull_request.number || github.sha) }} + || format('ci-pgupgrade-{0}-{1}-{2}', github.event.pull_request.number || github.sha, matrix.old_pg, matrix.new_pg) }} cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: @@ -656,12 +666,13 @@ jobs: # draft PR: this is a heavy job, and a draft author doesn't need the # pg_tle deployment path re-proven on every push while still iterating. if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true - # "ci-pgtle-" prefix - see the `test` job's concurrency comment above. + # "ci-pgtle-" prefix, matrix.pg split - see the `test` job's concurrency + # comment above. concurrency: group: >- ${{ needs.changes.outputs.priority == 'low' && format('ci-pgtle-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-pgtle-{0}', github.event.pull_request.number || github.sha) }} + || format('ci-pgtle-{0}-{1}', github.event.pull_request.number || github.sha, matrix.pg) }} cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} strategy: matrix: From 51b72276c29105efe5d7fb42047000d522d918b8 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 11:55:20 -0500 Subject: [PATCH 03/17] Empty follow-up push to exercise the low-priority-lane detection No content change - bin/patch_id_hash's base...head patch-id should come out identical to the previous push's, so the changes job should route this synchronize push to the shared low-priority lane instead of the per-PR high-priority one. See the top-of-file CI PRIORITY comment. From f42599b8e6349bb9bf45630149101abeef66a549 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 12:03:59 -0500 Subject: [PATCH 04/17] ci.yml: drop same-SHA cancellation, flag the low-priority lane's own bug The claude-code-review on this PR's original commit already flagged that "Cancel a superseded low-priority run for this SHA" always fires against the exact SHA the new escalated run is about to test (edited/ ready_for_review never change head.sha) - cancelling that other run's all-checks-passed job leaves a competing non-success conclusion for the same check name on the same SHA the escalated run is separately about to report success for. That finding was carried over unfixed into this rebuild; dropping the step entirely removes the race (at the cost of a redundant, already-superseded run finishing rather than being killed early, which was only ever a compute optimization). Also documents a second, more fundamental issue found by actually pushing an unchanged-patch-id follow-up commit: GitHub Actions' concurrency default only keeps one PENDING job per group and cancels the rest rather than queueing them (queue: max would fix it, but it can't combine with a conditionally-true cancel-in-progress, and queue doesn't take an expression) - so the low-priority lane's shared group name, as designed, drops most of a push's own matrix legs instead of deferring them, contradicting the "still gets the FULL matrix either way" guarantee this feature is built around. Left as a flagged open issue rather than guessing at a redesign. --- .github/workflows/ci.yml | 43 +++++++++++++++------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a73a0f3..7e33dc7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,7 +125,6 @@ jobs: permissions: contents: read checks: read # escalate step: has this SHA already got a full run? - actions: write # escalate step: cancel a superseded low-priority run outputs: docs_only: ${{ steps.diff.outputs.docs_only }} supported_pg: ${{ steps.pg.outputs.supported_pg }} @@ -167,29 +166,6 @@ jobs: echo "escalate=true" >> "$GITHUB_OUTPUT" fi - - name: Cancel a superseded low-priority run for this SHA - # An escalation (base retarget, or draft->ready below) is about to - # trigger a fresh high-priority run for this exact SHA; a - # low-priority run still queued/running for the SAME SHA would just - # duplicate that work, so kill it first. ready_for_review always - # qualifies (unconditionally, no escalate check) since its prior - # run, if any, was necessarily the reduced draft-time one - never - # worth keeping once the PR is no longer a draft. - if: >- - steps.escalate.outputs.escalate == 'true' || - github.event.action == 'ready_for_review' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - SHA: ${{ github.event.pull_request.head.sha }} - run: | - gh api "repos/$REPO/actions/workflows/ci.yml/runs" --paginate \ - --jq ".workflow_runs[] | select(.head_sha == \"$SHA\" and .status != \"completed\" and .id != ${{ github.run_id }}) | .id" \ - | while read -r run_id; do - echo "Cancelling superseded run $run_id for $SHA" - gh api -X POST "repos/$REPO/actions/runs/$run_id/cancel" || true - done - - name: Restore this PR's previously observed content patch-id id: patchid-restore if: github.event_name == 'pull_request' && github.event.action != 'edited' @@ -413,9 +389,22 @@ jobs: # leg of THIS SAME push's own matrix would collide on one shared group # name and cancel-in-progress would cancel siblings, not just a prior # push's run - the exact opposite of "always runs the full matrix - # either way". The low-priority group deliberately has NO such - # per-leg split: cancel-in-progress is false there, so a shared name - # only serializes legs through the lane instead of cancelling them. + # either way". Confirmed live: before this split, most legs of a normal + # (non-rebase) push came back cancelled instead of completing. + # + # KNOWN OPEN ISSUE, not yet resolved: the low-priority group deliberately + # has NO such per-leg split (that's the point - it's the shared, + # capped-concurrency lane), but GitHub Actions' default concurrency + # behavior only keeps ONE job PENDING per group at a time and CANCELS + # the previous pending one when a new one arrives (no `queue:` support + # for an expression, and `queue: max` can't combine with a + # conditionally-true `cancel-in-progress`) - confirmed live: pushing an + # unchanged-patch-id follow-up here cancelled most of this job's own + # matrix legs instead of queueing them, contradicting "still gets the + # FULL matrix either way" above for any push whose matrix has more legs + # than fit through instantaneously. Needs a real decision (accept a + # weaker/no concurrency cap, split low-priority into its own dedicated + # job with `queue: max`, or something else) before relying on this path. concurrency: group: >- ${{ needs.changes.outputs.priority == 'low' From f15c875f89132ec0695798e58de1fe8de640bc7c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 12:15:20 -0500 Subject: [PATCH 05/17] ci.yml: record the lint-on-every-edit tradeoff as a deliberate decision Flagged twice by claude-code-review (both times as low-severity/likely acceptable) with no decision recorded anywhere - a plain title/body edit (edited, no base change) now runs a real lint job, since edited was added to pull_request.types for the escalate check. Accepting it: lint is cheap (no PostgreSQL container) and gating it would duplicate the changes job's own docs_only computation on what's meant to be the always-on baseline check. --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e33dc7..d11f167 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -353,6 +353,13 @@ jobs: lint: name: ๐Ÿงน SQL lint + # Deliberately no docs_only/needs gate, unlike the heavy jobs below: a + # plain title/description edit (no base change) now triggers a real + # run of this job too, since `edited` was added to `pull_request.types` + # for the escalate check. Accepted rather than fixed - lint is cheap + # (a checkout + `make lint`, no PostgreSQL container), and gating it + # would need the SAME docs_only computation the `changes` job already + # does, on a job that's meant to be the cheap always-on baseline. runs-on: ubuntu-latest steps: - name: Check out the repo From ddfa663c9767b84bc536cd11edbb21cde0258f66 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:23:21 -0500 Subject: [PATCH 06/17] bin/: extract CI's inline shell logic into standalone scripts docs_only_diff, check_run_exists, and ci_priority pull the changes job's docs-only diff, escalate-check, and patch-id compare/persist logic out of ci.yml's run: blocks, matching this repo's existing convention (patch_id_hash, test_existing, ...) of keeping anything non-trivial testable/runnable on its own instead of only inline in a workflow. pg_binary_upgrade and verify_pgtle_smoke do the same for pg-upgrade-test's binary pg_upgrade invocation and pg-tle-test's post-install smoke check - both about to be duplicated across a high/low-priority job-ID split, so extracting them first keeps that split's diff smaller. --- bin/check_run_exists | 31 ++++++++++++++++++++++++++++ bin/ci_priority | 39 +++++++++++++++++++++++++++++++++++ bin/docs_only_diff | 43 +++++++++++++++++++++++++++++++++++++++ bin/pg_binary_upgrade | 46 ++++++++++++++++++++++++++++++++++++++++++ bin/verify_pgtle_smoke | 39 +++++++++++++++++++++++++++++++++++ 5 files changed, 198 insertions(+) create mode 100755 bin/check_run_exists create mode 100755 bin/ci_priority create mode 100755 bin/docs_only_diff create mode 100755 bin/pg_binary_upgrade create mode 100755 bin/verify_pgtle_smoke diff --git a/bin/check_run_exists b/bin/check_run_exists new file mode 100755 index 0000000..50517d9 --- /dev/null +++ b/bin/check_run_exists @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# +# check_run_exists - Report whether SHA already has a check run named NAME +# (any conclusion) in REPO, via the Checks API. Extracted from ci.yml's +# `changes` job "escalate" step, which uses this to decide whether a base +# retarget (the gh-stack-promotion signal described in ci.yml's top-of-file +# CI PRIORITY comment) has anything left to prove: if an all-checks-passed +# run already covers this exact SHA, escalating again would just re-run +# something whose result is already known. +# +# Prints "true" or "false" to stdout. Requires GH_TOKEN in the environment +# (as the `gh` CLI itself does) and network access to GitHub's API - so, +# unlike this repo's other bin/ scripts, it can't be exercised against a +# local checkout alone, only against a real repo/SHA you have API access +# to. +# +# USAGE: bin/check_run_exists REPO SHA NAME +set -euo pipefail +repo="$1" +sha="$2" +name="$3" + +existing=$(gh api "repos/$repo/commits/$sha/check-runs" --paginate \ + --jq "[.check_runs[] | select(.name == \"$name\")]" 2>/dev/null || echo '[]') +count=$(echo "$existing" | jq 'length') + +if [ "$count" -gt 0 ]; then + echo "true" +else + echo "false" +fi diff --git a/bin/ci_priority b/bin/ci_priority new file mode 100755 index 0000000..3915fe2 --- /dev/null +++ b/bin/ci_priority @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# +# ci_priority - Decide whether a PR's content patch-id (see bin/patch_id_hash) +# matches the last one observed for this PR - i.e. a pure gh-stack rebase +# cascade, not new work, safe to route to CI's low-priority lane - and +# persist this push's patch-id for the NEXT push to compare against. See +# ci.yml's top-of-file CI PRIORITY comment for why that distinction exists. +# Extracted from the `changes` job's diff step so this stateful +# compare-then-persist logic is testable on its own (given a scratch +# PREV_FILE), instead of only inline in a `run:` block. +# +# PREV_FILE holds the last observed patch-id. It's fine for it not to exist +# yet (this PR's first push) - that's always "high", nothing to compare +# against. It's OVERWRITTEN with this push's patch-id before exiting, +# regardless of the verdict, so the NEXT invocation has something to +# compare against. +# +# Prints "low" or "high" to stdout. The caller (ci.yml) is still +# responsible for only trusting a "low" verdict on an actual `synchronize` +# push - see the `changes` job, which also gates on that. +# +# USAGE: bin/ci_priority BASE HEAD PREV_FILE +set -euo pipefail +cd "$(dirname "$(readlink -f "$0")")/.." + +base="$1" +head="$2" +prev_file="$3" + +new_patch_id=$(bin/patch_id_hash "$base" "$head") +echo "content patch-id: $new_patch_id" >&2 + +priority=high +if [ -f "$prev_file" ] && [ "$(cat "$prev_file")" = "$new_patch_id" ]; then + priority=low +fi + +echo "$new_patch_id" > "$prev_file" +echo "$priority" diff --git a/bin/docs_only_diff b/bin/docs_only_diff new file mode 100755 index 0000000..f66abb7 --- /dev/null +++ b/bin/docs_only_diff @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# +# docs_only_diff - Report whether every file changed between BASE and HEAD +# has a docs-only extension (.md or .asc), so CI's `changes` job can let the +# heavy test jobs skip themselves on a pure-documentation push. Extracted +# from that job's "Compute per-push changed files, docs-only status, and CI +# priority" step so the file-extension rule is testable on its own, apart +# from the GitHub Actions event payload that decides which BASE/HEAD to +# pass in. +# +# A failed diff (bad/unreachable refs) or an empty changed-file list is +# treated as NOT docs-only - fail safe to running the full test matrix +# rather than silently skipping it on an edge case this script can't prove +# either way. +# +# Prints "true" or "false" to stdout - the only thing a caller should +# parse. The changed-file list itself goes to stderr, purely for the CI +# log. +# +# USAGE: bin/docs_only_diff BASE HEAD +set -euo pipefail +base="$1" +head="$2" + +changed=$(git diff --name-only "$base" "$head") || changed=__DIFF_FAILED__ + +echo "changed files:" >&2 +echo "$changed" >&2 + +if [ "$changed" = "__DIFF_FAILED__" ] || [ -z "$changed" ]; then + echo "false" + exit 0 +fi + +docs_only=true +while IFS= read -r f; do + case "$f" in + *.md|*.asc) ;; + *) docs_only=false; break ;; + esac +done <<< "$changed" + +echo "$docs_only" diff --git a/bin/pg_binary_upgrade b/bin/pg_binary_upgrade new file mode 100755 index 0000000..d6ab392 --- /dev/null +++ b/bin/pg_binary_upgrade @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# +# pg_binary_upgrade - Stop the OLD_PG cluster, create a fresh NEW_PG cluster +# (with the SAME initdb options the old one used - pg_upgrade refuses to run +# otherwise), binary pg_upgrade between them, then start the new cluster. +# Extracted from pg-upgrade-test's own "Stop old cluster, binary pg_upgrade +# ..., start new cluster" step so its high- and low-priority job-ID variants +# (see ci.yml's top-of-file CI PRIORITY comment for why there are two) share +# this instead of each carrying its own copy, and so it's runnable/debuggable +# directly against a local two-cluster setup, outside CI. +# +# Requires INITDB_OPTS in the environment (the exact options the OLD_PG +# cluster was created with) and both PostgreSQL majors already installed +# on PATH under /usr/lib/postgresql// (pg-start / apt-get, done by +# earlier CI steps - not something this script sets up itself). +# +# On failure, dumps every pg_upgrade log found in either of its two +# possible locations (PG17+ writes to $new_datadir/pg_upgrade_output.d/; +# older versions write to CWD) before exiting non-zero, so the failure +# reason is visible directly in the CI log instead of requiring a separate +# artifact download. +# +# USAGE: bin/pg_binary_upgrade OLD_PG NEW_PG +set -euo pipefail +old_pg="$1" +new_pg="$2" +: "${INITDB_OPTS:?INITDB_OPTS must be set - the same options the old cluster used}" + +pg_ctlcluster "$old_pg" test stop +pg_createcluster -p 5432 "$new_pg" test -- $INITDB_OPTS + +mkdir -p /tmp/pg_upgrade_logs +chown postgres:postgres /tmp/pg_upgrade_logs + +su -c "cd /tmp/pg_upgrade_logs && /usr/lib/postgresql/$new_pg/bin/pg_upgrade \ + -b /usr/lib/postgresql/$old_pg/bin \ + -B /usr/lib/postgresql/$new_pg/bin \ + -d /var/lib/postgresql/$old_pg/test \ + -D /var/lib/postgresql/$new_pg/test \ + -o '-c config_file=/etc/postgresql/$old_pg/test/postgresql.conf' \ + -O '-c config_file=/etc/postgresql/$new_pg/test/postgresql.conf'" postgres \ + || { find /tmp/pg_upgrade_logs \ + "/var/lib/postgresql/$new_pg/test/pg_upgrade_output.d" \ + -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } + +pg_ctlcluster "$new_pg" test start diff --git a/bin/verify_pgtle_smoke b/bin/verify_pgtle_smoke new file mode 100755 index 0000000..2935e82 --- /dev/null +++ b/bin/verify_pgtle_smoke @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# +# verify_pgtle_smoke - Smoke-test a count_nulls installation registered +# purely through pg_tle (no filesystem control file) in DBNAME: confirm the +# version pg_tle actually installed matches what this checkout's control +# file declares as its own default_version, then call null_count() for +# real. Extracted from pg-tle-test's own "Verify count_nulls works when +# deployed via pg_tle" step so its high- and low-priority job-ID variants +# (see ci.yml's top-of-file CI PRIORITY comment) share this instead of each +# carrying its own copy. +# +# Run from the repository root (needs `make` to read +# EXTENSION_count_nulls_VERSION) against a database where DBNAME already +# has count_nulls CREATE EXTENSIONed via pg_tle. +# +# USAGE: bin/verify_pgtle_smoke DBNAME +set -euo pipefail +cd "$(dirname "$(readlink -f "$0")")/.." + +db="$1" + +INSTALLED=$(psql -d "$db" -tAc "SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'") +# EXTENSION_count_nulls_VERSION (the .control file's default_version), NOT +# PGXNVERSION (the PGXN distribution version, from META.in.json) - a +# version-less CREATE EXTENSION installs whatever the control file's +# default_version says, and count_nulls' is currently the 'stable' +# pseudo-version, not the last real release. See RELEASE.md's note on +# distribution vs. extension versions. +EXPECTED=$(make -s print-EXTENSION_count_nulls_VERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') +echo "installed=$INSTALLED expected=$EXPECTED" +if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then + echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 +fi + +RESULT=$(psql -d "$db" -v ON_ERROR_STOP=1 -tAc "SELECT null_count(1, NULL, 2)") +echo "null_count(1, NULL, 2)=$RESULT" +if [ "$RESULT" != "1" ]; then + echo "FAIL: expected null_count(1, NULL, 2) = 1, got '$RESULT'"; exit 1 +fi From 567939f29341504f7257f98e4c71626da20929ea Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:23:31 -0500 Subject: [PATCH 07/17] ci.yml: split low-priority lane into dedicated job IDs, key by matrix leg queue: max (letting up to 100 runs queue in a concurrency group instead of cancelling all but one) cannot combine with a cancel-in-progress that could evaluate true at runtime - which the previous single-job-per-test-type design, with a priority-conditional concurrency: block, required. Confirmed live: pushing an unchanged-patch-id follow-up cancelled most of that job's own matrix legs instead of queueing them. Split test, pg-upgrade-test, and pg-tle-test into high/low-priority job-ID pairs instead: the high-priority job keeps today's per-PR-per-leg group and an unconditional cancel-in-progress: true; the low-priority job gets a literal cancel-in-progress: false plus queue: max, which is only legal because it's now a job a real push can never enter. While at it, key the low-priority group by the matrix leg itself (e.g. ci-test-lowprio-) instead of a hashed 0-2 lane number shared across unrelated legs - this preserves full within-push matrix parallelism (each leg queues independently) while still being "low priority" (only competes with other low-priority runs of that exact same leg, repo-wide). This makes the lane computation entirely redundant, so it's removed along with the changes job's lane output. --- .github/workflows/ci.yml | 412 ++++++++++++++++++++++++++------------- 1 file changed, 274 insertions(+), 138 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d11f167..a62aab5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,24 +69,46 @@ # feedback. `changes` tells the two apart via a base-independent content # hash (bin/patch_id_hash, robust to rebase-induced context shifts - see # its own comment - persisted per-PR across pushes via actions/cache since -# GitHub Actions has no other cross-run memory): a synchronize push whose -# hash matches the last one observed for this PR is routed to a -# LOW-priority lane (`ci--lowprio-<0..2>`, a small fixed set shared -# across ALL PRs, capping rebase noise to at most 3 concurrent runs -# repo-wide instead of letting it consume the account's whole concurrency -# budget) rather than the per-PR high-priority lane real pushes use. It -# still gets the FULL matrix either way, just possibly later - a rebase CAN -# break something the diff itself didn't touch, and this repo won't merge -# without a clean run regardless. `edited` is in `pull_request.types` so a -# base retarget (the signal gh stack sends when this PR is promoted to the -# bottom of its stack - the next one due to merge) can be caught by the -# `escalate` step and bumped to an immediate high-priority run bypassing -# whatever lane its last push landed in, UNLESS a full run already exists -# for that exact head SHA (checked via the Checks API), in which case -# there's nothing to gain by re-running it. `ready_for_review` gets the -# same escalation unconditionally (no existing-run check): its prior run, -# if any, was necessarily the reduced draft-time one above, which doesn't -# count as "already had one". +# GitHub Actions has no other cross-run memory, compare-then-persist logic +# in bin/ci_priority): a synchronize push whose hash matches the last one +# observed for this PR gets `changes.outputs.priority = low` instead of the +# default `high`. +# +# `test`, `pg-upgrade-test`, and `pg-tle-test` each exist as TWO job IDs +# (e.g. `test` / `test-lowprio`) rather than one job with a +# priority-conditional `if:`/`concurrency:` - see each job's own comment for +# why the split itself is necessary (in short: `queue: max`, the mechanism +# that lets the low-priority side QUEUE instead of cancelling, cannot +# combine with a `cancel-in-progress` that could evaluate `true` at +# runtime, so the low-priority variant needs a literal, unconditional +# `cancel-in-progress: false` living in a job a real push can never enter). +# The high-priority variant (`if: needs.changes.outputs.priority != 'low'`) +# keeps a per-PR-per-leg concurrency group and unconditional +# `cancel-in-progress: true`, same as before there were two job IDs. The +# low-priority variant (`if: needs.changes.outputs.priority == 'low'`) uses +# a group keyed by the MATRIX LEG ITSELF (e.g. +# `format('ci-test-lowprio-{0}', matrix.pg)`), shared across ALL PRs' low- +# priority pushes for that same leg value, plus `queue: max` (up to 100 +# runs queue in that group instead of all but one being cancelled). This +# preserves full within-push matrix parallelism (each leg queues +# independently) while still being "low priority": a low-priority run only +# ever competes with OTHER low-priority runs of that exact same leg, +# repo-wide - never with a real push's high-priority run, and never +# blocking unrelated legs of its own push. It still gets the FULL matrix +# either way, just possibly later - a rebase CAN break something the diff +# itself didn't touch, and this repo won't merge without a clean run +# regardless. +# +# `edited` is in `pull_request.types` so a base retarget (the signal gh +# stack sends when this PR is promoted to the bottom of its stack - the +# next one due to merge) can be caught by the `escalate` step (via +# bin/check_run_exists) and bumped to an immediate high-priority run +# bypassing whichever priority variant its last push landed in, UNLESS a +# full run already exists for that exact head SHA (checked via the Checks +# API), in which case there's nothing to gain by re-running it. +# `ready_for_review` gets the same escalation unconditionally (no +# existing-run check): its prior run, if any, was necessarily the reduced +# draft-time one above, which doesn't count as "already had one". # =========================================================================== name: CI on: @@ -131,7 +153,6 @@ jobs: newest_pg: ${{ steps.pg.outputs.newest_pg }} floor_pg: ${{ steps.pg.outputs.floor_pg }} priority: ${{ steps.diff.outputs.priority }} - lane: ${{ steps.lane.outputs.lane }} steps: - name: Check out the repo uses: actions/checkout@v4 @@ -156,10 +177,7 @@ jobs: REPO: ${{ github.repository }} SHA: ${{ github.event.pull_request.head.sha }} run: | - existing=$(gh api "repos/$REPO/commits/$SHA/check-runs" --paginate \ - --jq '[.check_runs[] | select(.name == "all-checks-passed")]' 2>/dev/null || echo '[]') - count=$(echo "$existing" | jq 'length') - if [ "$count" -gt 0 ]; then + if [ "$(bin/check_run_exists "$REPO" "$SHA" all-checks-passed)" = "true" ]; then echo "escalate=false" >> "$GITHUB_OUTPUT" echo "SHA $SHA already has an all-checks-passed run - nothing to escalate" else @@ -233,39 +251,25 @@ jobs: exit 0 fi - CHANGED=$(git diff --name-only "$BASE" "$HEAD" || echo __DIFF_FAILED__) - - DOCS_ONLY=true - if [ "$CHANGED" = "__DIFF_FAILED__" ] || [ -z "$CHANGED" ]; then - DOCS_ONLY=false - else - while IFS= read -r f; do - if ! [[ "$f" =~ \.(md|asc)$ ]]; then - DOCS_ONLY=false - break - fi - done <<< "$CHANGED" - fi - - echo "changed files:" - echo "$CHANGED" + DOCS_ONLY=$(bin/docs_only_diff "$BASE" "$HEAD") echo "docs_only=$DOCS_ONLY" >> "$GITHUB_OUTPUT" # Priority: a synchronize push whose own patch-id (the PR's # actual contribution, independent of what it's based on) matches # the last one observed for this PR is a pure gh-stack rebase - # cascade, not new work - safe to route to the low-priority lane. - # Real new commits, and this PR's first appearance (nothing to - # compare against yet), keep the high priority set above. + # cascade, not new work - safe to route to the low-priority job + # IDs (see the top-of-file CI PRIORITY comment). Real new + # commits, and this PR's first appearance (nothing to compare + # against yet), keep the high priority set above. bin/ci_priority + # always overwrites /tmp/prev_patch_id with this push's patch-id, + # regardless of the verdict, so the NEXT push has something to + # compare against - only the "was it a synchronize push" gate + # below is specific to this call site. if [ "${{ github.event_name }}" = "pull_request" ]; then - NEW_PATCH_ID=$(bin/patch_id_hash "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}") - echo "content patch-id: $NEW_PATCH_ID" - if [ "${{ github.event.action }}" = "synchronize" ] && \ - [ -f /tmp/prev_patch_id ] && \ - [ "$(cat /tmp/prev_patch_id)" = "$NEW_PATCH_ID" ]; then + PRIORITY=$(bin/ci_priority "${{ github.event.pull_request.base.sha }}" "${{ github.event.pull_request.head.sha }}" /tmp/prev_patch_id) + if [ "${{ github.event.action }}" = "synchronize" ] && [ "$PRIORITY" = "low" ]; then echo "priority=low" >> "$GITHUB_OUTPUT" fi - echo "$NEW_PATCH_ID" > /tmp/prev_patch_id fi - name: Save this push's content patch-id for the next push to compare against @@ -275,18 +279,6 @@ jobs: path: /tmp/prev_patch_id key: ci-patchid-pr-${{ github.event.pull_request.number }}-${{ github.run_id }} - - name: Assign a low-priority lane number - id: lane - # A fixed small number of shared lanes (not one lane per PR) bounds - # how much of the account's concurrent-runner budget rebase-cascade - # noise can ever occupy at once, so it can't crowd out real-work - # runs waiting for a runner - at the cost of not deduping a stale - # rebase run against a newer one for the SAME PR if they land in the - # same lane as an unrelated PR's job (acceptable: a stale completed - # check on an old SHA doesn't block merging the new SHA). - if: github.event_name == 'pull_request' - run: echo "lane=$(( ${{ github.event.pull_request.number }} % 3 ))" >> "$GITHUB_OUTPUT" - - name: Derive the supported-PostgreSQL-major list id: pg run: | @@ -384,40 +376,28 @@ jobs: # suite keeps its output schema-invariant). test: needs: [changes] - if: needs.changes.outputs.docs_only != 'true' + if: needs.changes.outputs.priority != 'low' && needs.changes.outputs.docs_only != 'true' # "ci-test-" prefix, distinct from pg-upgrade-test's/pg-tle-test's below: # those run CONCURRENTLY with this job within the same PR's workflow run, # so sharing a group name across job types would make them cancel/queue # behind EACH OTHER instead of behind their own prior runs for this job. - # See the top-of-file CI PRIORITY comment for the low-priority lane - # scheme this implements. # - # The high-priority group includes ${{ matrix.pg }}: without it, every - # leg of THIS SAME push's own matrix would collide on one shared group - # name and cancel-in-progress would cancel siblings, not just a prior - # push's run - the exact opposite of "always runs the full matrix - # either way". Confirmed live: before this split, most legs of a normal - # (non-rebase) push came back cancelled instead of completing. - # - # KNOWN OPEN ISSUE, not yet resolved: the low-priority group deliberately - # has NO such per-leg split (that's the point - it's the shared, - # capped-concurrency lane), but GitHub Actions' default concurrency - # behavior only keeps ONE job PENDING per group at a time and CANCELS - # the previous pending one when a new one arrives (no `queue:` support - # for an expression, and `queue: max` can't combine with a - # conditionally-true `cancel-in-progress`) - confirmed live: pushing an - # unchanged-patch-id follow-up here cancelled most of this job's own - # matrix legs instead of queueing them, contradicting "still gets the - # FULL matrix either way" above for any push whose matrix has more legs - # than fit through instantaneously. Needs a real decision (accept a - # weaker/no concurrency cap, split low-priority into its own dedicated - # job with `queue: max`, or something else) before relying on this path. + # This is the HIGH-priority job ID - see test-lowprio right below for its + # low-priority counterpart, and the top-of-file CI PRIORITY comment for + # why the two are separate job IDs rather than one job with a + # priority-conditional `if:` here. The group includes ${{ matrix.pg }}: + # without it, every leg of THIS SAME push's own matrix would collide on + # one shared group name and cancel-in-progress would cancel siblings, + # not just a prior push's run - the exact opposite of "always runs the + # full matrix either way". Confirmed live: before this per-leg split, + # most legs of a normal (non-rebase) push came back cancelled instead of + # completing. cancel-in-progress is unconditionally true here (a real + # push always wants its own latest commit's result, never a stale one) + # - it's only the LOW-priority job below that needs it to be + # unconditionally false, for `queue: max` to be usable at all. concurrency: - group: >- - ${{ needs.changes.outputs.priority == 'low' - && format('ci-test-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-test-{0}-{1}', github.event.pull_request.number || github.sha, matrix.pg) }} - cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} + group: ci-test-${{ github.event.pull_request.number || github.sha }}-${{ matrix.pg }} + cancel-in-progress: true strategy: matrix: # From the single source in the changes job. On a draft PR, reduced @@ -456,6 +436,56 @@ jobs: # standalone script, not `make test`), so invoked directly here. run: bin/compare_fresh_vs_update 0.9.6 + # LOW-priority counterpart of `test` above - same steps/matrix, only the + # `if:`/`concurrency:` differ. See the top-of-file CI PRIORITY comment for + # why a gh-stack rebase cascade is routed here instead of to `test`, and + # why that requires a genuinely separate job ID rather than a runtime + # branch inside one job: `queue: max` (letting up to 100 runs QUEUE in a + # concurrency group instead of cancelling all but the latest) cannot + # combine with a `cancel-in-progress` that could evaluate `true` at + # runtime - and a single job with `cancel-in-progress: ${{ + # needs.changes.outputs.priority != 'low' }}` is exactly that, since the + # SAME job could run with priority=high on some other push. Splitting into + # two job IDs means THIS job's `cancel-in-progress: false` is a literal + # that can never be anything else, which is what makes `queue: max` legal + # here at all. + # + # The group is keyed by ${{ matrix.pg }} ALONE (no PR number), so it's + # shared across every PR's low-priority pushes for that same PG major - + # not a small fixed hashed set of lanes (the previous, buggy design): a + # real leg value queues against other runs of that SAME leg, repo-wide, + # while still leaving every OTHER leg of this push's own matrix to queue + # independently in its own group - preserving "still gets the FULL matrix + # either way" from the top-of-file comment, which the previous single- + # hashed-lane design didn't (confirmed live: it collapsed a whole push's + # matrix onto one shared group and cancelled most of its own legs). + test-lowprio: + needs: [changes] + if: needs.changes.outputs.priority == 'low' && needs.changes.outputs.docs_only != 'true' + concurrency: + group: ci-test-lowprio-${{ matrix.pg }} + cancel-in-progress: false + queue: max + strategy: + matrix: + pg: ${{ github.event.pull_request.draft && fromJSON(format('[{0}]', needs.changes.outputs.newest_pg)) || fromJSON(needs.changes.outputs.supported_pg) }} + name: ๐Ÿ˜ PostgreSQL ${{ matrix.pg }} (low-priority) + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + steps: + - name: Start PostgreSQL ${{ matrix.pg }} + run: pg-start ${{ matrix.pg }} + - name: Check out the repo + uses: actions/checkout@v4 + - name: Test on PostgreSQL ${{ matrix.pg }} + run: make test + - name: Install count_nulls + run: make install + - name: Update 0.9.6 -> current and run the suite + run: make verify-results TEST_LOAD_SOURCE=update + - name: Structurally compare the updated objects against a fresh install + run: bin/compare_fresh_vs_update 0.9.6 + # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog # migration to a newer PostgreSQL major). Installs 0.9.6 on the oldest # supported PostgreSQL floor, plants a dependency guard, binary- @@ -509,16 +539,18 @@ jobs: # Skipped outright (not just matrix-reduced like `test`) on a draft PR: # this is a heavy job, and a draft author doesn't need a real binary # pg_upgrade re-proven on every push while still iterating. - if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + if: needs.changes.outputs.priority != 'low' && needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true # "ci-pgupgrade-" prefix, matrix.old_pg/new_pg split - see the `test` # job's concurrency comment above (currently a single-combination # matrix, but split the same way regardless in case that ever changes). + # This is the HIGH-priority job ID; see pg-upgrade-test-lowprio below for + # its low-priority counterpart, and the top-of-file CI PRIORITY comment + # for why splitting into two job IDs (rather than one job with a + # priority-conditional `if:` here) is what makes `queue: max` usable on + # the low-priority side at all. concurrency: - group: >- - ${{ needs.changes.outputs.priority == 'low' - && format('ci-pgupgrade-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-pgupgrade-{0}-{1}-{2}', github.event.pull_request.number || github.sha, matrix.old_pg, matrix.new_pg) }} - cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} + group: ci-pgupgrade-${{ github.event.pull_request.number || github.sha }}-${{ matrix.old_pg }}-${{ matrix.new_pg }} + cancel-in-progress: true strategy: matrix: # Single-element lists built from the changes job's floor_pg/ @@ -581,24 +613,7 @@ jobs: # PATH may not be the new version's. run: make install PG_CONFIG=/usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_config - name: Stop old cluster, binary pg_upgrade to PostgreSQL ${{ matrix.new_pg }}, start new cluster - run: | - pg_ctlcluster ${{ matrix.old_pg }} test stop - pg_createcluster -p 5432 ${{ matrix.new_pg }} test -- $INITDB_OPTS - # PG17+ writes logs to $new_datadir/pg_upgrade_output.d/; older - # versions write to CWD. Search both on failure. - mkdir -p /tmp/pg_upgrade_logs - chown postgres:postgres /tmp/pg_upgrade_logs - su -c "cd /tmp/pg_upgrade_logs && /usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_upgrade \ - -b /usr/lib/postgresql/${{ matrix.old_pg }}/bin \ - -B /usr/lib/postgresql/${{ matrix.new_pg }}/bin \ - -d /var/lib/postgresql/${{ matrix.old_pg }}/test \ - -D /var/lib/postgresql/${{ matrix.new_pg }}/test \ - -o '-c config_file=/etc/postgresql/${{ matrix.old_pg }}/test/postgresql.conf' \ - -O '-c config_file=/etc/postgresql/${{ matrix.new_pg }}/test/postgresql.conf'" postgres \ - || { find /tmp/pg_upgrade_logs \ - /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ - -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } - pg_ctlcluster ${{ matrix.new_pg }} test start + run: bin/pg_binary_upgrade ${{ matrix.old_pg }} ${{ matrix.new_pg }} - name: Update to current after pg_upgrade (proves pg_upgrade preserves OLDEST-version objects) # Exercises ALTER EXTENSION UPDATE AFTER the binary pg_upgrade # above, against the NEW cluster (already started by the previous @@ -644,6 +659,65 @@ jobs: bin/compare_fresh_vs_update 0.9.6 upgrade_oldest_first bin/compare_fresh_vs_update 0.9.6 upgrade_current_first + # LOW-priority counterpart of pg-upgrade-test above - same steps/matrix, + # only the `if:`/`concurrency:` differ. See test-lowprio's comment (right + # after `test` above) for why this needs to be a separate job ID rather + # than a runtime branch inside pg-upgrade-test itself, and the top-of-file + # CI PRIORITY comment for the overall scheme. Group keyed by + # matrix.old_pg + matrix.new_pg alone (no PR number) - shared across every + # PR's low-priority pg-upgrade-test pushes for that same leg combination, + # not the small fixed hashed-lane set the previous design used. + pg-upgrade-test-lowprio: + needs: [changes] + if: needs.changes.outputs.priority == 'low' && needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + concurrency: + group: ci-pgupgrade-lowprio-${{ matrix.old_pg }}-${{ matrix.new_pg }} + cancel-in-progress: false + queue: max + strategy: + matrix: + old_pg: ["${{ needs.changes.outputs.floor_pg }}"] + new_pg: ["${{ needs.changes.outputs.newest_pg }}"] + name: ๐Ÿ”„ Binary pg_upgrade ${{ matrix.old_pg }} โ†’ ${{ matrix.new_pg }} (low-priority) + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + INITDB_OPTS: --data-checksums --auth trust + steps: + - name: Install PostgreSQL ${{ matrix.old_pg }} (cluster created next, with the right initdb options from the start) + run: NO_CLUSTER=1 pg-start ${{ matrix.old_pg }} + - name: Create old cluster with data checksums enabled + run: | + pg_createcluster -p 5432 ${{ matrix.old_pg }} test -- $INITDB_OPTS + pg_ctlcluster ${{ matrix.old_pg }} test start + pg_isready -t 30 + - name: Check out the repo + uses: actions/checkout@v4 + - name: Install count_nulls into old cluster + run: make install + - name: Prepare the old cluster (install + dependency guard), twin databases + run: | + bin/test_existing prepare-old upgrade_oldest_first 0.9.6 + bin/test_existing prepare-old upgrade_current_first 0.9.6 + - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects) + run: bin/test_existing update upgrade_current_first + - name: Install PostgreSQL ${{ matrix.new_pg }} + run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} + - name: Install count_nulls into new cluster + run: make install PG_CONFIG=/usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_config + - name: Stop old cluster, binary pg_upgrade to PostgreSQL ${{ matrix.new_pg }}, start new cluster + run: bin/pg_binary_upgrade ${{ matrix.old_pg }} ${{ matrix.new_pg }} + - name: Update to current after pg_upgrade (proves pg_upgrade preserves OLDEST-version objects) + run: bin/test_existing update upgrade_oldest_first + - name: Run the suite against the pg_upgraded database(s) (existing mode), twin databases + run: | + bin/test_existing run-suite upgrade_oldest_first + bin/test_existing run-suite upgrade_current_first + - name: Structurally compare the pg_upgraded databases against a fresh install + run: | + bin/compare_fresh_vs_update 0.9.6 upgrade_oldest_first + bin/compare_fresh_vs_update 0.9.6 upgrade_current_first + # Covers both a fresh install AND the 0.9.6 -> current update path, both # purely via pg_tle. pgxntool 2.3.0's fix for installcheck's ordering bug # (Postgres-Extensions/pgxntool#83) made `installcheck` (and so `make @@ -661,15 +735,16 @@ jobs: # Skipped outright (not just matrix-reduced like `test` above) on a # draft PR: this is a heavy job, and a draft author doesn't need the # pg_tle deployment path re-proven on every push while still iterating. - if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + if: needs.changes.outputs.priority != 'low' && needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true # "ci-pgtle-" prefix, matrix.pg split - see the `test` job's concurrency - # comment above. + # comment above. This is the HIGH-priority job ID; see + # pg-tle-test-lowprio below for its low-priority counterpart, and the + # top-of-file CI PRIORITY comment for why splitting into two job IDs + # (rather than one job with a priority-conditional `if:` here) is what + # makes `queue: max` usable on the low-priority side at all. concurrency: - group: >- - ${{ needs.changes.outputs.priority == 'low' - && format('ci-pgtle-lowprio-{0}', needs.changes.outputs.lane) - || format('ci-pgtle-{0}-{1}', github.event.pull_request.number || github.sha, matrix.pg) }} - cancel-in-progress: ${{ needs.changes.outputs.priority != 'low' }} + group: ci-pgtle-${{ github.event.pull_request.number || github.sha }}-${{ matrix.pg }} + cancel-in-progress: true strategy: matrix: # Intersection of count_nulls' own supported range (10-18, see the @@ -768,24 +843,7 @@ jobs: createdb count_nulls_smoke psql -d count_nulls_smoke -c "CREATE EXTENSION count_nulls" - name: Verify count_nulls works when deployed via pg_tle - run: | - INSTALLED=$(psql -d count_nulls_smoke -tAc "SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'") - # EXTENSION_count_nulls_VERSION (the .control file's default_version), - # NOT PGXNVERSION (the PGXN distribution version, from META.in.json) - # -- a version-less CREATE EXTENSION installs whatever the control - # file's default_version says, and count_nulls' is currently the - # 'stable' pseudo-version, not the last real release. See - # RELEASE.md's note on distribution vs. extension versions. - EXPECTED=$(make -s print-EXTENSION_count_nulls_VERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') - echo "installed=$INSTALLED expected=$EXPECTED" - if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then - echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 - fi - RESULT=$(psql -d count_nulls_smoke -v ON_ERROR_STOP=1 -tAc "SELECT null_count(1, NULL, 2)") - echo "null_count(1, NULL, 2)=$RESULT" - if [ "$RESULT" != "1" ]; then - echo "FAIL: expected null_count(1, NULL, 2) = 1, got '$RESULT'"; exit 1 - fi + run: bin/verify_pgtle_smoke count_nulls_smoke - name: Verify no stray extension control files after the fresh-install smoke test run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control - name: Install count_nulls at 0.9.6, purely via pg_tle (update-path prep) @@ -827,6 +885,84 @@ jobs: # behind a scratch DESTDIR. run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + # LOW-priority counterpart of pg-tle-test above - same steps/matrix, only + # the `if:`/`concurrency:` differ. See test-lowprio's comment (right after + # `test`, near the top of this file) for why this needs to be a separate + # job ID rather than a runtime branch inside pg-tle-test itself, and the + # top-of-file CI PRIORITY comment for the overall scheme. Group keyed by + # matrix.pg alone (no PR number) - shared across every PR's low-priority + # pg-tle-test pushes for that same PG major, not the small fixed + # hashed-lane set the previous design used. + pg-tle-test-lowprio: + needs: [changes] + if: needs.changes.outputs.priority == 'low' && needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true + concurrency: + group: ci-pgtle-lowprio-${{ matrix.pg }} + cancel-in-progress: false + queue: max + strategy: + matrix: + pg: [18, 17, 16, 15, 14, 13, 12] + name: ๐Ÿงฉ pg_tle ${{ matrix.pg }} (low-priority) + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + PG_TLE_BUILD_VERSION: "1.5.2" + steps: + - name: Start PostgreSQL ${{ matrix.pg }} + run: pg-start ${{ matrix.pg }} + - name: Check out the repo + uses: actions/checkout@v4 + - name: Install rsync + run: apt-get install -y rsync + - name: Snapshot filesystem extension control files (pre-pgtap baseline) + run: bin/assert_fs_clean snapshot ${{ matrix.pg }} /tmp/pre_pgtap_baseline.txt + - name: Install pgtap (test harness dependency) + run: make pgtap + - name: Verify make pgtap installed exactly pgtap.control, nothing else + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/pre_pgtap_baseline.txt pgtap.control + - name: Snapshot filesystem extension control files (pre-pg_tle baseline) + run: bin/assert_fs_clean snapshot ${{ matrix.pg }} /tmp/control_baseline.txt + - name: Build and install pg_tle ${{ env.PG_TLE_BUILD_VERSION }} + run: | + apt-get install -y flex bison libkrb5-dev + git clone --branch v${{ env.PG_TLE_BUILD_VERSION }} --depth 1 https://github.com/aws/pg_tle.git /tmp/pg_tle + make -C /tmp/pg_tle install + - name: Enable pg_tle and restart PostgreSQL ${{ matrix.pg }} + run: | + echo "shared_preload_libraries = 'pg_tle'" >> /etc/postgresql/${{ matrix.pg }}/test/postgresql.conf + pg_ctlcluster ${{ matrix.pg }} test restart + pg_isready -t 30 + - name: Register pg_tle + count_nulls against template1 + run: | + psql -d template1 -c "CREATE EXTENSION pg_tle" + PGDATABASE=template1 make run-pgtle + - name: Verify no stray extension control files landed on the filesystem + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Install count_nulls purely via pg_tle (fresh install, no filesystem trace) + run: | + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/count_nulls.control + createdb count_nulls_smoke + psql -d count_nulls_smoke -c "CREATE EXTENSION count_nulls" + - name: Verify count_nulls works when deployed via pg_tle + run: bin/verify_pgtle_smoke count_nulls_smoke + - name: Verify no stray extension control files after the fresh-install smoke test + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Install count_nulls at 0.9.6, purely via pg_tle (update-path prep) + run: | + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/count_nulls.control + bin/test_existing prepare-old count_nulls_pgtle_update 0.9.6 + - name: Verify no stray extension control files after installing 0.9.6 via pg_tle + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Update 0.9.6 -> current, purely via pg_tle + run: bin/test_existing update count_nulls_pgtle_update + - name: Verify no stray extension control files after the pg_tle update + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Run the real pgTAP suite against the pg_tle-updated database (existing mode) + run: TEST_EXISTING_DEPLOY=pgtle bin/test_existing run-suite count_nulls_pgtle_update + - name: Verify no stray extension control files after the pgTAP suite + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + # A single stable check name for use as a required status check in branch # protection rules. Matrix jobs produce check names like # "๐Ÿ˜ PostgreSQL 14" (one per supported major) which would all need to be @@ -835,7 +971,7 @@ jobs: # off by the `changes` job on a docs-only push), and fails if any failed # or were cancelled. all-checks-passed: - needs: [changes, lint, test, pg-upgrade-test, pg-tle-test] + needs: [changes, lint, test, test-lowprio, pg-upgrade-test, pg-upgrade-test-lowprio, pg-tle-test, pg-tle-test-lowprio] if: always() runs-on: ubuntu-latest steps: From c20d6e7eac6bca1aead92a7449009d4b143d29f3 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:23:36 -0500 Subject: [PATCH 08/17] .github/workflows/README.md: concise CI overview with diagrams Orientation for a first-time reader landing on ci.yml cold: what each job proves, the docs-only gate, the draft-PR reduction, and the priority-lane mechanism, with a job-graph diagram and a priority-routing decision-flow diagram. The detailed rationale still lives in ci.yml's own inline comments; this is deliberately just the map, not the territory. --- .github/workflows/README.md | 115 ++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 .github/workflows/README.md diff --git a/.github/workflows/README.md b/.github/workflows/README.md new file mode 100644 index 0000000..8b04812 --- /dev/null +++ b/.github/workflows/README.md @@ -0,0 +1,115 @@ +# CI workflows + +Orientation for `ci.yml` (and `claude-code-review.yml`) - the high-level shape +only. Each file's own inline comments carry the detailed rationale; this +doc exists so a first-time reader isn't starting from zero. + +## `ci.yml` jobs + +| Job | Proves | +| --- | --- | +| `changes` | Cheap gate: is this push docs-only? What's the supported PostgreSQL major list (from `META.json`)? Is this push high- or low-priority (see below)? | +| `lint` | `make lint` - SQL style, always runs in full. | +| `test` / `test-lowprio` | Fresh `CREATE EXTENSION` + in-place `ALTER EXTENSION UPDATE`, across every supported PostgreSQL major. | +| `pg-upgrade-test` / `-lowprio` | A REAL binary `pg_upgrade` between the oldest and newest supported majors, both update-vs-upgrade orderings. | +| `pg-tle-test` / `-lowprio` | Fresh install and update, both deployed purely via AWS pg_tle's catalog (no filesystem `.control` file). | +| `all-checks-passed` | Single stable required-status-check name; passes iff every job above passed or was legitimately skipped. | + +```mermaid +flowchart LR + changes --> test + changes --> testlp[test-lowprio] + changes --> pgu[pg-upgrade-test] + changes --> pgulp[pg-upgrade-test-lowprio] + changes --> pgt[pg-tle-test] + changes --> pgtlp[pg-tle-test-lowprio] + lint --> all[all-checks-passed] + test --> all + testlp --> all + pgu --> all + pgulp --> all + pgt --> all + pgtlp --> all + changes --> all +``` + +## Docs-only gate + +`changes` diffs the push's changed files (`bin/docs_only_diff`); if every +changed file is `.md`/`.asc`, the heavy jobs (`test*`, `pg-upgrade-test*`, +`pg-tle-test*`) skip themselves via `if: needs.changes.outputs.docs_only != +'true'`. `lint` and `changes` itself always run, so `all-checks-passed` +never gets stuck pending on a doc-only push. + +## Draft PRs + +While a PR is a draft: `lint` runs in full, `test`'s matrix drops to just +the newest PostgreSQL major (still real signal, cheap), and +`pg-upgrade-test`/`pg-tle-test` are skipped entirely. Marking the PR +`ready_for_review` retriggers a full run immediately. + +## Priority lanes (rebase-cascade pushes vs. real work) + +A `gh stack rebase` pushes to every PR above the one actually being +changed, purely to move it onto a new base - the PR's own diff doesn't +change. Running that at the same priority as genuine new commits lets a +burst of cascade pushes crowd out runner capacity that active work needs. + +`changes` tells the two apart with a base-independent content hash +(`bin/patch_id_hash`, compared/persisted per-PR via `bin/ci_priority` + +`actions/cache`): a `synchronize` push whose hash matches the last one +seen for this PR is a pure rebase cascade -> `priority: low`. Everything +else, including this PR's first push, is `priority: high`. + +`test`, `pg-upgrade-test`, and `pg-tle-test` each exist as TWO job IDs +(e.g. `test` / `test-lowprio`), not one job with a priority-conditional +`concurrency:` block - `queue: max` (letting up to 100 runs queue in a +concurrency group instead of cancelling all but one) cannot combine with a +`cancel-in-progress` that could evaluate `true` at runtime, so the +low-priority side needs a literal, unconditional `cancel-in-progress: +false` living in a job a real push can never enter: + +- **High-priority** job: `if: priority != 'low'`, concurrency group keyed + per-PR-per-matrix-leg, `cancel-in-progress: true`. Behaves exactly like + before there were two job IDs. +- **Low-priority** job: `if: priority == 'low'`, concurrency group keyed + by the **matrix leg alone** (e.g. `ci-test-lowprio-`) - shared + across every PR's low-priority pushes for that same leg, repo-wide - + `cancel-in-progress: false`, `queue: max`. Each leg of a single push + still queues independently, so a whole push's matrix isn't collapsed + onto one shared slot; it just competes only with other low-priority + runs of that exact same leg, never with real work. + +Either way, the full matrix still eventually runs - a rebase CAN break +something the diff itself didn't touch, and this repo won't merge without +a clean run regardless. + +```mermaid +flowchart TD + push[PR synchronize push] --> cmp{content patch-id same\nas last push for this PR?} + cmp -- "no (or first push)" --> high[priority: high] + cmp -- "yes" --> low[priority: low] + high --> hijob["high-priority job IDs\ngroup per PR+leg\ncancel-in-progress: true"] + low --> lowjob["low-priority job IDs\ngroup per leg, shared across PRs\ncancel-in-progress: false, queue: max"] + retarget["base retarget (edited) /\nready_for_review"] --> esc{full run already\nexists for this SHA?} + esc -- no --> hijob + esc -- yes --> skip[nothing to escalate] +``` + +### Escalation + +A base retarget (the signal `gh stack` sends when a PR is promoted to the +bottom of its stack, the next one due to merge) or `ready_for_review` +bypasses whatever priority its last push landed in and forces an +immediate high-priority run - unless a full `all-checks-passed` run +already exists for that exact head SHA (`bin/check_run_exists`), in which +case there's nothing to gain by re-running it. + +## `claude-code-review.yml` + +Runs Claude's PR review via `pull_request_target`, gated to PRs authored +by the maintainer (secret-bearing job). It has its own, independent +content-check (same `bin/patch_id_hash` cache namespace as `ci.yml`'s +`changes` job) that skips the review outright on a rebase-cascade push - +it doesn't use `ci.yml`'s priority/lane mechanism at all, since there's +nothing to "queue", only skip or run. From 0f52747de67d3222945bec1812282dc8389be612 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:27:34 -0500 Subject: [PATCH 09/17] Empty follow-up push to exercise the low-priority-lane detection From a4f6656054e5f96e1168951a12e7dfbf8a55bbbe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:30:54 -0500 Subject: [PATCH 10/17] Second empty follow-up push, to check same-leg cross-push queueing From 22419ed551431a146b1f8e93a1506c6439a1dcb4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:36:11 -0500 Subject: [PATCH 11/17] Third empty follow-up push (overlap attempt A) From 0fe67083cdeb6dea3019385f0b952838414a3704 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:36:13 -0500 Subject: [PATCH 12/17] Fourth empty follow-up push (overlap attempt B) From 38d7c57cc35696e8f05a1d9ed4035df5e0c18a21 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:41:23 -0500 Subject: [PATCH 13/17] Fifth empty follow-up push (escalate-while-in-flight test) From 34a963f3b45080a17523d17a935cf10514aab37f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:48:57 -0500 Subject: [PATCH 14/17] Empty follow-up push: let the code review run to completion undisturbed From 07fee4377625ea586efebc2c645594f963f0c60b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 14:58:56 -0500 Subject: [PATCH 15/17] bin/check_run_exists: only count a COMPLETED check run as "already exists" GitHub creates a job's check-run (status "queued") as soon as its workflow run starts, well before its needs: are satisfied - so all-checks-passed's own check-run exists in queued/in_progress state for a SHA's entire matrix runtime, which for a low-priority push (queue: max) can be a long time. Filtering only on .name meant a base retarget arriving while that low-priority run was still queued would see a "queued" all-checks-passed and wrongly conclude there was nothing to escalate - exactly the rebase-cascade-then-promoted-to-bottom-of-stack scenario this feature exists for, not an edge case. Add a status == "completed" filter so only a finished run counts. Caught by Claude Code Review on this PR. --- bin/check_run_exists | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/bin/check_run_exists b/bin/check_run_exists index 50517d9..508d8ce 100755 --- a/bin/check_run_exists +++ b/bin/check_run_exists @@ -1,12 +1,23 @@ #!/usr/bin/env bash # -# check_run_exists - Report whether SHA already has a check run named NAME -# (any conclusion) in REPO, via the Checks API. Extracted from ci.yml's -# `changes` job "escalate" step, which uses this to decide whether a base -# retarget (the gh-stack-promotion signal described in ci.yml's top-of-file -# CI PRIORITY comment) has anything left to prove: if an all-checks-passed -# run already covers this exact SHA, escalating again would just re-run -# something whose result is already known. +# check_run_exists - Report whether SHA already has a COMPLETED check run +# named NAME (any conclusion) in REPO, via the Checks API. Extracted from +# ci.yml's `changes` job "escalate" step, which uses this to decide whether +# a base retarget (the gh-stack-promotion signal described in ci.yml's +# top-of-file CI PRIORITY comment) has anything left to prove: if an +# all-checks-passed run already covers this exact SHA, escalating again +# would just re-run something whose result is already known. +# +# Deliberately filters on status == "completed", not just name: GitHub +# creates a job's check-run (status "queued") as soon as its workflow run +# starts, well BEFORE its `needs:` are satisfied - so all-checks-passed's +# own check-run exists in "queued"/"in_progress" state for this SHA's +# entire matrix runtime, which for a low-priority push (queue: max) can be +# a long time. Without this filter, a retarget arriving while that +# low-priority run is still queued would see a "queued" all-checks-passed +# and wrongly conclude escalation has nothing to gain - exactly the +# rebase-cascade-then-promoted-to-bottom-of-stack scenario this feature +# exists for, not an edge case. # # Prints "true" or "false" to stdout. Requires GH_TOKEN in the environment # (as the `gh` CLI itself does) and network access to GitHub's API - so, @@ -21,7 +32,7 @@ sha="$2" name="$3" existing=$(gh api "repos/$repo/commits/$sha/check-runs" --paginate \ - --jq "[.check_runs[] | select(.name == \"$name\")]" 2>/dev/null || echo '[]') + --jq "[.check_runs[] | select(.name == \"$name\" and .status == \"completed\")]" 2>/dev/null || echo '[]') count=$(echo "$existing" | jq 'length') if [ "$count" -gt 0 ]; then From d60c3da1b02eb8afcffcd3cd402fd6304c15d1a6 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 17:02:23 -0500 Subject: [PATCH 16/17] ci.yml/README.md: fix ambiguous wording, remove narrative duplication "every other heavy job" in the draft-PR paragraph read as "every second job" (alternating) rather than "each of the remaining heavy jobs" - reworded for clarity. ci.yml's top-of-file comment and the new workflows/README.md had grown into two full copies of the same narrative (job-by-job test strategy, and the CI-priority-lane/escalation rationale), which will drift out of sync the next time only one gets edited. Reallocated so each topic has one source of truth: the overview narrative lives in README.md (which already has the table/diagrams for it), ci.yml's top comment is now a short pointer plus what's genuinely specific to this file, and each low-priority job's own inline comment remains the source of truth for its concurrency-group mechanics. Preserved the one implementation detail that was only stated in the removed text (why `ready_for_review` skips the `escalate` step's existing-run check) by moving it next to the `escalate` step itself. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/README.md | 32 ++++---- .github/workflows/ci.yml | 155 ++++++++++-------------------------- 2 files changed, 59 insertions(+), 128 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 8b04812..202fb0a 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -63,22 +63,22 @@ else, including this PR's first push, is `priority: high`. `test`, `pg-upgrade-test`, and `pg-tle-test` each exist as TWO job IDs (e.g. `test` / `test-lowprio`), not one job with a priority-conditional -`concurrency:` block - `queue: max` (letting up to 100 runs queue in a -concurrency group instead of cancelling all but one) cannot combine with a -`cancel-in-progress` that could evaluate `true` at runtime, so the -low-priority side needs a literal, unconditional `cancel-in-progress: -false` living in a job a real push can never enter: - -- **High-priority** job: `if: priority != 'low'`, concurrency group keyed - per-PR-per-matrix-leg, `cancel-in-progress: true`. Behaves exactly like - before there were two job IDs. -- **Low-priority** job: `if: priority == 'low'`, concurrency group keyed - by the **matrix leg alone** (e.g. `ci-test-lowprio-`) - shared - across every PR's low-priority pushes for that same leg, repo-wide - - `cancel-in-progress: false`, `queue: max`. Each leg of a single push - still queues independently, so a whole push's matrix isn't collapsed - onto one shared slot; it just competes only with other low-priority - runs of that exact same leg, never with real work. +`concurrency:` block: + +- **High-priority** job: concurrency group keyed per-PR-per-matrix-leg, + `cancel-in-progress: true` - behaves exactly like before there were two + job IDs. +- **Low-priority** job: concurrency group keyed by the **matrix leg + alone** (e.g. `ci-test-lowprio-`), shared across every PR's + low-priority pushes for that same leg, repo-wide, `cancel-in-progress: + false` + `queue: max` - so a low-priority run only ever competes with + other low-priority runs of that exact same leg, never with real work, + while still running its own push's full matrix. + +See `test-lowprio`'s own comment in `ci.yml` for exactly why this split +into two literal job IDs is necessary (in short: `queue: max` can't +combine with a `cancel-in-progress` that could evaluate `true` at +runtime). Either way, the full matrix still eventually runs - a rebase CAN break something the diff itself didn't touch, and this repo won't merge without diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a62aab5..f85abd0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,40 +1,9 @@ # =========================================================================== -# Test strategy -# -# count_nulls can be arrived at several ways, each of which can break -# differently, so each is exercised by its own job below: -# -# test -- FRESH install: CREATE EXTENSION at the current -# version, across every supported PostgreSQL -# major. Also proves the IN-PLACE extension -# update path (CREATE EXTENSION at 0.9.6, then -# ALTER EXTENSION UPDATE - same PostgreSQL, no -# pg_upgrade) in the same job/matrix, rather -# than a dedicated job: a load mode is just an -# input the same assertions run against, not a -# real environment difference, so giving it its -# own job would only duplicate this job's own -# per-PG-version container/checkout setup for -# no added confidence. -# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on the OLD -# PostgreSQL floor (the same META.json-derived -# floor the `changes` job's supported_pg uses - -# see its floor_pg output), then binary-upgrade -# the cluster to the newest supported major -# (changes job's newest_pg). A single leg (not -# the full PG matrix - by far the most -# expensive job here, installing two full -# PostgreSQL majors and running the real -# pg_upgrade binary), which via twin databases -# migrated by the SAME binary pg_upgrade call -# compares BOTH update-vs-pg_upgrade orderings -# a real user could hit - see the job's own -# comment for why one floor is enough to cover -# both. -# pg-tle-test -- pg_tle DEPLOYMENT: fresh install AND the -# 0.9.6 -> current update path, both registered -# through AWS pg_tle's database-backed catalog -# instead of a filesystem .control file. +# See .github/workflows/README.md for the full job list (what `test`, +# `pg-upgrade-test`, and `pg-tle-test` each prove), the docs-only/draft-PR +# reductions, and the priority-lane design - this comment covers only what +# isn't already there, or what's specific to this workflow file's own +# mechanics. # # During testing we always install into a freshly, randomly generated # schema (see test/install/load.sql; test/README.md covers how @@ -51,9 +20,9 @@ # in full; `test`'s matrix drops to just the newest supported PostgreSQL # major (see its own comment) instead of running full or being skipped # outright, since it's cheap per-leg and a draft author still wants signal -# on every push; every other heavy job (`pg-tle-test`, `pg-upgrade-test`) -# is skipped entirely via an added `&& github.event.pull_request.draft != -# true` on its existing `if:`. None of this applies to a `push` event (e.g. +# on every push; the other heavy jobs (`pg-tle-test`, `pg-upgrade-test`) +# are skipped entirely via an added `&& github.event.pull_request.draft != +# true` on their existing `if:`. None of this applies to a `push` event (e.g. # the post-merge run on master) or a non-draft PR, both of which always run # the full suite exactly as before. `pull_request.types` below includes # `ready_for_review` specifically so marking a draft PR ready retriggers @@ -61,54 +30,14 @@ # fires) instead of leaving the reduced draft-time result on the PR's last # commit as its current status until some later real push. # -# CI PRIORITY: a `gh stack rebase` cascade pushes to every PR above the one -# actually being changed, purely to move it onto a new base - the PR's own -# diff (what it actually contributes) is unchanged. Running that at the -# same priority as real new commits means a burst of cascade pushes can -# crowd out the runner capacity a genuinely active push needs for fast -# feedback. `changes` tells the two apart via a base-independent content -# hash (bin/patch_id_hash, robust to rebase-induced context shifts - see -# its own comment - persisted per-PR across pushes via actions/cache since -# GitHub Actions has no other cross-run memory, compare-then-persist logic -# in bin/ci_priority): a synchronize push whose hash matches the last one -# observed for this PR gets `changes.outputs.priority = low` instead of the -# default `high`. -# -# `test`, `pg-upgrade-test`, and `pg-tle-test` each exist as TWO job IDs -# (e.g. `test` / `test-lowprio`) rather than one job with a -# priority-conditional `if:`/`concurrency:` - see each job's own comment for -# why the split itself is necessary (in short: `queue: max`, the mechanism -# that lets the low-priority side QUEUE instead of cancelling, cannot -# combine with a `cancel-in-progress` that could evaluate `true` at -# runtime, so the low-priority variant needs a literal, unconditional -# `cancel-in-progress: false` living in a job a real push can never enter). -# The high-priority variant (`if: needs.changes.outputs.priority != 'low'`) -# keeps a per-PR-per-leg concurrency group and unconditional -# `cancel-in-progress: true`, same as before there were two job IDs. The -# low-priority variant (`if: needs.changes.outputs.priority == 'low'`) uses -# a group keyed by the MATRIX LEG ITSELF (e.g. -# `format('ci-test-lowprio-{0}', matrix.pg)`), shared across ALL PRs' low- -# priority pushes for that same leg value, plus `queue: max` (up to 100 -# runs queue in that group instead of all but one being cancelled). This -# preserves full within-push matrix parallelism (each leg queues -# independently) while still being "low priority": a low-priority run only -# ever competes with OTHER low-priority runs of that exact same leg, -# repo-wide - never with a real push's high-priority run, and never -# blocking unrelated legs of its own push. It still gets the FULL matrix -# either way, just possibly later - a rebase CAN break something the diff -# itself didn't touch, and this repo won't merge without a clean run -# regardless. -# -# `edited` is in `pull_request.types` so a base retarget (the signal gh -# stack sends when this PR is promoted to the bottom of its stack - the -# next one due to merge) can be caught by the `escalate` step (via -# bin/check_run_exists) and bumped to an immediate high-priority run -# bypassing whichever priority variant its last push landed in, UNLESS a -# full run already exists for that exact head SHA (checked via the Checks -# API), in which case there's nothing to gain by re-running it. -# `ready_for_review` gets the same escalation unconditionally (no -# existing-run check): its prior run, if any, was necessarily the reduced -# draft-time one above, which doesn't count as "already had one". +# See README.md's "Priority lanes" and "Escalation" sections for why a +# `gh stack rebase` cascade is routed to a separate, lower-priority job ID +# instead of competing with real work, and for the base-retarget/ +# `ready_for_review` escalation rules. Each `test`/`pg-upgrade-test`/ +# `pg-tle-test` (-lowprio) job's own comment carries the mechanics specific +# to why ITS `if:`/`concurrency:` block needs two literal job IDs; the +# `escalate` and `diff` steps below carry the mechanics specific to this +# job's own escalation/priority logic. # =========================================================================== name: CI on: @@ -163,10 +92,14 @@ jobs: - name: Base retargeted onto a new branch? Check whether this SHA already has a full run # Fires only for `edited` WITH a base change - the gh-stack-promotion - # signal described at the top of this file. A plain title/body edit - # also fires `edited` but leaves `github.event.changes.base` absent, - # so it's excluded here and falls through to the diff step's - # edited-with-nothing-to-do branch below. + # signal described in README.md's Escalation section. A plain + # title/body edit also fires `edited` but leaves + # `github.event.changes.base` absent, so it's excluded here and + # falls through to the diff step's edited-with-nothing-to-do branch + # below. `ready_for_review` needs no equivalent check here: it isn't + # `edited`, so it never enters this step at all - the diff step's + # default (below) already gives it `priority: high` unconditionally, + # since only a `synchronize` push can ever be downgraded to low. id: escalate if: >- github.event_name == 'pull_request' && @@ -258,7 +191,7 @@ jobs: # actual contribution, independent of what it's based on) matches # the last one observed for this PR is a pure gh-stack rebase # cascade, not new work - safe to route to the low-priority job - # IDs (see the top-of-file CI PRIORITY comment). Real new + # IDs (see README.md's Priority lanes section). Real new # commits, and this PR's first appearance (nothing to compare # against yet), keep the high priority set above. bin/ci_priority # always overwrites /tmp/prev_patch_id with this push's patch-id, @@ -383,9 +316,9 @@ jobs: # behind EACH OTHER instead of behind their own prior runs for this job. # # This is the HIGH-priority job ID - see test-lowprio right below for its - # low-priority counterpart, and the top-of-file CI PRIORITY comment for - # why the two are separate job IDs rather than one job with a - # priority-conditional `if:` here. The group includes ${{ matrix.pg }}: + # low-priority counterpart, and for why the two are separate job IDs + # rather than one job with a priority-conditional `if:` here. The group + # includes ${{ matrix.pg }}: # without it, every leg of THIS SAME push's own matrix would collide on # one shared group name and cancel-in-progress would cancel siblings, # not just a prior push's run - the exact opposite of "always runs the @@ -437,7 +370,7 @@ jobs: run: bin/compare_fresh_vs_update 0.9.6 # LOW-priority counterpart of `test` above - same steps/matrix, only the - # `if:`/`concurrency:` differ. See the top-of-file CI PRIORITY comment for + # `if:`/`concurrency:` differ. See README.md's Priority lanes section for # why a gh-stack rebase cascade is routed here instead of to `test`, and # why that requires a genuinely separate job ID rather than a runtime # branch inside one job: `queue: max` (letting up to 100 runs QUEUE in a @@ -456,9 +389,10 @@ jobs: # real leg value queues against other runs of that SAME leg, repo-wide, # while still leaving every OTHER leg of this push's own matrix to queue # independently in its own group - preserving "still gets the FULL matrix - # either way" from the top-of-file comment, which the previous single- - # hashed-lane design didn't (confirmed live: it collapsed a whole push's - # matrix onto one shared group and cancelled most of its own legs). + # either way" from README.md's Priority lanes section, which the + # previous single-hashed-lane design didn't (confirmed live: it collapsed + # a whole push's matrix onto one shared group and cancelled most of its + # own legs). test-lowprio: needs: [changes] if: needs.changes.outputs.priority == 'low' && needs.changes.outputs.docs_only != 'true' @@ -544,10 +478,8 @@ jobs: # job's concurrency comment above (currently a single-combination # matrix, but split the same way regardless in case that ever changes). # This is the HIGH-priority job ID; see pg-upgrade-test-lowprio below for - # its low-priority counterpart, and the top-of-file CI PRIORITY comment - # for why splitting into two job IDs (rather than one job with a - # priority-conditional `if:` here) is what makes `queue: max` usable on - # the low-priority side at all. + # its low-priority counterpart, and for why splitting into two job IDs + # is what makes `queue: max` usable on the low-priority side at all. concurrency: group: ci-pgupgrade-${{ github.event.pull_request.number || github.sha }}-${{ matrix.old_pg }}-${{ matrix.new_pg }} cancel-in-progress: true @@ -662,8 +594,8 @@ jobs: # LOW-priority counterpart of pg-upgrade-test above - same steps/matrix, # only the `if:`/`concurrency:` differ. See test-lowprio's comment (right # after `test` above) for why this needs to be a separate job ID rather - # than a runtime branch inside pg-upgrade-test itself, and the top-of-file - # CI PRIORITY comment for the overall scheme. Group keyed by + # than a runtime branch inside pg-upgrade-test itself, and README.md's + # Priority lanes section for the overall scheme. Group keyed by # matrix.old_pg + matrix.new_pg alone (no PR number) - shared across every # PR's low-priority pg-upgrade-test pushes for that same leg combination, # not the small fixed hashed-lane set the previous design used. @@ -738,10 +670,9 @@ jobs: if: needs.changes.outputs.priority != 'low' && needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true # "ci-pgtle-" prefix, matrix.pg split - see the `test` job's concurrency # comment above. This is the HIGH-priority job ID; see - # pg-tle-test-lowprio below for its low-priority counterpart, and the - # top-of-file CI PRIORITY comment for why splitting into two job IDs - # (rather than one job with a priority-conditional `if:` here) is what - # makes `queue: max` usable on the low-priority side at all. + # pg-tle-test-lowprio below for its low-priority counterpart, and for + # why splitting into two job IDs is what makes `queue: max` usable on + # the low-priority side at all. concurrency: group: ci-pgtle-${{ github.event.pull_request.number || github.sha }}-${{ matrix.pg }} cancel-in-progress: true @@ -888,9 +819,9 @@ jobs: # LOW-priority counterpart of pg-tle-test above - same steps/matrix, only # the `if:`/`concurrency:` differ. See test-lowprio's comment (right after # `test`, near the top of this file) for why this needs to be a separate - # job ID rather than a runtime branch inside pg-tle-test itself, and the - # top-of-file CI PRIORITY comment for the overall scheme. Group keyed by - # matrix.pg alone (no PR number) - shared across every PR's low-priority + # job ID rather than a runtime branch inside pg-tle-test itself, and + # README.md's Priority lanes section for the overall scheme. Group keyed + # by matrix.pg alone (no PR number) - shared across every PR's low-priority # pg-tle-test pushes for that same PG major, not the small fixed # hashed-lane set the previous design used. pg-tle-test-lowprio: From 7ba00e26567d16e2227e25c997c181bc043feb73 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 17:10:00 -0500 Subject: [PATCH 17/17] README.md: fix Escalation section - ready_for_review has no existing-run check The Escalation section and its diagram stated ready_for_review goes through the same has-a-full-run-already-happened check as a base retarget, but the escalate step (ci.yml) only ever fires for edited with a base change - it never runs for ready_for_review at all, which gets priority: high unconditionally from the diff step's own default. Split the prose and diagram into the two actual distinct rules. --- .github/workflows/README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 202fb0a..284eb42 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -91,19 +91,23 @@ flowchart TD cmp -- "yes" --> low[priority: low] high --> hijob["high-priority job IDs\ngroup per PR+leg\ncancel-in-progress: true"] low --> lowjob["low-priority job IDs\ngroup per leg, shared across PRs\ncancel-in-progress: false, queue: max"] - retarget["base retarget (edited) /\nready_for_review"] --> esc{full run already\nexists for this SHA?} + retarget["base retarget (edited)"] --> esc{full run already\nexists for this SHA?} esc -- no --> hijob esc -- yes --> skip[nothing to escalate] + ready[ready_for_review] --> hijob ``` ### Escalation A base retarget (the signal `gh stack` sends when a PR is promoted to the -bottom of its stack, the next one due to merge) or `ready_for_review` -bypasses whatever priority its last push landed in and forces an -immediate high-priority run - unless a full `all-checks-passed` run -already exists for that exact head SHA (`bin/check_run_exists`), in which -case there's nothing to gain by re-running it. +bottom of its stack, the next one due to merge) bypasses whatever +priority its last push landed in and forces an immediate high-priority +run - unless a full `all-checks-passed` run already exists for that exact +head SHA (`bin/check_run_exists`), in which case there's nothing to gain +by re-running it. `ready_for_review` always forces an immediate +high-priority run unconditionally, with no existing-run check: its prior +run, if any, was necessarily the reduced draft-time one, which never +counts as "already covered." ## `claude-code-review.yml`