From 24f64c52a2583d9daf227a8c18c6aff46adf0924 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 30 Jul 2026 14:36:46 -0500
Subject: [PATCH 01/15] Add binary pg_upgrade CI testing, docs-only gate,
 single-source PG matrix

Implements the remaining CI-structure items from the advanced update+upgrade
testing pattern (modeled on Postgres-Extensions/cat_tools's ci.yml/
bin/test_existing and what has landed on its master since), stacked on the
foundation from PR #22 (TEST_LOAD_SOURCE, dependency guard, load.sql):

- Scope `push` to `branches: [master]`; `pull_request` stays unrestricted --
  fixes the double-triggered-CI-on-every-PR-commit bug (test_factory was
  named as one of the repos still needing this).
- Job-level `changes` gate: computes a per-push docs-only diff (fail-safe =
  not-docs-only as the literal first line) instead of a workflow-level
  `paths-ignore`, so heavy jobs can skip on doc-only pushes without leaving
  all-checks-passed stuck Pending.
- Single source of truth for the supported-PostgreSQL-major list (NEWEST=17,
  FLOOR=10, matching the existing matrix), derived once in `changes` and
  consumed via fromJSON by both the `test` and new `pg-upgrade-test`
  matrices.
- `all-checks-passed` gate, self-checking that its `needs` list matches the
  actual job set.
- New `pg-upgrade-test` job: binary pg_upgrade legs (10->17, 16->17) --
  install the current version on an old cluster, pg_upgrade to a newer
  major, then run the suite against the migrated objects in existing mode.
  No bridge step needed: test_factory has shipped only one version, so
  every leg installs current directly on the old cluster. Mechanics
  factored into `.github/scripts/pg_upgrade_cluster` (generic, modeled on
  cat_tools's script of the same name) and `bin/test_existing`
  (test_factory-specific, much smaller than cat_tools's own since there's
  no bridge/multi-origin machinery to carry -- prepare-old + run-suite is
  the whole surface).
- `test` job now gates on `make verify-results` instead of pgxn-tools'
  `pg-build-test`: pgxntool marks installcheck `.IGNORE`, so the old job
  was silently exiting 0 even when regression.diffs was nonempty.
  PGXNTOOL_ENABLE_VERIFY_RESULTS is already pgxntool's own default but is
  now pinned explicitly in the Makefile, matching ENABLE_TEST_INSTALL's
  existing explicit-over-implicit convention.
- Dynamic version assertion (bin/test_existing's assert_version): the
  installed version is always derived from `make -s print-PGXNVERSION`,
  never hardcoded, with empty-value guards on both sides.

Real bug found by actually running the pg_upgrade dry run locally (PG12/16
-> PG17, using throwaway data directories, per the verification requirement
-- not just written and trusted): test/helpers/create.sql's security-definer
function check had no ORDER BY, so its row order depended on pg_proc's
physical layout. That happens to match creation order on a fresh CREATE
EXTENSION but is NOT preserved by pg_upgrade's dump/restore, which produced
a real (but harmless -- every individual assertion still said "ok") text
diff against the fresh-install expected output. Fixed with an explicit
ORDER BY, which turns out to make one set of expected-output files valid
for fresh, existing, AND pg_upgraded modes alike -- no third numbered
alternate file needed, simpler than it first looked.

Skipped, per the scoping decided before starting (see PR description for
full reasoning): extension-update-test job (no second version has ever
shipped), bridge/multi-origin update machinery (cat_tools-specific technical
debt test_factory doesn't have), pg-upgrade-stepwise (test_factory has no
catalog-internals-touching views/functions), pg_tle testing (not a
deployment target), the `stable` pseudo-version (real feature work, not
requested).

Verified locally: make verify-results passes cleanly on both PG12 and PG17
(shared clusters); a full old-cluster-install -> pg_upgrade -> new-cluster
existing-mode suite run (PG12->PG17 and PG12->PG16, using scratch data
directories, never touching the shared clusters) passes with zero raw "not
ok" TAP lines using the actual committed bin/test_existing and
pg_upgrade_cluster scripts, not just ad hoc commands.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/scripts/pg_upgrade_cluster |  90 +++++++++++
 .github/workflows/ci.yml           | 239 ++++++++++++++++++++++++++++-
 Makefile                           |   8 +
 bin/test_existing                  | 111 ++++++++++++++
 test/helpers/create.sql            |   8 +
 5 files changed, 454 insertions(+), 2 deletions(-)
 create mode 100755 .github/scripts/pg_upgrade_cluster
 create mode 100755 bin/test_existing

diff --git a/.github/scripts/pg_upgrade_cluster b/.github/scripts/pg_upgrade_cluster
new file mode 100755
index 0000000..6ee59c0
--- /dev/null
+++ b/.github/scripts/pg_upgrade_cluster
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash
+#
+# pg_upgrade_cluster - Shared binary pg_upgrade mechanics for CI. Modeled on
+# Postgres-Extensions/cat_tools's script of the same name/purpose -- this
+# part of the flow (recreating the pgxn-tools image's "test" cluster with
+# matching initdb options, then driving pg_upgrade between two majors) has
+# nothing extension-specific about it, so it lives here once instead of
+# being inlined in the workflow.
+#
+# Lives under .github/, not bin/: unlike bin/test_existing (which a developer
+# can run locally against a scratch database), this is CI-mechanics-only --
+# pg_ctlcluster/pg_createcluster/pg_upgrade against the pgxn-tools image's
+# "test" cluster convention isn't a locally-runnable workflow.
+#
+# USAGE: .github/scripts/pg_upgrade_cluster <subcommand> [args]
+#
+#   recreate-old PG_VERSION
+#       pg-start's default "test" cluster doesn't have data checksums
+#       enabled, but binary pg_upgrade requires the old and new clusters to
+#       have MATCHING checksum/auth settings (see $INITDB_OPTS below) --
+#       stop, drop, and recreate the "test" cluster for PG_VERSION with them,
+#       then start it and wait for readiness.
+#
+#   upgrade OLD_PG NEW_PG
+#       Stop the old cluster, create the new cluster's "test" cluster (same
+#       $INITDB_OPTS), binary pg_upgrade OLD_PG -> NEW_PG, then start the new
+#       cluster and wait for readiness. On pg_upgrade failure, dumps its logs
+#       (PG17+ writes them to $new_datadir/pg_upgrade_output.d/; older
+#       versions write to CWD -- both are searched) before failing.
+#
+# $INITDB_OPTS (env var, required): initdb options both clusters must share so
+# pg_upgrade sees consistent settings on old and new (e.g.
+# "--data-checksums --auth trust"). Deliberately left unquoted at each use
+# site so its (space-separated) options word-split into separate arguments.
+set -euo pipefail
+
+: "${INITDB_OPTS:?INITDB_OPTS must be set}"
+
+recreate_old() {
+  local pg=$1
+  pg_ctlcluster "$pg" test stop
+  pg_dropcluster "$pg" test
+  # -p 5432: pg_createcluster assigns the next available port, which may not
+  # be 5432 after pg-start has claimed and released it. Force 5432 so
+  # subsequent psql/createdb calls connect without -p.
+  pg_createcluster -p 5432 "$pg" test -- $INITDB_OPTS
+  pg_ctlcluster "$pg" test start
+  pg_isready -t 30
+}
+
+upgrade() {
+  local old_pg=$1 new_pg=$2
+  pg_ctlcluster "$old_pg" test stop
+  pg_createcluster -p 5432 "$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/$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
+  pg_isready -t 30
+}
+
+usage() {
+  echo "usage: .github/scripts/pg_upgrade_cluster <subcommand> [args]" >&2
+  echo "  recreate-old PG_VERSION" >&2
+  echo "  upgrade OLD_PG NEW_PG" >&2
+  exit 2
+}
+
+main() {
+  local cmd=${1:-}
+  shift || true
+  case "$cmd" in
+    recreate-old) recreate_old "$@" ;;
+    upgrade)      upgrade "$@" ;;
+    *)            usage ;;
+  esac
+}
+
+main "$@"
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index a55553f..86af27c 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,5 +1,39 @@
+# Test strategy
+#
+# A test_factory install can be arrived at two ways, each of which can break
+# differently, so each gets its own job below:
+#
+#   test             -- FRESH install: CREATE EXTENSION at the current
+#                        version, on every supported PostgreSQL major. The
+#                        baseline a brand-new user gets.
+#   pg-upgrade-test   -- BINARY pg_upgrade: install the current version on an
+#                        OLD major, binary-upgrade the cluster to a NEWER
+#                        major, then run the suite against the migrated
+#                        objects in "existing" mode (test/install/load.sql).
+#                        Proves objects created on an old server still work
+#                        read back on a new one -- fresh-install testing
+#                        never exercises this at all.
+#
+# test_factory has shipped only one version (0.5.0), so there is no
+# in-place `ALTER EXTENSION UPDATE` path to test yet (no extension-update-test
+# job) and the pg_upgrade job needs no bridge step (it always installs the
+# CURRENT version on the old cluster -- there's no older, pg_upgrade-unsafe
+# version in the wild to carry forward). Both jobs derive their PostgreSQL
+# major list from the single source of truth computed in the `changes` job
+# below, so adding/dropping a supported major is a one-line edit there.
 name: CI
-on: [push, pull_request]
+on:
+  # Post-merge CI only; pull_request already covers every PR commit. Without
+  # this, a commit on a branch with an open PR fires the workflow TWICE for
+  # the identical SHA (once for push, once for pull_request/synchronize) --
+  # double the compute, and a flake on one duplicate run shows a confusing
+  # red check right next to an identical green one for the same commit.
+  push:
+    branches:
+      - master
+  pull_request:
+env:
+  PGUSER: postgres
 jobs:
   # Style linter (https://github.com/Postgres-Extensions/linter, vendored at
   # .vendor/linter). Deliberately checked out WITHOUT submodules -- `make
@@ -19,10 +53,101 @@ jobs:
       - name: Lint SQL
         run: make lint
 
+  # Cheap gate that lets the heavy jobs below skip themselves on commits that
+  # touch only docs, without a workflow-level `paths-ignore` (which would skip
+  # the *whole* workflow, including all-checks-passed, on a docs-only push --
+  # leaving a required check stuck Pending forever). Also derives, from a
+  # single set of constants, the supported-PostgreSQL-major list every heavy
+  # job below consumes, so adding a major is a one-line edit here instead of
+  # N separate matrix edits.
+  changes:
+    name: 🔍 Detect changes & derive PG matrix
+    runs-on: ubuntu-latest
+    outputs:
+      docs_only: ${{ steps.diff.outputs.docs_only }}
+      supported_pg: ${{ steps.pg.outputs.supported_pg }}
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v4
+        with:
+          # Full history needed so BASE and HEAD below are both reachable
+          # for `git diff`.
+          fetch-depth: 0
+      - name: Compute per-push changed files
+        id: diff
+        run: |
+          # Fail-safe is the literal first line: any early exit or error
+          # further down (a bad BASE/HEAD, a failed git diff) leaves this in
+          # place, so a broken check never silently skips real testing.
+          echo "docs_only=false" >> "$GITHUB_OUTPUT"
+
+          if [ "${{ github.event_name }}" = "pull_request" ] && \
+             [ "${{ github.event.action }}" = "synchronize" ] && \
+             [ -n "${{ github.event.before }}" ]; then
+            # A push to an already-open PR: before/after give the true
+            # per-push diff, same as for a branch push.
+            BASE="${{ github.event.before }}"
+            HEAD="${{ github.event.after }}"
+          elif [ "${{ github.event_name }}" = "pull_request" ]; then
+            # First run for this PR (opened/reopened/etc, or synchronize
+            # without a usable before): fall back to the whole base...head
+            # diff.
+            BASE="${{ github.event.pull_request.base.sha }}"
+            HEAD="${{ github.event.pull_request.head.sha }}"
+          else
+            BASE="${{ github.event.before }}"
+            HEAD="${{ github.event.after }}"
+          fi
+
+          echo "base=$BASE"
+          echo "head=$HEAD"
+
+          # A missing HEAD, or an all-zeros BASE (e.g. a new branch's first
+          # push, where GitHub reports no prior commit), means we can't
+          # compute a real diff -- the fail-safe default above stands.
+          if [ -z "$HEAD" ] || [ -z "$BASE" ] || [[ "$BASE" =~ ^0+$ ]]; then
+            exit 0
+          fi
+
+          CHANGED=$(git diff --name-only "$BASE" "$HEAD" || echo __DIFF_FAILED__)
+          echo "changed files:"
+          echo "$CHANGED"
+
+          if [ "$CHANGED" = "__DIFF_FAILED__" ] || [ -z "$CHANGED" ]; then
+            exit 0
+          fi
+
+          DOCS_ONLY=true
+          while IFS= read -r f; do
+            if ! [[ "$f" =~ \.(md|asc)$ ]]; then
+              DOCS_ONLY=false
+              break
+            fi
+          done <<< "$CHANGED"
+
+          echo "docs_only=$DOCS_ONLY" >> "$GITHUB_OUTPUT"
+      - name: Derive the supported-PostgreSQL-major list
+        id: pg
+        run: |
+          # Single source of truth for the supported PostgreSQL majors: the
+          # `test` and `pg-upgrade-test` matrices below both derive their
+          # version lists from here, so they cannot silently drift onto
+          # different lists. To add or drop a major, edit only NEWEST/FLOOR.
+          NEWEST=17
+          FLOOR=10
+          supported=$(seq "$NEWEST" -1 "$FLOOR")
+          # Emit a JSON array from a list of ints, for matrix: to consume
+          # with fromJSON.
+          json() { printf '%s\n' "$@" | paste -sd, - | sed 's/^/[/; s/$/]/'; }
+          echo "supported_pg=$(json $supported)" >> "$GITHUB_OUTPUT"
+
   test:
+    needs: [changes]
+    if: needs.changes.outputs.docs_only != 'true'
     strategy:
       matrix:
-        pg: [17, 16, 15, 14, 13, 12, 11, 10]
+        # Current-supported majors, from the single source in the changes job.
+        pg: ${{ fromJSON(needs.changes.outputs.supported_pg) }}
     name: 🐘 PostgreSQL ${{ matrix.pg }}
     runs-on: ubuntu-latest
     container: pgxn/pgxn-tools
@@ -46,3 +171,113 @@ jobs:
       # extension itself, so no separate `make install` step is needed here.
       - name: Test on PostgreSQL ${{ matrix.pg }}
         run: make verify-results
+
+  # Proves test_factory survives a BINARY pg_upgrade (in-place catalog
+  # migration to a newer PostgreSQL major), not just a fresh CREATE EXTENSION.
+  # Each leg: install the CURRENT version on an old cluster, binary-pg_upgrade
+  # to a newer major, then run the suite against the REAL migrated objects in
+  # "existing" mode (test/install/load.sql asserts the version, plants, and
+  # proves the dependency guard -- see bin/test_existing and
+  # test/install/load.sql). No bridge step: unlike an extension with multiple
+  # shipped versions, test_factory has only ever shipped 0.5.0, so there is no
+  # older, pg_upgrade-unsafe install to carry forward -- every leg installs
+  # the current version directly on the old cluster.
+  #
+  # Two legs, not an every-major stepwise climb (deliberately out of scope --
+  # see the PR description): the oldest-to-newest jump (widest catalog
+  # distance) and the newest-boundary jump (most likely to hit a *new*
+  # PostgreSQL major's catalog change first). test_factory has no views or
+  # functions touching catalog internals (no SELECT * over a system catalog),
+  # so the per-major-boundary risk a full stepwise climb protects against is
+  # low here.
+  pg-upgrade-test:
+    needs: [changes]
+    if: needs.changes.outputs.docs_only != 'true'
+    strategy:
+      matrix:
+        include:
+          - old_pg: "10"
+            new_pg: "17"
+          - old_pg: "16"
+            new_pg: "17"
+    name: 🔄 Binary pg_upgrade ${{ matrix.old_pg }} → ${{ matrix.new_pg }}
+    runs-on: ubuntu-latest
+    container: pgxn/pgxn-tools
+    env:
+      # Both clusters must use the same initdb options so pg_upgrade sees
+      # consistent settings (checksums, auth) on old and new clusters.
+      INITDB_OPTS: --data-checksums --auth trust
+    steps:
+      - name: Start PostgreSQL ${{ matrix.old_pg }}
+        run: pg-start ${{ matrix.old_pg }}
+      - name: Check out the repo
+        uses: actions/checkout@v4
+      - name: Recreate old cluster with data checksums enabled
+        run: .github/scripts/pg_upgrade_cluster recreate-old ${{ matrix.old_pg }}
+      - name: Install test_factory into old cluster
+        run: make install
+      - name: Prepare the old cluster (install current version + tap schema)
+        # test_factory_pgtap needs pgtap installed into a dedicated "tap"
+        # schema BEFORE it installs (see bin/test_existing's prepare-old
+        # comment) -- a bare CREATE EXTENSION test_factory_pgtap CASCADE with
+        # no schema prep leaves pgtap wherever the ambient search_path
+        # resolves, and the suite's SET search_path = tap then can't find
+        # pgtap's functions.
+        run: bin/test_existing prepare-old test_factory_upgrade
+      - name: Install PostgreSQL ${{ matrix.new_pg }}
+        run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }}
+      - name: Install test_factory into new cluster
+        # PG_CONFIG must be specified explicitly: at this point both old and
+        # new PostgreSQL are installed, and the default pg_config on 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: .github/scripts/pg_upgrade_cluster upgrade ${{ matrix.old_pg }} ${{ matrix.new_pg }}
+      - name: Run the suite against the pg_upgraded database (existing mode)
+        # run-suite dynamically asserts the installed version (never
+        # hardcoded), then runs the suite via --use-existing (so pg_regress
+        # does not drop/recreate the database) and gates on verify-results --
+        # test/install/load.sql's existing-mode branch plants and proves the
+        # dependency guard as part of this same invocation.
+        run: bin/test_existing run-suite test_factory_upgrade
+
+  # A single stable check name for use as a required status check in branch
+  # protection rules (not configured by this PR -- that needs a repo admin).
+  # Matrix jobs produce check names like "🐘 PostgreSQL 14", which would all
+  # need to be listed individually and updated whenever the matrix changes.
+  # This job passes if every other job passed or was skipped (e.g. the heavy
+  # jobs gated off by the `changes` job on a docs-only push), and fails if any
+  # failed or were cancelled.
+  all-checks-passed:
+    needs: [changes, test, pg-upgrade-test]
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v4
+      - name: Verify all jobs are listed in needs
+        # Ensures this job won't silently ignore a newly-added job that was
+        # omitted from the needs list above.
+        run: |
+          DEFINED=$(python3 -c "
+          import yaml
+          with open('.github/workflows/ci.yml') as f:
+              w = yaml.safe_load(f)
+          print('\n'.join(sorted(j for j in w['jobs'] if j != 'all-checks-passed')))
+          ")
+          NEEDED=$(echo '${{ toJson(needs) }}' | python3 -c "
+          import json, sys
+          print('\n'.join(sorted(json.load(sys.stdin))))
+          ")
+          if [ "$DEFINED" != "$NEEDED" ]; then
+            echo "Some jobs are missing from all-checks-passed needs:"
+            diff <(echo "$DEFINED") <(echo "$NEEDED")
+            exit 1
+          fi
+      - name: Check all jobs passed or were skipped
+        run: |
+          if [[ "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" == "true" ]]; then
+            echo "One or more jobs failed or were cancelled"
+            exit 1
+          fi
+# vi: expandtab ts=2 sw=2
diff --git a/Makefile b/Makefile
index c8787b5..bb7b306 100644
--- a/Makefile
+++ b/Makefile
@@ -7,6 +7,14 @@ PGXNTOOL_ENABLE_TEST_BUILD = yes
 
 PGXNTOOL_ENABLE_TEST_INSTALL = yes
 
+# Already pgxntool's own default, but pinned explicitly (same reasoning as
+# ENABLE_TEST_INSTALL above): CI's `test`/`pg-upgrade-test` jobs gate on
+# `make verify-results`, not a bare `make test` -- pgxntool marks
+# installcheck .IGNORE, so a plain test run exits 0 even when
+# regression.diffs is nonempty. Relying on a default silently protects that
+# gate only until pgxntool's own default changes.
+PGXNTOOL_ENABLE_VERIFY_RESULTS = yes
+
 # ------------------------------------------------------------------------------
 # TEST_LOAD_SOURCE: how test/install/load.sql gets the extension to its
 # target state (fresh/update/existing). See test/install/load.sql for what
diff --git a/bin/test_existing b/bin/test_existing
new file mode 100755
index 0000000..96b6a66
--- /dev/null
+++ b/bin/test_existing
@@ -0,0 +1,111 @@
+#!/usr/bin/env bash
+#
+# Exercise the test_factory test suite against a REAL database whose
+# extension was installed -- and, for the pg_upgrade CI job, binary
+# pg_upgraded -- OUTSIDE the suite ("existing" mode; see
+# test/install/load.sql and test/CLAUDE.md). Modeled on
+# Postgres-Extensions/cat_tools's bin/test_existing (same purpose), scoped
+# down: test_factory has shipped only one version, so there is no
+# bridge/multi-origin update path to carry, and every subcommand below
+# always targets the CURRENT build.
+#
+# USAGE: bin/test_existing <subcommand> [args]
+#
+#   prepare-old DB
+#       Old-cluster prep for the pg_upgrade CI job: create DB, then install
+#       pgtap into a dedicated "tap" schema BEFORE test_factory_pgtap
+#       installs, then CREATE EXTENSION test_factory_pgtap CASCADE (which
+#       pulls in test_factory) at the current version -- there is no older
+#       version to bridge from. The tap-schema-first ordering matters: if
+#       pgtap lands wherever the ambient search_path resolves instead (e.g.
+#       a bare CREATE EXTENSION test_factory_pgtap CASCADE with no schema
+#       prep), test_factory_pgtap's own suite (SET search_path = tap) can't
+#       find pgtap's functions and every test errors with "function
+#       no_plan() does not exist" -- hit for real while testing the
+#       foundation this script builds on (PR #22).
+#
+#   run-suite DB
+#       Assert the installed version matches the current build (never
+#       hardcoded -- derived from `make -s print-PGXNVERSION`, so a broken
+#       extraction can't silently compare "" to ""), then run the suite in
+#       existing mode via --use-existing (pg_regress must not drop/recreate
+#       DB) and gate on verify-results, not a bare `make test` (pgxntool
+#       marks installcheck .IGNORE, so a plain test run exits 0 even when
+#       regression.diffs is nonempty).
+#
+# No separate guard-planting subcommand: unlike cat_tools, test_factory's
+# dependency guard (a view in schema test_factory_drop_guard depending on
+# tf.tap(text,text)) is planted AND proved entirely inside
+# test/install/load.sql's existing-mode branch, which run-suite's `make
+# test` invocation triggers as its own first regression step. This script
+# doesn't need to plant or re-prove it separately -- it exists to protect
+# the SUITE RUN itself (a stray non-CASCADE drop from a bug elsewhere in the
+# suite) rather than to prove the guard survived pg_upgrade, since binary
+# pg_upgrade either carries every user object over intact or fails outright;
+# there's no partial-survival case here for a pre-upgrade guard to detect
+# that a post-upgrade one wouldn't already catch.
+set -euo pipefail
+
+# Run from the repository root regardless of the caller's cwd, same
+# resolution cat_tools's bin/test_existing uses.
+cd "$(dirname "$(readlink -f "$0")")/.."
+
+psql_do() {
+  local db=$1
+  shift
+  psql -d "$db" -v ON_ERROR_STOP=1 "$@"
+}
+
+current_version() {
+  make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p'
+}
+
+installed_version() {
+  psql -d "$1" -tAc "SELECT extversion FROM pg_extension WHERE extname = 'test_factory'"
+}
+
+# The empty-value guards matter -- "" != "" is false, so a broken extraction
+# on either side must not silently pass.
+assert_version() {
+  local db=$1 expected installed
+  expected=$(current_version)
+  installed=$(installed_version "$db")
+  echo "version check '$db': installed='$installed' expected='$expected'"
+  if [ -z "$installed" ] || [ -z "$expected" ] || [ "$installed" != "$expected" ]; then
+    echo "FAIL: test_factory in '$db' is '$installed', expected '$expected'" >&2
+    exit 1
+  fi
+}
+
+prepare_old() {
+  local db=$1
+  createdb "$db"
+  psql_do "$db" -c "CREATE SCHEMA tap; CREATE EXTENSION pgtap SCHEMA tap; CREATE EXTENSION test_factory_pgtap CASCADE;"
+}
+
+run_suite() {
+  local db=$1
+  assert_version "$db"
+  local existing_args="TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=$db EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no"
+  make test $existing_args
+  make verify-results $existing_args
+}
+
+usage() {
+  echo "usage: bin/test_existing <subcommand> [args]" >&2
+  echo "  prepare-old DB" >&2
+  echo "  run-suite DB" >&2
+  exit 2
+}
+
+main() {
+  local cmd=${1:-}
+  shift || true
+  case "$cmd" in
+    prepare-old) prepare_old "$@" ;;
+    run-suite)   run_suite "$@" ;;
+    *)           usage ;;
+  esac
+}
+
+main "$@"
diff --git a/test/helpers/create.sql b/test/helpers/create.sql
index 8ff7b7f..bf9f6dc 100644
--- a/test/helpers/create.sql
+++ b/test/helpers/create.sql
@@ -90,6 +90,13 @@ SELECT hasnt_table(
  * anymore now that this file no longer runs it itself.
  */
 
+-- ORDER BY is load-bearing, not cosmetic: with no ordering this scans
+-- pg_proc in physical order, which happens to match creation order on a
+-- fresh CREATE EXTENSION but is NOT preserved by pg_upgrade (its
+-- dump/restore reconstructs pg_proc in a different, e.g. name-sorted,
+-- order) -- discovered because the pg_upgrade CI leg's "existing" run
+-- produced a row-reordered (but otherwise identical) diff against this
+-- same query's fresh-install expected output.
 SELECT cmp_ok(
       proconfig
       , '@>'
@@ -100,6 +107,7 @@ SELECT cmp_ok(
     JOIN pg_namespace n ON n.oid = pronamespace
   WHERE n.nspname IN ( 'tf', '_tf' )
     AND p.prosecdef
+  ORDER BY p.oid::regproc::text
 ;
 
 

From da0fb49974fb5dadd810428912e76755b1380829 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 30 Jul 2026 14:51:54 -0500
Subject: [PATCH 02/15] Fix two real CI bugs found by the first PR #23 CI run

- test job: `make verify-results` alone races install vs installcheck under
  this container's ambient parallel make (they're independent prerequisites
  of the same `test` target) -- pg_regress could start, and fail with
  "extension ... is not available", before install's file copy finished.
  Split into two separate `make` invocations, which can't race with each
  other. Reproduced the failure mode's shape locally (though not the race
  itself -- couldn't get this container's make to lose the race on demand)
  and confirmed the two-step form still passes.

- pg-upgrade-test job: never installed pgtap system-wide on either cluster.
  It worked by accident in local dry-runs only because this dev container
  already had pgtap installed for some PG majors from earlier testing --
  confirmed by deliberately clearing /usr/share/postgresql/16/extension/
  (a major this container had never used) and re-running the full
  recreate-old -> prepare-old -> pg_upgrade -> run-suite cycle end to end:
  it failed the same way PR #23's CI did ("extension pgtap is not
  available"), then passed once both `pgxn install pgtap --sudo
  --pg_config ...` steps were added (old cluster before prepare-old, new
  cluster before its make install -- pg_upgrade itself needs pgtap
  available on the new cluster too, not just post-upgrade). --pg_config is
  explicit on both, not left to rely on pg-start's PATH-switching, since
  that's exactly the kind of ambient-state assumption that already broke
  once in this same job.

Verified locally end-to-end (real pg_ctlcluster/pg_createcluster/pg_upgrade,
not just make test): old=12/new=16, a pair this container had never
exercised before, all the way through bin/test_existing run-suite with zero
raw "not ok" TAP lines. Also re-confirmed plain `make test` still passes on
PG12 and PG17 after these changes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/workflows/ci.yml | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 86af27c..7163589 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -214,6 +214,17 @@ jobs:
         uses: actions/checkout@v4
       - name: Recreate old cluster with data checksums enabled
         run: .github/scripts/pg_upgrade_cluster recreate-old ${{ matrix.old_pg }}
+      - name: Install pgtap into the old cluster
+        # test_factory_pgtap requires pgtap (see prepare-old below). Unlike
+        # the fresh-install `test` job, which gets this for free because
+        # pg-start's default cluster already has it, this job recreates the
+        # cluster from scratch (previous step) with none of the fresh-install
+        # job's setup -- pgtap must be installed explicitly here too, for
+        # every PostgreSQL major this job touches, old and new alike.
+        # --pg_config is explicit (not relying on pg-start having already
+        # pointed the bare `pg_config` on PATH at this major) so this step's
+        # correctness doesn't depend on that PATH-switching behavior.
+        run: pgxn install pgtap --sudo --pg_config /usr/lib/postgresql/${{ matrix.old_pg }}/bin/pg_config
       - name: Install test_factory into old cluster
         run: make install
       - name: Prepare the old cluster (install current version + tap schema)
@@ -226,6 +237,15 @@ jobs:
         run: bin/test_existing prepare-old test_factory_upgrade
       - name: Install PostgreSQL ${{ matrix.new_pg }}
         run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }}
+      - name: Install pgtap into the new cluster
+        # pg_upgrade validates that every extension installed in a database
+        # being upgraded is also available (control file + library) on the
+        # NEW cluster's PostgreSQL install -- without this, the upgrade
+        # itself fails, not just the post-upgrade suite run. --pg_config is
+        # required here (unlike the previous pgtap install step): this step
+        # runs before pg-start ever points at the new major, so the plain
+        # `pg_config` on PATH would still resolve to the OLD version's.
+        run: pgxn install pgtap --sudo --pg_config /usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_config
       - name: Install test_factory into new cluster
         # PG_CONFIG must be specified explicitly: at this point both old and
         # new PostgreSQL are installed, and the default pg_config on PATH may

From 32085bab7e4e215b83618d2030090fd27b281914 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 30 Jul 2026 17:08:06 -0500
Subject: [PATCH 03/15] Convert multi-line SQL comment to /* */ style

The ORDER BY rationale added for the pg_upgrade row-ordering fix used
consecutive -- lines for one continuous remark; convert it to a /* */
block per the repo's comment convention (see the sibling foundation
fix in advanced-testing/foundation).
---
 test/helpers/create.sql | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/test/helpers/create.sql b/test/helpers/create.sql
index bf9f6dc..775a299 100644
--- a/test/helpers/create.sql
+++ b/test/helpers/create.sql
@@ -90,13 +90,15 @@ SELECT hasnt_table(
  * anymore now that this file no longer runs it itself.
  */
 
--- ORDER BY is load-bearing, not cosmetic: with no ordering this scans
--- pg_proc in physical order, which happens to match creation order on a
--- fresh CREATE EXTENSION but is NOT preserved by pg_upgrade (its
--- dump/restore reconstructs pg_proc in a different, e.g. name-sorted,
--- order) -- discovered because the pg_upgrade CI leg's "existing" run
--- produced a row-reordered (but otherwise identical) diff against this
--- same query's fresh-install expected output.
+/*
+ * ORDER BY is load-bearing, not cosmetic: with no ordering this scans
+ * pg_proc in physical order, which happens to match creation order on a
+ * fresh CREATE EXTENSION but is NOT preserved by pg_upgrade (its
+ * dump/restore reconstructs pg_proc in a different, e.g. name-sorted,
+ * order) -- discovered because the pg_upgrade CI leg's "existing" run
+ * produced a row-reordered (but otherwise identical) diff against this
+ * same query's fresh-install expected output.
+ */
 SELECT cmp_ok(
       proconfig
       , '@>'

From 5f5223af3dcc60ceaf87bccfb4e9f9f0ac4fc037 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Tue, 4 Aug 2026 17:32:39 -0500
Subject: [PATCH 04/15] Gate pg-upgrade-test behind test, add concurrency,
 dedupe run_suite, bump checkout

- ci.yml: pg-upgrade-test now needs [changes, test], not just [changes] --
  a trivially-broken PR (fails the cheap fresh-install matrix) no longer
  also burns the full, expensive binary pg_upgrade matrix. Added a
  top-level concurrency block (cancel-in-progress) so a superseded push
  cancels an in-flight run instead of letting that expensive matrix run to
  completion for nothing. all-checks-passed's own needs/self-check
  invariant is unaffected (its needs list and job set didn't change shape).
- bin/test_existing: run_suite() called make test then make
  verify-results back to back -- verify-results already depends on test
  (pgxntool's base.mk), so every pg-upgrade-test CI leg paid for two full
  pg_regress --use-existing runs against the real migrated database
  instead of one. Dropped the redundant call.
- actions/checkout@v4 -> @v7 (current latest major), all 5 occurrences
  (lint, changes, test, pg-upgrade-test, all-checks-passed -- one more
  than the brief's "4" since the lint job wasn't part of this branch when
  that count was written).

Verified locally: full make lint + make verify-results (fresh mode)
still pass. Reproduced a REAL binary pg_upgrade leg end to end (16 -> 17,
using throwaway pg_createcluster data directories on custom ports, never
touching this container's shared main clusters): recreated old cluster
with data checksums, installed pgtap + test_factory, prepared it via
bin/test_existing prepare-old, created the new cluster, installed pgtap +
test_factory there too, ran the actual pg_upgrade binary, started the new
cluster, then ran bin/test_existing run-suite against it -- all 3 tests
passed with the deduplicated run_suite().
---
 .github/workflows/ci.yml | 22 ++++++++++++++++------
 bin/test_existing        | 21 +++++++++++++--------
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 7163589..6573841 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -32,6 +32,13 @@ on:
     branches:
       - master
   pull_request:
+# A superseded push (e.g. a quick follow-up commit on the same PR) cancels
+# any in-flight run for the same branch instead of letting the full,
+# expensive matrix -- including pg-upgrade-test's binary pg_upgrade legs --
+# run to completion for a commit nobody cares about anymore.
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
 env:
   PGUSER: postgres
 jobs:
@@ -49,7 +56,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
       - name: Lint SQL
         run: make lint
 
@@ -68,7 +75,7 @@ jobs:
       supported_pg: ${{ steps.pg.outputs.supported_pg }}
     steps:
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           # Full history needed so BASE and HEAD below are both reachable
           # for `git diff`.
@@ -155,7 +162,7 @@ jobs:
       - name: Start PostgreSQL ${{ matrix.pg }}
         run: pg-start ${{ matrix.pg }}
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
       # rsync first: pgxntool/run-test-build.sh needs it to sync
       # test/build/*.sql into test/build/sql/, and the CONTAINER IMAGE
       # doesn't ship rsync.
@@ -191,7 +198,10 @@ jobs:
   # so the per-major-boundary risk a full stepwise climb protects against is
   # low here.
   pg-upgrade-test:
-    needs: [changes]
+    # Also needs `test`, not just `changes`: without this, a trivially-broken
+    # PR (fails the cheap fresh-install matrix) still burns the full,
+    # expensive binary pg_upgrade matrix for nothing.
+    needs: [changes, test]
     if: needs.changes.outputs.docs_only != 'true'
     strategy:
       matrix:
@@ -211,7 +221,7 @@ jobs:
       - name: Start PostgreSQL ${{ matrix.old_pg }}
         run: pg-start ${{ matrix.old_pg }}
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
       - name: Recreate old cluster with data checksums enabled
         run: .github/scripts/pg_upgrade_cluster recreate-old ${{ matrix.old_pg }}
       - name: Install pgtap into the old cluster
@@ -274,7 +284,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Check out the repo
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
       - name: Verify all jobs are listed in needs
         # Ensures this job won't silently ignore a newly-added job that was
         # omitted from the needs list above.
diff --git a/bin/test_existing b/bin/test_existing
index 96b6a66..a4f8a74 100755
--- a/bin/test_existing
+++ b/bin/test_existing
@@ -37,13 +37,14 @@
 # dependency guard (a view in schema test_factory_drop_guard depending on
 # tf.tap(text,text)) is planted AND proved entirely inside
 # test/install/load.sql's existing-mode branch, which run-suite's `make
-# test` invocation triggers as its own first regression step. This script
-# doesn't need to plant or re-prove it separately -- it exists to protect
-# the SUITE RUN itself (a stray non-CASCADE drop from a bug elsewhere in the
-# suite) rather than to prove the guard survived pg_upgrade, since binary
-# pg_upgrade either carries every user object over intact or fails outright;
-# there's no partial-survival case here for a pre-upgrade guard to detect
-# that a post-upgrade one wouldn't already catch.
+# verify-results` invocation triggers as its own first regression step. This
+# script doesn't need to plant or re-prove it separately -- it exists to
+# protect the SUITE RUN itself (a stray non-CASCADE drop from a bug
+# elsewhere in the suite) rather than to prove the guard survived
+# pg_upgrade, since binary pg_upgrade either carries every user object over
+# intact or fails outright; there's no partial-survival case here for a
+# pre-upgrade guard to detect that a post-upgrade one wouldn't already
+# catch.
 set -euo pipefail
 
 # Run from the repository root regardless of the caller's cwd, same
@@ -87,7 +88,11 @@ run_suite() {
   local db=$1
   assert_version "$db"
   local existing_args="TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=$db EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no"
-  make test $existing_args
+  # verify-results already depends on `test` (pgxntool's base.mk), so a
+  # separate `make test` call first would just re-run the entire regression
+  # suite a second time against the real migrated database for nothing --
+  # every pg-upgrade-test CI leg was paying for two full
+  # `pg_regress --use-existing` runs instead of one.
   make verify-results $existing_args
 }
 

From 00afebf5911e68415163ace1a24e1d8c0923b078 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Tue, 4 Aug 2026 17:33:54 -0500
Subject: [PATCH 05/15] Bump NEWEST to 18: PG18 confirmed installable in
 pgxn-tools image

The advanced-extension-testing doc's reference implementation
(cat_tools) is on NEWEST=18; this repo was still on 17. Rather than
assume pg-start's `apt.postgresql.org.sh -i -p -v "$PGVERSION"` can
install PG18 (a matrix expansion that silently failed to install would
be a much worse failure mode than not bumping), confirmed it via an
actual CI run: pushed NEWEST=18 alone first and watched the new
"PostgreSQL 18" job -- it installed postgresql-18 18.4-1.pgdg13+1 via
pg-start and the full fresh-install suite passed.

With NEWEST=18, also shifted pg-upgrade-test's "newest-boundary" leg
from 16->17 to 17->18 and its "oldest-to-newest" leg from 10->17 to
10->18, keeping both legs matching the job's own stated rationale
(widest catalog distance; most likely to hit a *new* major's catalog
change first) now that 18 is the newest major instead of 17.
---
 .github/workflows/ci.yml | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 6573841..c207a14 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -140,7 +140,7 @@ jobs:
           # `test` and `pg-upgrade-test` matrices below both derive their
           # version lists from here, so they cannot silently drift onto
           # different lists. To add or drop a major, edit only NEWEST/FLOOR.
-          NEWEST=17
+          NEWEST=18
           FLOOR=10
           supported=$(seq "$NEWEST" -1 "$FLOOR")
           # Emit a JSON array from a list of ints, for matrix: to consume
@@ -207,9 +207,9 @@ jobs:
       matrix:
         include:
           - old_pg: "10"
-            new_pg: "17"
-          - old_pg: "16"
-            new_pg: "17"
+            new_pg: "18"
+          - old_pg: "17"
+            new_pg: "18"
     name: 🔄 Binary pg_upgrade ${{ matrix.old_pg }} → ${{ matrix.new_pg }}
     runs-on: ubuntu-latest
     container: pgxn/pgxn-tools

From b1018e5819463dcc4b8c30c6fc73c4d5b8a9119a Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Tue, 4 Aug 2026 17:44:23 -0500
Subject: [PATCH 06/15] Fix all-checks-passed: add missing lint job to its own
 needs list

Caught by the job's own self-verification step on the first full CI run
of this rebased branch (confirmed via a real run, not just local YAML
parsing): the `lint` job (SQL Lint, inherited via the pgxntool 2.3.0
sync stacked below this PR -- not part of this PR's own original
commits) was present in the workflow but missing from
all-checks-passed's needs: list, since that list was carried over
unmodified from before `lint` existed on this branch. all-checks-passed
would otherwise silently ignore SQL Lint results entirely -- exactly
the class of bug its own self-check step exists to catch, which is
what actually caught it here.
---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index c207a14..507bf1a 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -279,7 +279,7 @@ jobs:
   # jobs gated off by the `changes` job on a docs-only push), and fails if any
   # failed or were cancelled.
   all-checks-passed:
-    needs: [changes, test, pg-upgrade-test]
+    needs: [changes, lint, test, pg-upgrade-test]
     if: always()
     runs-on: ubuntu-latest
     steps:

From 54a3a29d42ed9b458c78af7d2aae01a04324496a Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Wed, 5 Aug 2026 15:56:05 -0500
Subject: [PATCH 07/15] Fix claude-review's fork-PR checkout refusal; bump
 checkout on both workflows

claude-code-review.yml's "Check out PR head" step has been failing on every
fork-headed PR all session ("Refusing to check out fork pull request code
from a 'pull_request_target' workflow") -- actions/checkout v4.4.0+
(backported to every floating major tag, including the @v4 this workflow
was pinned to) added a default-on refusal to check out a fork PR's head
under pull_request_target, requiring an explicit opt-in
(allow-unsafe-pr-checkout: true). This had been treated as a pre-existing,
unrelated failure; it is not -- it's a real, fixable gap.

Safe to opt in here specifically because both halves of the required
two-layer defense already hold: the job's trust gate
(head.repo.owner.login == 'jnasbyupgrade') restricts this to PRs from the
trusted fork only, and the checked-out code is read-only from there on
(persist-credentials: false, fed only to the review action, never built or
executed). Added allow-unsafe-pr-checkout: true, and strengthened the
trust-gate's own comment to a loud, unmissable warning -- once this flag is
set, that condition is the entire security boundary between an arbitrary
external fork and this job's secrets + a checked-out copy of that fork's
code, so a future edit that loosens it must not be able to do so quietly.

Also bumped actions/checkout@v4 -> @v7 on both claude-code-review.yml and
claude.yml (the Node.js-20-deprecation-warning fix requested for these two
files specifically; claude.yml doesn't need allow-unsafe-pr-checkout since
its checkout step never targets a fork's PR head at all -- it's triggered
by issue_comment/pull_request_review*, not pull_request_target, and checks
out the plain default ref).

Verification note (structural limitation, not a gap in this PR): this PR's
OWN claude-review check cannot demonstrate this fix -- pull_request_target
always runs the workflow file from the PR's base branch, never the PR's own
copy, and separately, all of PRs #32-35 are upstream-headed (not
jnasbyupgrade-fork-headed), so the trust gate itself evaluates false and the
job shows "skipping" regardless of this fix. Real verification only happens
on a subsequent fork-headed PR (the normal PR pattern for this repo) whose
base branch already includes this commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/workflows/claude-code-review.yml | 25 +++++++++++++++++++++---
 .github/workflows/claude.yml             |  2 +-
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index 6585d24..0c52015 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -21,8 +21,18 @@ concurrency:
 
 jobs:
   claude-review:
-    # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs).
-    # To add more trusted owners, extend the head-owner check.
+    # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login
+    # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external
+    # fork's PR and this job's write-capable GITHUB_TOKEN, CLAUDE_CODE_OAUTH_TOKEN,
+    # and -- now that the checkout step below sets allow-unsafe-pr-checkout: true --
+    # a checked-out copy of that fork's own code running in this trusted context.
+    # Drop or loosen this check and the checkout step's "safe because the job is
+    # already gated to a trusted fork" justification stops being true, turning this
+    # into a textbook "pwn request" vulnerability. To trust an additional fork,
+    # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) --
+    # never replace it with something broader (a wildcard, a check on PR author
+    # instead of head repo owner, etc.). Skips drafts too (don't spend API/CI on
+    # unfinished PRs).
     if: >-
       github.event.pull_request.draft == false &&
       github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade'
@@ -78,12 +88,21 @@ jobs:
         if: steps.gate.outputs.decision == 'run'
         # Intentionally tracks the major-version tag (not a pinned SHA) so
         # upstream fixes are picked up automatically.
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           repository: ${{ github.event.pull_request.head.repo.full_name }}
           ref: ${{ github.event.pull_request.head.sha }}
           fetch-depth: 1
           persist-credentials: false
+          # actions/checkout v4.4.0+ (backported to every floating major tag)
+          # refuses to check out a fork PR's head under pull_request_target by
+          # default ("Refusing to check out fork pull request code..."). Safe to
+          # opt in here specifically because BOTH halves of the two-layer
+          # defense this needs are already true: the job-level trust gate above
+          # restricts this to PRs from the trusted jnasbyupgrade fork only, and
+          # the checked-out code is read-only from here on (persist-credentials:
+          # false, fed only to the review action below, never built or run).
+          allow-unsafe-pr-checkout: true
 
       - name: Run Claude Code Review
         if: steps.gate.outputs.decision == 'run'
diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml
index 7d1656c..dc559d1 100644
--- a/.github/workflows/claude.yml
+++ b/.github/workflows/claude.yml
@@ -36,7 +36,7 @@ jobs:
       - name: Checkout repository
         # Intentionally tracks the major-version tag (not a pinned SHA) so
         # upstream fixes are picked up automatically.
-        uses: actions/checkout@v4
+        uses: actions/checkout@v7
         with:
           fetch-depth: 1
           persist-credentials: false

From 460736389f29a1ec90dd38bf273d56aeadedb1c3 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Wed, 5 Aug 2026 16:00:00 -0500
Subject: [PATCH 08/15] Document the CI-only PR title convention in CLAUDE.md

Standing instruction: a PR whose diff is confined entirely to
.github/workflows/ should be titled "CI: ...". Applied retroactively to
PR #29 (the only currently-open PR that qualifies).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 CLAUDE.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CLAUDE.md b/CLAUDE.md
index 9d060fc..5f2ec66 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -6,6 +6,10 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 After pushing to a branch with an open PR, monitor CI using `gh pr checks --watch` in a background subagent until all jobs pass or a failure is confirmed. Investigate and fix failures immediately rather than leaving them for the user to notice.
 
+### PR title convention for CI-only PRs
+
+A PR whose diff is confined entirely to CI configuration should have a title starting with `CI: ` (capital, colon, space). "CI-only" means literally that -- every changed file lives under `.github/workflows/`, nothing else. A PR that's CI-*motivated* but also adds/changes a real file elsewhere (a new `bin/` script a workflow calls, a linter's Makefile wiring, a test fixture) is NOT CI-only under this reading, even though CI is the reason it exists -- don't stretch the prefix to cover those. Check with `gh pr view <n> --json files --jq '.files[].path'` before applying it, don't guess from the title/description alone.
+
 ## psql Script Conventions
 
 Any `\if`/`\elsif`/`\else`/`\endif` block spanning more than ~a dozen lines needs a short comment naming the block on each of its control statements (including `\endif`), so a reader scrolling past `\else`/`\endif` on their own can tell at a glance which `\if` they belong to without scrolling back up. Put the comment on its own line immediately above the control statement, NOT trailing on the same line -- unlike SQL statements, psql's `\if`/`\else`/`\endif` don't treat a trailing `--` as a comment to strip: `\if` parses the entire rest of the line as its boolean expression (so a trailing comment breaks parsing outright), and `\else`/`\endif` parse it as an "extra argument" that gets ignored but still prints a warning into the actual output. Example:

From e91167b1be22a997a487475d2011ecb09958f516 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Wed, 5 Aug 2026 16:53:57 -0500
Subject: [PATCH 09/15] Fix the fork-checkout fix: don't redirect origin, let
 claude-code-action fetch the PR itself

The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal
error but traded it for a different, worse one: anthropics/claude-code-action
fetches and reads a PR's actual content itself (src/github/operations/
branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub
maintains on the BASE repo for any PR, fork or not). Redirecting the checkout
step's `origin` to the fork (via repository:/ref:) breaks that internal
fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote --
`fatal: couldn't find remote ref pull/<n>/head`.

Caught before it ever hit CI here by checking
Postgres-Extensions/extension_tools#28, which hit and fixed the exact same
mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28
corrected it). anthropics/claude-code-action's own docs/security.md names
this checkout pattern (checking out the PR's own untrusted ref into the
workspace) as the anti-pattern to avoid in the first place; its preferred
pattern is a plain checkout of the base ref, nothing more.

Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely
-- just `uses: actions/checkout@v7` with no inputs, checking out this repo's
own base branch. The if: trust gate is unchanged and still load-bearing
defense-in-depth, even though the checkout itself is now safe by
construction regardless of that check.

Same self-verification limitation as before: this PR's own claude-review
check runs the OLD workflow from the base branch and can't demonstrate this
on itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/workflows/claude-code-review.yml | 46 ++++++++++++------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index 0c52015..7771732 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -9,8 +9,10 @@ name: Claude Code Review
 # write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade`
 # fork only — an arbitrary external fork can never trigger this secret-bearing
 # job. The workflow file always comes from the base branch (master), so a PR
-# cannot modify the reviewer that runs on it. We check out the PR head only for
-# read context (persist-credentials: false) and never build or execute PR code.
+# cannot modify the reviewer that runs on it. This workflow never checks out
+# the PR's own ref into the workspace (see the checkout step below) --
+# claude-code-action fetches and reads the PR's content itself, safely, and
+# never builds or executes it.
 on:
   pull_request_target:
     types: [opened, synchronize, reopened, ready_for_review]
@@ -23,12 +25,9 @@ jobs:
   claude-review:
     # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login
     # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external
-    # fork's PR and this job's write-capable GITHUB_TOKEN, CLAUDE_CODE_OAUTH_TOKEN,
-    # and -- now that the checkout step below sets allow-unsafe-pr-checkout: true --
-    # a checked-out copy of that fork's own code running in this trusted context.
-    # Drop or loosen this check and the checkout step's "safe because the job is
-    # already gated to a trusted fork" justification stops being true, turning this
-    # into a textbook "pwn request" vulnerability. To trust an additional fork,
+    # fork's PR and this job's write-capable GITHUB_TOKEN and
+    # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and any fork can trigger
+    # a job that runs with this repo's secrets. To trust an additional fork,
     # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) --
     # never replace it with something broader (a wildcard, a check on PR author
     # instead of head repo owner, etc.). Skips drafts too (don't spend API/CI on
@@ -84,25 +83,26 @@ jobs:
           echo "decision=$decision" >> "$GITHUB_OUTPUT"
           echo "gate decision: $decision"
 
-      - name: Check out PR head (read-only context)
+      - name: Check out base branch
         if: steps.gate.outputs.decision == 'run'
+        # Deliberately NO ref:/repository: override -- this checks out this
+        # repo's own base branch (master), not the PR's fork/ref. Checking
+        # out an untrusted PR ref into the workspace root before this action
+        # is exactly the anti-pattern anthropics/claude-code-action's own
+        # docs/security.md warns against; its "preferred" pattern is a plain
+        # checkout of the base ref, nothing more. claude-code-action fetches
+        # and reviews the PR's actual content itself, from its own internal
+        # logic (src/github/operations/branch.ts): for a fork PR it fetches
+        # origin's refs/pull/<n>/head -- a ref GitHub maintains on THIS repo
+        # for any PR, fork or not, so it never needs direct access to the
+        # fork's own remote at all. That's why this step must leave `origin`
+        # pointing at this repo (the default) rather than being redirected to
+        # the fork: an earlier version of this step did that, which broke the
+        # action's own internal fetch ("couldn't find remote ref
+        # pull/<n>/head") since that ref doesn't exist on the fork.
         # Intentionally tracks the major-version tag (not a pinned SHA) so
         # upstream fixes are picked up automatically.
         uses: actions/checkout@v7
-        with:
-          repository: ${{ github.event.pull_request.head.repo.full_name }}
-          ref: ${{ github.event.pull_request.head.sha }}
-          fetch-depth: 1
-          persist-credentials: false
-          # actions/checkout v4.4.0+ (backported to every floating major tag)
-          # refuses to check out a fork PR's head under pull_request_target by
-          # default ("Refusing to check out fork pull request code..."). Safe to
-          # opt in here specifically because BOTH halves of the two-layer
-          # defense this needs are already true: the job-level trust gate above
-          # restricts this to PRs from the trusted jnasbyupgrade fork only, and
-          # the checked-out code is read-only from here on (persist-credentials:
-          # false, fed only to the review action below, never built or run).
-          allow-unsafe-pr-checkout: true
 
       - name: Run Claude Code Review
         if: steps.gate.outputs.decision == 'run'

From e1104890e79f07d4056da96750cb2758cd2dcdd9 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Wed, 5 Aug 2026 17:16:25 -0500
Subject: [PATCH 10/15] Add actions:write permission and --allowedTools for
 inline comments

Two more gotchas from the updated ~/security-notice.md handoff doc
(Postgres-Extensions/cat_tools PR #62 and #47), both silent failure modes
that don't change the job's overall pass/fail status:

- claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'
  A bare `prompt:` (no @claude mention) runs claude-code-action in "agent
  mode", which decides which MCP servers to start from --allowedTools inside
  claude_args, NOT from the invoked plugin's own allowed-tools frontmatter.
  Without this, the inline-comment MCP server never starts at all, so the
  code-review plugin silently falls back to one consolidated PR comment
  instead of real per-line inline comments -- no error, no warning, every
  review before this fix used the wrong output shape.

- permissions: actions: write
  There is no narrower scope for cache-write alone. Without it, any
  cache-save claude-code-action's own internal setup does silently fails
  ("Cache reservation failed: cache write denied: token has no writable
  scopes") -- a warning, not a hard failure, so this was invisible from the
  job's pass/fail status alone.

Same self-verification limitation as the checkout fixes in this file's
other recent commits: neither of these is visible from this PR's own
claude-review check (which runs the OLD workflow from the base branch, and
in this stack's case is also upstream-headed so the trust gate skips it
entirely regardless).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/workflows/claude-code-review.yml | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index 7771732..514c63a 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -41,6 +41,13 @@ jobs:
       contents: read
       pull-requests: write   # post the review comments
       checks: read           # read sibling check-runs for the cost gate
+      # No narrower scope exists for cache-write alone; without this,
+      # claude-code-action's own internal setup silently fails to save its
+      # Actions cache ("Cache reservation failed: cache write denied: token
+      # has no writable scopes") -- a warning, not a hard failure, so the job
+      # still completes and looks fine, just slower/uncached every run. Don't
+      # try to "tighten" this down to something narrower; it doesn't exist.
+      actions: write
     steps:
       # COST GATE: the paid Claude review is the last thing to run. Wait for the
       # PR head's OTHER check-runs to finish and only proceed if they are clean.
@@ -118,6 +125,16 @@ jobs:
           # marketplace repo's default branch (upstream anthropics/claude-code).
           plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
           plugins: 'code-review@claude-code-plugins'
+          # A bare prompt: (no @claude mention) runs claude-code-action in
+          # "agent mode", which decides which MCP servers to start by
+          # scanning --allowedTools inside claude_args -- it does NOT consult
+          # the invoked plugin's own allowed-tools frontmatter. Without this,
+          # mcp__github_inline_comment__create_inline_comment never starts
+          # (not "exists but blocked" -- genuinely absent), so the
+          # code-review plugin silently falls back to one consolidated PR
+          # comment instead of real per-line inline comments. No error, no
+          # warning -- every review just quietly uses the wrong output shape.
+          claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'
           # --comment is required: without it, the code-review plugin only
           # prints its findings to the job log and never posts anything to
           # the PR (confirmed by capturing the hidden SDK transcript on a

From 7a6623887b075c4ddf59a03cc608c5b262e74135 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Wed, 5 Aug 2026 17:17:26 -0500
Subject: [PATCH 11/15] claude.yml: actions: read -> write for the same
 cache-permission gap

Same anthropics/claude-code-action cache-write gotcha as the previous
commit's claude-code-review.yml fix (Postgres-Extensions/cat_tools PR #47) --
this workflow calls the same action, so its own internal setup hits the
same silent "Cache reservation failed" warning without actions: write.
`write` still covers the existing "read CI results" need (write implies
read here), so this replaces the read with write rather than adding a
duplicate key.

claude.yml does NOT need the --allowedTools inline-comments fix from the
previous commit: it never sets prompt: (it responds to actual @claude
mention text, not agent mode), so that specific gotcha doesn't apply here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 .github/workflows/claude.yml | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml
index dc559d1..4fa242b 100644
--- a/.github/workflows/claude.yml
+++ b/.github/workflows/claude.yml
@@ -31,7 +31,14 @@ jobs:
       pull-requests: read
       issues: read
       id-token: write
-      actions: read # Required for Claude to read CI results on PRs
+      # write (not just read) despite the name: `write` includes read access
+      # to CI results, AND there's no narrower scope for cache-write alone --
+      # without it, claude-code-action's own internal setup silently fails to
+      # save its Actions cache ("Cache reservation failed: cache write
+      # denied: token has no writable scopes"), a warning not a hard
+      # failure, so the job still completes and looks fine, just
+      # slower/uncached every run. Don't "tighten" this back down to `read`.
+      actions: write
     steps:
       - name: Checkout repository
         # Intentionally tracks the major-version tag (not a pinned SHA) so

From 02a90a2f12ed0116f63a8b0f9ba7b3d5043b14ab Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 6 Aug 2026 16:25:14 -0500
Subject: [PATCH 12/15] Regenerate base.out/pgtap.out for foundation's
 install/dependency rework

Rebase fallout from advanced-testing/foundation's redesign (test/install/
load.sql now owns installation in every mode; test/sql/pgtap.sql's
dependency check moved to pg_depend inspection) -- same content, reordered
by this branch's own pre-existing ORDER BY fix on the security-definer
function query.
---
 test/expected/base.out  | 10 +++++-----
 test/expected/pgtap.out | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/test/expected/base.out b/test/expected/base.out
index 75a9c3a..7af44d8 100644
--- a/test/expected/base.out
+++ b/test/expected/base.out
@@ -3,11 +3,11 @@ ok 1 - Register test customers
 ok 2 - Create function customer__add
 ok 3 - Register test invoices
 ok 4 - Ensure original_role temp table was dropped
-ok 5 - Security definer function _tf.schema__getsert has search_path=pg_catalog
-ok 6 - Security definer function _tf.test_factory__get has search_path=pg_catalog
-ok 7 - Security definer function _tf.test_factory__set has search_path=pg_catalog
-ok 8 - Security definer function _tf.table_create has search_path=pg_catalog
-ok 9 - Security definer function _tf.get has search_path=pg_catalog
+ok 5 - Security definer function _tf.get has search_path=pg_catalog
+ok 6 - Security definer function _tf.schema__getsert has search_path=pg_catalog
+ok 7 - Security definer function _tf.table_create has search_path=pg_catalog
+ok 8 - Security definer function _tf.test_factory__get has search_path=pg_catalog
+ok 9 - Security definer function _tf.test_factory__set has search_path=pg_catalog
 ok 10 - customer table is empty
 ok 11 - invoice table is empty
 ok 12 - invoice factory output
diff --git a/test/expected/pgtap.out b/test/expected/pgtap.out
index 2e529f2..1793e6e 100644
--- a/test/expected/pgtap.out
+++ b/test/expected/pgtap.out
@@ -4,11 +4,11 @@ ok 2 - Register test customers
 ok 3 - Create function customer__add
 ok 4 - Register test invoices
 ok 5 - Ensure original_role temp table was dropped
-ok 6 - Security definer function _tf.schema__getsert has search_path=pg_catalog
-ok 7 - Security definer function _tf.test_factory__get has search_path=pg_catalog
-ok 8 - Security definer function _tf.test_factory__set has search_path=pg_catalog
-ok 9 - Security definer function _tf.table_create has search_path=pg_catalog
-ok 10 - Security definer function _tf.get has search_path=pg_catalog
+ok 6 - Security definer function _tf.get has search_path=pg_catalog
+ok 7 - Security definer function _tf.schema__getsert has search_path=pg_catalog
+ok 8 - Security definer function _tf.table_create has search_path=pg_catalog
+ok 9 - Security definer function _tf.test_factory__get has search_path=pg_catalog
+ok 10 - Security definer function _tf.test_factory__set has search_path=pg_catalog
 ok 11 - Get test data set "base" for table invoice
 ok 12 - Get test data set "base" for table invoice
 ok 13 - Ensure we get sane error for a non-existent table

From 56177320eb8ee10c7bdf57066ba26d7974c49b6d Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 6 Aug 2026 18:46:36 -0500
Subject: [PATCH 13/15] claude-code-review.yml: gate on PR author, not head
 repo owner

head.repo.owner.login only distinguishes "who owns the fork" for
fork-headed PRs. For an upstream-branch-headed PR (base and head both in
this repo -- what `gh stack` requires), head.repo.owner.login is always
this repo's own org, never the actual author, so the gate silently skipped
review on every PR in this session's stack regardless of who opened it.

Confirmed via the check-runs API that claude-review's conclusion was
"skipped" on PRs #33/#34/#35 -- all legitimately jnasbyupgrade's own work,
recreated as upstream-branch PRs specifically so `gh stack` could link
them.

PR author can't be spoofed by a third party any more than head repo owner
can, and it's the more direct question for this gate's actual purpose:
trusting the PERSON asking for a review to run with this repo's secrets,
not the repository their branch happens to live in.
---
 .github/workflows/claude-code-review.yml | 46 +++++++++++++++---------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index 514c63a..d77f74b 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -6,13 +6,13 @@ name: Claude Code Review
 # `pull_request` version never worked for fork PRs.
 #
 # SECURITY: pull_request_target runs in the BASE repo with secrets and a
-# write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade`
-# fork only — an arbitrary external fork can never trigger this secret-bearing
-# job. The workflow file always comes from the base branch (master), so a PR
-# cannot modify the reviewer that runs on it. This workflow never checks out
-# the PR's own ref into the workspace (see the checkout step below) --
-# claude-code-action fetches and reads the PR's content itself, safely, and
-# never builds or executes it.
+# write-capable token. The job is gated to PRs authored by the trusted
+# `jnasbyupgrade` account only — an arbitrary external actor can never
+# trigger this secret-bearing job. The workflow file always comes from the
+# base branch (master), so a PR cannot modify the reviewer that runs on it.
+# This workflow never checks out the PR's own ref into the workspace (see the
+# checkout step below) -- claude-code-action fetches and reads the PR's
+# content itself, safely, and never builds or executes it.
 on:
   pull_request_target:
     types: [opened, synchronize, reopened, ready_for_review]
@@ -23,18 +23,30 @@ concurrency:
 
 jobs:
   claude-review:
-    # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE head.repo.owner.login
-    # CHECK BELOW !!! It is the ONLY thing standing between an arbitrary external
-    # fork's PR and this job's write-capable GITHUB_TOKEN and
-    # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and any fork can trigger
-    # a job that runs with this repo's secrets. To trust an additional fork,
-    # EXTEND this condition explicitly (e.g. `|| ... == 'other-trusted-account'`) --
-    # never replace it with something broader (a wildcard, a check on PR author
-    # instead of head repo owner, etc.). Skips drafts too (don't spend API/CI on
-    # unfinished PRs).
+    # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE user.login CHECK
+    # BELOW !!! It is the ONLY thing standing between an arbitrary external
+    # actor's PR and this job's write-capable GITHUB_TOKEN and
+    # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and anyone can trigger
+    # a job that runs with this repo's secrets. To trust an additional
+    # account, EXTEND this condition explicitly (e.g. `|| ... ==
+    # 'other-trusted-account'`) -- never replace it with something broader
+    # (a wildcard, etc.).
+    #
+    # Checks PR AUTHOR (github.event.pull_request.user.login), not head repo
+    # owner: an earlier version of this check used head.repo.owner.login,
+    # which only works for fork-headed PRs -- for an upstream-branch-headed
+    # PR (e.g. one opened for `gh stack`, base and head both in this repo),
+    # head.repo.owner.login is always this repo's OWN org, never the actual
+    # author, so that check silently skipped review on every such PR
+    # regardless of who opened it (caught when review kept skipping on a
+    # whole PR stack that was legitimately jnasbyupgrade's own work). PR
+    # author can't be spoofed by a third party either way, and it's the
+    # right question here anyway -- we're trusting the PERSON asking for a
+    # review to run, not the repository their branch happens to live in.
+    # Skips drafts too (don't spend API/CI on unfinished PRs).
     if: >-
       github.event.pull_request.draft == false &&
-      github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade'
+      github.event.pull_request.user.login == 'jnasbyupgrade'
     runs-on: ubuntu-latest
     timeout-minutes: 60
     permissions:

From b0f30ca67902a232925aaf9f26fa60abd9566a81 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Thu, 6 Aug 2026 19:20:14 -0500
Subject: [PATCH 14/15] claude-code-review.yml: fold in PR #38's phrasing on
 author-check safety

PR #38 (Postgres-Extensions/test_factory, part of a 7-repo sweep for this
same trust-gate bug) independently landed the identical user.login fix
targeting master directly. Closing #38 in favor of this stack's copy since
it'll reach master when the stack merges anyway -- but its phrasing on why
user.login can't be spoofed ("by PR content" specifically) is worth folding
in here, since PR content is untrusted input in this exact threat model.
---
 .github/workflows/claude-code-review.yml | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index d77f74b..4ca4afc 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -39,10 +39,13 @@ jobs:
     # head.repo.owner.login is always this repo's OWN org, never the actual
     # author, so that check silently skipped review on every such PR
     # regardless of who opened it (caught when review kept skipping on a
-    # whole PR stack that was legitimately jnasbyupgrade's own work). PR
-    # author can't be spoofed by a third party either way, and it's the
-    # right question here anyway -- we're trusting the PERSON asking for a
-    # review to run, not the repository their branch happens to live in.
+    # whole PR stack that was legitimately jnasbyupgrade's own work).
+    # user.login is the PR's original author and can't be spoofed by PR
+    # content (unlike, say, a string embedded in the PR body or a commit
+    # message), so this check holds regardless of whether the PR head lives
+    # in this repo or an external fork -- it's the right question here
+    # anyway: we're trusting the PERSON asking for a review to run, not the
+    # repository their branch happens to live in.
     # Skips drafts too (don't spend API/CI on unfinished PRs).
     if: >-
       github.event.pull_request.draft == false &&

From 3b0d906897eb019415cee6bd5b7351291b8b91a6 Mon Sep 17 00:00:00 2001
From: jnasbyupgrade <jnasby@upgrade.com>
Date: Fri, 7 Aug 2026 18:55:34 -0500
Subject: [PATCH 15/15] claude-code-review.yml: restore persist-credentials:
 false on the checkout step

Lost when the fork-checkout override (repository:/ref:) was removed from
this step's with: block earlier -- deleting the whole block wholesale threw
away persist-credentials: false along with it, silently reverting to
actions/checkout's default of true. This job's permissions grant
pull-requests: write, a real write-capable credential; nothing here
legitimately runs git push, so there's no reason to leave it persisted in
.git/config for the rest of the job to misuse if anything later goes wrong.

Found via the same pattern in Postgres-Extensions/object_reference#27 (see
~/more-fixes.md), where the same wholesale with:-block deletion had the
same effect; pg_count_nulls#53's version of this fix got it right by
removing only repository:/ref: and keeping fetch-depth/persist-credentials.
---
 .github/workflows/claude-code-review.yml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
index 4ca4afc..f86281f 100644
--- a/.github/workflows/claude-code-review.yml
+++ b/.github/workflows/claude-code-review.yml
@@ -125,6 +125,14 @@ jobs:
         # Intentionally tracks the major-version tag (not a pinned SHA) so
         # upstream fixes are picked up automatically.
         uses: actions/checkout@v7
+        with:
+          # This job's permissions include pull-requests: write, a real
+          # write-capable credential -- nothing here legitimately runs `git
+          # push` (review comments post via the API/claude-code-action, not
+          # git), so there's no reason to leave that credential sitting in
+          # .git/config for the rest of the job to misuse if anything later
+          # goes wrong.
+          persist-credentials: false
 
       - name: Run Claude Code Review
         if: steps.gate.outputs.decision == 'run'