From 4b91878d775f5b987b680c9d5e592a944f7623dc Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 27 Jul 2026 16:43:09 -0500 Subject: [PATCH 01/12] Add tests for pgxntool-version.sh and the pgxntool-version make target Covers HISTORY.asc parsing (stamped version, STABLE/unreleased checkout, missing file, empty file, default path) in isolation, plus an integration test that make pgxntool-version matches the embedded pgxntool/HISTORY.asc inside a real foundation repo. Also adds pgxntool/pgxntool-version.sh to dist-expected-files.txt, since pgxntool now ships that new script. Co-Authored-By: Claude Sonnet 5 --- test/lib/dist-expected-files.txt | 1 + test/standard/pgxntool-version.bats | 80 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 test/standard/pgxntool-version.bats diff --git a/test/lib/dist-expected-files.txt b/test/lib/dist-expected-files.txt index 7f4d5c7..f6751a6 100644 --- a/test/lib/dist-expected-files.txt +++ b/test/lib/dist-expected-files.txt @@ -80,6 +80,7 @@ pgxntool/run-test-build.sh pgxntool/safesed pgxntool/setup.sh pgxntool/pgxntool-sync.sh +pgxntool/pgxntool-version.sh pgxntool/update-setup-files.sh pgxntool/verify-results-pgtap.sh pgxntool/WHAT_IS_THIS diff --git a/test/standard/pgxntool-version.bats b/test/standard/pgxntool-version.bats new file mode 100644 index 0000000..1436e60 --- /dev/null +++ b/test/standard/pgxntool-version.bats @@ -0,0 +1,80 @@ +#!/usr/bin/env bats + +# Test: pgxntool-version +# +# Tests pgxntool-version.sh's HISTORY.asc parsing in isolation (a stamped +# version, an unreleased "STABLE" checkout, a missing file, an empty file), +# plus that `make pgxntool-version` is correctly wired to it inside a real +# embedded pgxntool copy. + +load ../lib/helpers + +setup_file() { + setup_topdir + + # Independent test - gets its own isolated environment with foundation TEST_REPO + load_test_env "pgxntool-version" + ensure_foundation "$TEST_DIR" + + export VERSION_SCRIPT="$TOPDIR/../pgxntool/pgxntool-version.sh" + export SCRATCH_DIR="$TEST_DIR/pgxntool-version-tests" +} + +setup() { + load_test_env "pgxntool-version" + + rm -rf "$SCRATCH_DIR" + mkdir -p "$SCRATCH_DIR" +} + +@test "pgxntool-version.sh prints a stamped version number" { + echo "2.1.0" > "$SCRATCH_DIR/HISTORY.asc" + + run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" + assert_success + [ "$output" = "2.1.0" ] +} + +@test "pgxntool-version.sh prints STABLE for an unreleased checkout" { + printf 'STABLE\n------\n' > "$SCRATCH_DIR/HISTORY.asc" + + run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" + assert_success + [ "$output" = "STABLE" ] +} + +@test "pgxntool-version.sh errors on a missing HISTORY.asc" { + run "$VERSION_SCRIPT" "$SCRATCH_DIR/does-not-exist.asc" + assert_failure + assert_contains "$output" "not found" +} + +@test "pgxntool-version.sh errors on an empty HISTORY.asc" { + : > "$SCRATCH_DIR/HISTORY.asc" + + run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" + assert_failure + assert_contains "$output" "empty" +} + +@test "pgxntool-version.sh defaults to its own directory's HISTORY.asc" { + run "$VERSION_SCRIPT" + assert_success + [ -n "$output" ] +} + +@test "make pgxntool-version matches the embedded pgxntool/HISTORY.asc" { + cd "$TEST_REPO" + + local expected + expected=$(head -n1 pgxntool/HISTORY.asc) + + # --no-print-directory: our own test suite runs under `make test-all`, which + # propagates MAKEFLAGS/MAKELEVEL to this nested `make` call and would + # otherwise wrap the output in "make[1]: Entering/Leaving directory" banners. + run make --no-print-directory pgxntool-version + assert_success + [ "$output" = "$expected" ] +} + +# vi: expandtab sw=2 ts=2 From 2c32ce2d8f3cc9a5e0d65da9411b0df5e6e18285 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 27 Jul 2026 17:11:33 -0500 Subject: [PATCH 02/12] Test that pgxntool-version.sh rejects malformed HISTORY.asc lines Covers a lowercase 'stable', a v-prefixed version, a pre-release suffix, a truncated X.Y version, and an arbitrary non-version string. Co-Authored-By: Claude Sonnet 5 --- test/standard/pgxntool-version.bats | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/standard/pgxntool-version.bats b/test/standard/pgxntool-version.bats index 1436e60..bedd02d 100644 --- a/test/standard/pgxntool-version.bats +++ b/test/standard/pgxntool-version.bats @@ -3,9 +3,9 @@ # Test: pgxntool-version # # Tests pgxntool-version.sh's HISTORY.asc parsing in isolation (a stamped -# version, an unreleased "STABLE" checkout, a missing file, an empty file), -# plus that `make pgxntool-version` is correctly wired to it inside a real -# embedded pgxntool copy. +# version, an unreleased "STABLE" checkout, a missing file, an empty file, +# malformed first lines), plus that `make pgxntool-version` is correctly +# wired to it inside a real embedded pgxntool copy. load ../lib/helpers @@ -57,6 +57,16 @@ setup() { assert_contains "$output" "empty" } +@test "pgxntool-version.sh rejects a malformed first line" { + for bad in "stable" "v2.1.0" "2.1.0-beta" "2.1" "not a version"; do + echo "$bad" > "$SCRATCH_DIR/HISTORY.asc" + + run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" + assert_failure + assert_contains "$output" "neither STABLE nor a X.Y.Z version" + done +} + @test "pgxntool-version.sh defaults to its own directory's HISTORY.asc" { run "$VERSION_SCRIPT" assert_success From 00a17d780b2b89b47c32f978f58fd06af49ef3f1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 27 Jul 2026 18:32:18 -0500 Subject: [PATCH 03/12] Strengthen pgxntool-version test; add release-skill sanity check Mark the test that runs pgxntool-version.sh against the real, unmodified ../pgxntool checkout as CRITICAL, and strengthen it to an exact match against HISTORY.asc's first line instead of a non-empty check. This is the test CI relies on to catch a release PR that stamps HISTORY.asc incorrectly, especially once the top line is no longer STABLE. Also add a Step 7 to the /release skill that exercises the real 'make pgxntool-version' target (via a symlinked scratch dir, not a copy) against the just-stamped HISTORY.asc before tagging and pushing, as a safeguard against a bad stamp shipping. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 35 +++++++++++++++++++++++++---- test/standard/pgxntool-version.bats | 23 +++++++++++++++---- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 972d306..c69ee26 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -187,7 +187,34 @@ Summarize what was found and how each item was resolved before moving on. cd ../pgxntool && git commit -am "Stamp VERSION" ``` -## Step 7: Tag and Push pgxntool +## Step 7: Sanity-Check pgxntool-version Output + +CI depends on `pgxntool-version.sh` (see `../pgxntool/pgxntool-version.sh`, +wired up as `make pgxntool-version`) correctly reflecting whatever is +actually on the first line of `HISTORY.asc` -- that's what lets a release PR +be caught in CI if the stamp in Step 6 didn't take effect the way it should +have. Exercise the real `make pgxntool-version` target (not just the script) +against the just-stamped, unmodified `../pgxntool` checkout: + +```bash +scratch=$(mktemp -d) +ln -s "$(cd ../pgxntool && pwd)" "$scratch/pgxntool" +echo 'include pgxntool/base.mk' > "$scratch/Makefile" +(cd "$scratch" && make --no-print-directory pgxntool-version) +rm -rf "$scratch" +``` + +Expect a harmless `ERROR: Usage: control.mk.sh ...` / `make: *** Deleting +file 'control.mk'` line before the real output -- the scratch dir has no +`.control` files, so `base.mk`'s `-include control.mk` fails to build it and +make silently continues (same class of noise the `make list` trick above +produces). **The last line printed must be exactly VERSION.** If it's +`STABLE`, an old version, or an error instead, the Step 6 edit didn't take +effect correctly (or `pgxntool-version.sh`/`base.mk` themselves regressed). +Stop and fix it -- do not tag or push a release that `make pgxntool-version` +doesn't agree with. + +## Step 8: Tag and Push pgxntool **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** @@ -198,7 +225,7 @@ git push PGXNTOOL_UPSTREAM master git push PGXNTOOL_UPSTREAM VERSION ``` -## Step 8: Stamp, Tag, and Push pgxntool-test +## Step 9: Stamp, Tag, and Push pgxntool-test **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** @@ -212,7 +239,7 @@ git push PGXNTOOL_TEST_UPSTREAM master git push PGXNTOOL_TEST_UPSTREAM VERSION ``` -## Step 9: Update `release` Tag +## Step 10: Update `release` Tag Both repos have a `release` tag on upstream that must always point to the latest release. This is a moving tag that requires force-push to update. @@ -229,7 +256,7 @@ git tag -f release VERSION git push PGXNTOOL_TEST_UPSTREAM -f refs/tags/release ``` -## Step 10: Verify and Report +## Step 11: Verify and Report ```bash cd ../pgxntool && git checkout master diff --git a/test/standard/pgxntool-version.bats b/test/standard/pgxntool-version.bats index bedd02d..59035c2 100644 --- a/test/standard/pgxntool-version.bats +++ b/test/standard/pgxntool-version.bats @@ -4,8 +4,10 @@ # # Tests pgxntool-version.sh's HISTORY.asc parsing in isolation (a stamped # version, an unreleased "STABLE" checkout, a missing file, an empty file, -# malformed first lines), plus that `make pgxntool-version` is correctly -# wired to it inside a real embedded pgxntool copy. +# malformed first lines), that `make pgxntool-version` is correctly wired to +# it inside a real embedded pgxntool copy, and (CRITICAL -- see comment +# below) that it matches the actual, unmodified pgxntool checkout this test +# suite is running against. load ../lib/helpers @@ -67,10 +69,23 @@ setup() { done } -@test "pgxntool-version.sh defaults to its own directory's HISTORY.asc" { +# CRITICAL: this test must run pgxntool-version.sh directly against the real, +# unmodified $TOPDIR/../pgxntool checkout -- never a scratch file, an rsync'd +# copy, or a cached foundation snapshot. A release PR stamps HISTORY.asc with +# the real version and relies on CI running this exact check to catch a bad +# stamp before it's tagged and pushed -- that guarantee only holds if the +# check reads the literal, current pgxntool/HISTORY.asc. This matters most +# precisely when the top line is NOT "STABLE": that's the release commit +# itself, and a copy-based or stale check could pass against old content and +# let a wrong version ship. Do not weaken this to a scratch-file test or to a +# non-empty check. +@test "pgxntool-version.sh matches the current pgxntool/HISTORY.asc with no modifications" { + local expected + expected=$(head -n1 "$TOPDIR/../pgxntool/HISTORY.asc") + run "$VERSION_SCRIPT" assert_success - [ -n "$output" ] + [ "$output" = "$expected" ] } @test "make pgxntool-version matches the embedded pgxntool/HISTORY.asc" { From 68c12b04bbeb5b1028937d82b3c9740db8e665d4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 13:32:07 -0500 Subject: [PATCH 04/12] Update pgxntool-version tests and release skill for bin/version rename pgxntool-version.sh was renamed to bin/version (companion change in pgxntool PR #73). Update the test script path, dist manifest entry, and simplify the release skill's Step 7 to call the script directly instead of exercising it through a throwaway scratch Makefile. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 30 ++++++++++------------------- test/lib/dist-expected-files.txt | 3 ++- test/standard/pgxntool-version.bats | 18 ++++++++--------- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index c69ee26..b94254b 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -189,30 +189,20 @@ Summarize what was found and how each item was resolved before moving on. ## Step 7: Sanity-Check pgxntool-version Output -CI depends on `pgxntool-version.sh` (see `../pgxntool/pgxntool-version.sh`, -wired up as `make pgxntool-version`) correctly reflecting whatever is -actually on the first line of `HISTORY.asc` -- that's what lets a release PR -be caught in CI if the stamp in Step 6 didn't take effect the way it should -have. Exercise the real `make pgxntool-version` target (not just the script) -against the just-stamped, unmodified `../pgxntool` checkout: +CI depends on `bin/version` (see `../pgxntool/bin/version`, also wired up as +`make pgxntool-version`) correctly reflecting whatever is actually on the +first line of `HISTORY.asc` -- that's what lets a release PR be caught in CI +if the stamp in Step 6 didn't take effect the way it should have. Run it +directly against the just-stamped, unmodified `../pgxntool` checkout: ```bash -scratch=$(mktemp -d) -ln -s "$(cd ../pgxntool && pwd)" "$scratch/pgxntool" -echo 'include pgxntool/base.mk' > "$scratch/Makefile" -(cd "$scratch" && make --no-print-directory pgxntool-version) -rm -rf "$scratch" +../pgxntool/bin/version ``` -Expect a harmless `ERROR: Usage: control.mk.sh ...` / `make: *** Deleting -file 'control.mk'` line before the real output -- the scratch dir has no -`.control` files, so `base.mk`'s `-include control.mk` fails to build it and -make silently continues (same class of noise the `make list` trick above -produces). **The last line printed must be exactly VERSION.** If it's -`STABLE`, an old version, or an error instead, the Step 6 edit didn't take -effect correctly (or `pgxntool-version.sh`/`base.mk` themselves regressed). -Stop and fix it -- do not tag or push a release that `make pgxntool-version` -doesn't agree with. +**The output must be exactly VERSION.** If it's `STABLE`, an old version, or +an error instead, the Step 6 edit didn't take effect correctly (or +`bin/version` itself regressed). Stop and fix it -- do not tag or push a +release that doesn't agree with this. ## Step 8: Tag and Push pgxntool diff --git a/test/lib/dist-expected-files.txt b/test/lib/dist-expected-files.txt index f6751a6..21df56f 100644 --- a/test/lib/dist-expected-files.txt +++ b/test/lib/dist-expected-files.txt @@ -66,6 +66,8 @@ test/sql/verify_install_marker.sql pgxntool/ pgxntool/_.gitignore pgxntool/.gitignore +pgxntool/bin/ +pgxntool/bin/version pgxntool/base.mk pgxntool/build_meta.sh pgxntool/JSON.sh @@ -80,7 +82,6 @@ pgxntool/run-test-build.sh pgxntool/safesed pgxntool/setup.sh pgxntool/pgxntool-sync.sh -pgxntool/pgxntool-version.sh pgxntool/update-setup-files.sh pgxntool/verify-results-pgtap.sh pgxntool/WHAT_IS_THIS diff --git a/test/standard/pgxntool-version.bats b/test/standard/pgxntool-version.bats index 59035c2..5fe121f 100644 --- a/test/standard/pgxntool-version.bats +++ b/test/standard/pgxntool-version.bats @@ -2,7 +2,7 @@ # Test: pgxntool-version # -# Tests pgxntool-version.sh's HISTORY.asc parsing in isolation (a stamped +# Tests bin/version's HISTORY.asc parsing in isolation (a stamped # version, an unreleased "STABLE" checkout, a missing file, an empty file, # malformed first lines), that `make pgxntool-version` is correctly wired to # it inside a real embedded pgxntool copy, and (CRITICAL -- see comment @@ -18,7 +18,7 @@ setup_file() { load_test_env "pgxntool-version" ensure_foundation "$TEST_DIR" - export VERSION_SCRIPT="$TOPDIR/../pgxntool/pgxntool-version.sh" + export VERSION_SCRIPT="$TOPDIR/../pgxntool/bin/version" export SCRATCH_DIR="$TEST_DIR/pgxntool-version-tests" } @@ -29,7 +29,7 @@ setup() { mkdir -p "$SCRATCH_DIR" } -@test "pgxntool-version.sh prints a stamped version number" { +@test "bin/version prints a stamped version number" { echo "2.1.0" > "$SCRATCH_DIR/HISTORY.asc" run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" @@ -37,7 +37,7 @@ setup() { [ "$output" = "2.1.0" ] } -@test "pgxntool-version.sh prints STABLE for an unreleased checkout" { +@test "bin/version prints STABLE for an unreleased checkout" { printf 'STABLE\n------\n' > "$SCRATCH_DIR/HISTORY.asc" run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" @@ -45,13 +45,13 @@ setup() { [ "$output" = "STABLE" ] } -@test "pgxntool-version.sh errors on a missing HISTORY.asc" { +@test "bin/version errors on a missing HISTORY.asc" { run "$VERSION_SCRIPT" "$SCRATCH_DIR/does-not-exist.asc" assert_failure assert_contains "$output" "not found" } -@test "pgxntool-version.sh errors on an empty HISTORY.asc" { +@test "bin/version errors on an empty HISTORY.asc" { : > "$SCRATCH_DIR/HISTORY.asc" run "$VERSION_SCRIPT" "$SCRATCH_DIR/HISTORY.asc" @@ -59,7 +59,7 @@ setup() { assert_contains "$output" "empty" } -@test "pgxntool-version.sh rejects a malformed first line" { +@test "bin/version rejects a malformed first line" { for bad in "stable" "v2.1.0" "2.1.0-beta" "2.1" "not a version"; do echo "$bad" > "$SCRATCH_DIR/HISTORY.asc" @@ -69,7 +69,7 @@ setup() { done } -# CRITICAL: this test must run pgxntool-version.sh directly against the real, +# CRITICAL: this test must run bin/version directly against the real, # unmodified $TOPDIR/../pgxntool checkout -- never a scratch file, an rsync'd # copy, or a cached foundation snapshot. A release PR stamps HISTORY.asc with # the real version and relies on CI running this exact check to catch a bad @@ -79,7 +79,7 @@ setup() { # itself, and a copy-based or stale check could pass against old content and # let a wrong version ship. Do not weaken this to a scratch-file test or to a # non-empty check. -@test "pgxntool-version.sh matches the current pgxntool/HISTORY.asc with no modifications" { +@test "bin/version matches the current pgxntool/HISTORY.asc with no modifications" { local expected expected=$(head -n1 "$TOPDIR/../pgxntool/HISTORY.asc") From 05d86a0ef151e38ae1b65aa363c56ceea08b4cfe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 14:15:12 -0500 Subject: [PATCH 05/12] release skill: drop step numbers, add HISTORY reorder step, encourage parallel subagents Numbered steps (Step 2, Step 6, ...) required hand-renumbering every cross-reference whenever a step was added, removed, or reordered -- exactly what happened when this same commit added a HISTORY.asc reordering step. Steps are now referenced by their heading name instead, which stays correct no matter how the list changes. Also adds a step to sort each release's STABLE entries by importance (breaking changes, then non-breaking behavior changes, then new features, then bugfixes) before stamping, and a Process Notes section noting where independent work (the two review agents, and the two repos' tag/push steps) can run as parallel subagents instead of sequentially. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 176 +++++++++++++++++++++----------- 1 file changed, 114 insertions(+), 62 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index b94254b..47fb2e4 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -18,15 +18,16 @@ Create a release for pgxntool and pgxntool-test. - **STABLE section**: The heading in `HISTORY.asc` where unreleased changes are documented. During a release, this heading is replaced with the version number. This has nothing to do with git branches. - **UPSTREAM_REMOTE**: The local git remote pointing to the main project repos at `https://github.com/Postgres-Extensions/`. Releases must be pushed here -- never to a fork. The remote name varies; it is identified by URL pattern in the pre-flight script. -- **User-facing API surface**: what the Step 2 review agents treat as "the - documented API" of pgxntool. The canonical, evolving definition lives in - this repo's own `CLAUDE.md`, under "User-Facing API Surface of pgxntool" - (not `../pgxntool/CLAUDE.md` — that file is user-facing docs for - extension developers, not dev/audit tooling docs). Read that section - fresh before launching the Step 2 agents and give them its current text - verbatim as their scope, since it's expected to change over time. When a - reviewer hits a case that doesn't clearly fit, flag it for the user - rather than guessing, and consider updating that section afterward. +- **User-facing API surface**: what the review agents launched in **Launch + API Documentation Review Agents** treat as "the documented API" of + pgxntool. The canonical, evolving definition lives in this repo's own + `CLAUDE.md`, under "User-Facing API Surface of pgxntool" (not + `../pgxntool/CLAUDE.md` — that file is user-facing docs for extension + developers, not dev/audit tooling docs). Read that section fresh before + launching those agents and give them its current text verbatim as their + scope, since it's expected to change over time. When a reviewer hits a + case that doesn't clearly fit, flag it for the user rather than guessing, + and consider updating that section afterward. - **Discovering make targets**: the definition's target list should be found with `make list` (a target pgxntool itself provides — see `base.mk`), not by grepping for target definitions, since pattern rules @@ -42,9 +43,31 @@ Create a release for pgxntool and pgxntool-test. `test/install/*.sql` exist) won't show up this way. Read `base.mk` directly for these rather than relying on `make list` alone. +## Process Notes + +- **Steps below are headings, not numbers.** Earlier versions of this skill + numbered each step and cross-referenced them as "Step 6", "Step 2", etc. + That broke every time a step was inserted, removed, or reordered -- every + reference elsewhere in the document had to be found and renumbered by + hand, and it was easy to miss one. Steps are referenced by name instead + (bolded to match their heading text), so the document stays correct + regardless of how steps shift around. +- **Parallelize independent work.** The steps below are written in the order + they're normally reasoned about, but that doesn't mean everything has to + run one at a time. Where two pieces of work don't depend on each other's + output, hand them to separate subagents and let them run concurrently + instead of sequentially -- e.g. the two review efforts in **Launch API + Documentation Review Agents** already run as parallel background agents, + and **Tag and Push pgxntool** / **Stamp, Tag, and Push pgxntool-test** are + two independent repos that can likewise run at the same time rather than + one after the other. Anything that reads or depends on another step's + output (e.g. **Sanity-Check bin/version Output** needs the stamp from + **Update HISTORY.asc and Commit** to already exist) must still wait for + that dependency first. + --- -## Step 1: Run Pre-flight Checks +## Run Pre-flight Checks Run the pre-flight script, passing VERSION if provided: @@ -68,16 +91,17 @@ The script checks: - `PGXNTOOL_UPSTREAM` - remote name for pgxntool (e.g., "upstream") - `PGXNTOOL_TEST_UPSTREAM` - remote name for pgxntool-test (e.g., "upstream") -## Step 2: Launch API Documentation Review Agents +## Launch API Documentation Review Agents Immediately after pre-flight passes, launch the review agents below via the Agent tool, running in the background. This happens early so the review has -time to finish while Steps 3-4 (version number, confirming HISTORY.asc) are -worked through. +time to finish while the version number is determined and HISTORY.asc is +confirmed (below). -**Gate: do not proceed past Step 6 (Update HISTORY.asc and Commit) — i.e. do +**Gate: do not proceed past Update HISTORY.asc and Commit (below) — i.e. do not make any release-related change to git — until both sets of findings -below have been retrieved and inspected.** See Step 5. +below have been retrieved and inspected.** See Inspect API Documentation +Review Findings. Launch two independent review efforts. Each may be one agent or a small set of agents if splitting the surface area (e.g. by file) makes sense; give @@ -117,7 +141,7 @@ history) - This review ignores git history entirely — it only compares the README against the code as they exist right now. -## Step 3: Determine Version Number +## Determine Version Number If VERSION was not provided as an argument, ask the user: @@ -131,7 +155,7 @@ Use AskUserQuestion: .claude/skills/release/scripts/release-preflight.sh VERSION ``` -## Step 4: Confirm HISTORY.asc +## Confirm HISTORY.asc Read `../pgxntool/HISTORY.asc` and show the user what's in the STABLE section. @@ -139,26 +163,27 @@ Read `../pgxntool/HISTORY.asc` and show the user what's in the STABLE section. - Warn: "No STABLE section found. No changes are documented for this release." - Ask user if they want to continue using AskUserQuestion. -## Step 5: Inspect API Documentation Review Findings +## Inspect API Documentation Review Findings -Retrieve the results from both review efforts launched in Step 2 (wait for -them if they haven't finished). This is a hard gate: **do not proceed to -Step 6 until this step is complete** — Step 6 is the first release step -that changes git state, and the whole point of launching the reviews early -was to have their findings in hand before that happens. +Retrieve the results from both review efforts launched in **Launch API +Documentation Review Agents** (wait for them if they haven't finished). This +is a hard gate: **do not proceed to Update HISTORY.asc and Commit until this +step is complete** — that's the first release step that changes git state, +and the whole point of launching the reviews early was to have their +findings in hand before that happens. For each finding: -- **Since-last-release findings (2A):** a behavior change without a STABLE - entry MUST be fixed before continuing. Either add the missing entry to the - STABLE section now (folded into Step 6's edit), or ask the user how they - want it documented — do not release with an undocumented behavior change. - API items added/removed by these commits but missing from README.asc must - also be fixed (edit README.asc) before continuing. -- **Comprehensive findings (2B):** these may include pre-existing drift - unrelated to this release. Show the findings to the user and ask whether - to fix now (as part of this release), file as follow-up work, or dismiss - as a false positive — don't silently fix or silently ignore them. +- **Since-last-release findings (review A):** a behavior change without a + STABLE entry MUST be fixed before continuing. Either add the missing entry + to the STABLE section now (folded into the HISTORY.asc edit below), or ask + the user how they want it documented — do not release with an undocumented + behavior change. API items added/removed by these commits but missing from + README.asc must also be fixed (edit README.asc) before continuing. +- **Comprehensive findings (review B):** these may include pre-existing + drift unrelated to this release. Show the findings to the user and ask + whether to fix now (as part of this release), file as follow-up work, or + dismiss as a false positive — don't silently fix or silently ignore them. - **Ambiguous user-facing API surface calls (either agent):** show these to the user too. If a pattern recurs or the user gives a clear answer, consider updating the "User-facing API surface" definition in Terminology @@ -166,33 +191,55 @@ For each finding: Summarize what was found and how each item was resolved before moving on. -## Step 6: Update HISTORY.asc and Commit +## Update HISTORY.asc and Commit + +### Reorder the STABLE section entries by importance + +By this point (the gate in **Inspect API Documentation Review Findings**) +every entry this release needs is already in place. Before stamping, sort +those entries -- most important first -- into: + +- Breaking changes -- anything that could break an existing consumer's + build, tests, or behavior +- Non-breaking behavior changes -- existing behavior changed, but nothing + should break as a result (expect this category to be rare) +- New features/additions -- new targets, variables, scripts, etc. +- Bugfixes + +An entry that fits more than one category goes under the highest (earliest) +one that applies. This is a one-time pass over *this release's* entries +only -- it's not a standing order for HISTORY.asc as a whole, and older, +already-released sections are never touched. If an entry's category is +genuinely unclear, ask the user rather than guessing. + +### Stamp the version + +Edit `../pgxntool/HISTORY.asc`: replace the `STABLE` heading with the version number. -1. Edit `../pgxntool/HISTORY.asc`: Replace the `STABLE` heading with the version number +Replace: +``` +STABLE +------ +``` +With: +``` +VERSION +------- +``` +(Adjust dashes to match version string length) - Replace: - ``` - STABLE - ------ - ``` - With: - ``` - VERSION - ------- - ``` - (Adjust dashes to match version string length) +### Commit -2. Commit: - ```bash - cd ../pgxntool && git commit -am "Stamp VERSION" - ``` +```bash +cd ../pgxntool && git commit -am "Stamp VERSION" +``` -## Step 7: Sanity-Check pgxntool-version Output +## Sanity-Check bin/version Output CI depends on `bin/version` (see `../pgxntool/bin/version`, also wired up as `make pgxntool-version`) correctly reflecting whatever is actually on the first line of `HISTORY.asc` -- that's what lets a release PR be caught in CI -if the stamp in Step 6 didn't take effect the way it should have. Run it +if the stamp above didn't take effect the way it should have. Run it directly against the just-stamped, unmodified `../pgxntool` checkout: ```bash @@ -200,11 +247,11 @@ directly against the just-stamped, unmodified `../pgxntool` checkout: ``` **The output must be exactly VERSION.** If it's `STABLE`, an old version, or -an error instead, the Step 6 edit didn't take effect correctly (or -`bin/version` itself regressed). Stop and fix it -- do not tag or push a -release that doesn't agree with this. +an error instead, the stamp in **Update HISTORY.asc and Commit** didn't take +effect correctly (or `bin/version` itself regressed). Stop and fix it -- do +not tag or push a release that doesn't agree with this. -## Step 8: Tag and Push pgxntool +## Tag and Push pgxntool **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** @@ -215,7 +262,7 @@ git push PGXNTOOL_UPSTREAM master git push PGXNTOOL_UPSTREAM VERSION ``` -## Step 9: Stamp, Tag, and Push pgxntool-test +## Stamp, Tag, and Push pgxntool-test **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** @@ -229,7 +276,10 @@ git push PGXNTOOL_TEST_UPSTREAM master git push PGXNTOOL_TEST_UPSTREAM VERSION ``` -## Step 10: Update `release` Tag +This is independent of **Tag and Push pgxntool** (separate repo, separate +remote) — see Process Notes above on running the two as parallel subagents. + +## Update the release Tag Both repos have a `release` tag on upstream that must always point to the latest release. This is a moving tag that requires force-push to update. @@ -246,7 +296,7 @@ git tag -f release VERSION git push PGXNTOOL_TEST_UPSTREAM -f refs/tags/release ``` -## Step 11: Verify and Report +## Verify and Report ```bash cd ../pgxntool && git checkout master @@ -285,9 +335,11 @@ Verify releases: - Note which repo failed and what state the other repo is in **Rollback guidance if partial failure:** -- If pgxntool push succeeded but pgxntool-test failed: - - Note that pgxntool is already released - - Provide commands to manually complete pgxntool-test release +- If pgxntool push succeeded but pgxntool-test failed (or vice versa -- + if these ran as parallel subagents, either can fail independently of + the other): + - Note which repo already succeeded + - Provide commands to manually complete the other repo's release - If failure during push: - Local state is complete, just need to retry push From 1bcec32a5a242c652ce47a87f758bf35f85dacc4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 14:21:56 -0500 Subject: [PATCH 06/12] release skill: link step cross-references to their anchors Bolded prose references ('**Update HISTORY.asc and Commit**') were an improvement over numbered steps but still just text -- nothing caught them drifting out of sync with the actual heading, and they gave a human reading this file on GitHub nothing to click. Real markdown links to each heading's anchor are unambiguous (a broken one is visibly broken) and clickable in GitHub's rendered view. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 94 ++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 38 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 47fb2e4..4865cc0 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -18,16 +18,17 @@ Create a release for pgxntool and pgxntool-test. - **STABLE section**: The heading in `HISTORY.asc` where unreleased changes are documented. During a release, this heading is replaced with the version number. This has nothing to do with git branches. - **UPSTREAM_REMOTE**: The local git remote pointing to the main project repos at `https://github.com/Postgres-Extensions/`. Releases must be pushed here -- never to a fork. The remote name varies; it is identified by URL pattern in the pre-flight script. -- **User-facing API surface**: what the review agents launched in **Launch - API Documentation Review Agents** treat as "the documented API" of - pgxntool. The canonical, evolving definition lives in this repo's own - `CLAUDE.md`, under "User-Facing API Surface of pgxntool" (not - `../pgxntool/CLAUDE.md` — that file is user-facing docs for extension - developers, not dev/audit tooling docs). Read that section fresh before - launching those agents and give them its current text verbatim as their - scope, since it's expected to change over time. When a reviewer hits a - case that doesn't clearly fit, flag it for the user rather than guessing, - and consider updating that section afterward. +- **User-facing API surface**: what the review agents launched in + [Launch API Documentation Review Agents](#launch-api-documentation-review-agents) + treat as "the documented API" of pgxntool. The canonical, evolving + definition lives in this repo's own `CLAUDE.md`, under "User-Facing API + Surface of pgxntool" (not `../pgxntool/CLAUDE.md` — that file is + user-facing docs for extension developers, not dev/audit tooling docs). + Read that section fresh before launching those agents and give them its + current text verbatim as their scope, since it's expected to change over + time. When a reviewer hits a case that doesn't clearly fit, flag it for + the user rather than guessing, and consider updating that section + afterward. - **Discovering make targets**: the definition's target list should be found with `make list` (a target pgxntool itself provides — see `base.mk`), not by grepping for target definitions, since pattern rules @@ -45,25 +46,34 @@ Create a release for pgxntool and pgxntool-test. ## Process Notes -- **Steps below are headings, not numbers.** Earlier versions of this skill - numbered each step and cross-referenced them as "Step 6", "Step 2", etc. - That broke every time a step was inserted, removed, or reordered -- every - reference elsewhere in the document had to be found and renumbered by - hand, and it was easy to miss one. Steps are referenced by name instead - (bolded to match their heading text), so the document stays correct - regardless of how steps shift around. +- **Steps below are headings, referenced by anchor link.** Earlier versions + of this skill numbered each step and cross-referenced them as "Step 6", + "Step 2", etc. That broke every time a step was inserted, removed, or + reordered -- every reference elsewhere in the document had to be found + and renumbered by hand, and it was easy to miss one. A later revision + switched to bolding the step's heading text instead, which fixed the + renumbering problem but was still just prose -- nothing stopped it from + drifting out of sync with the actual heading, and it gave a human reading + this on GitHub nothing to click. Steps are referenced by a real markdown + link to the heading's anchor instead (`[Heading Text](#heading-text)`), + which is both unambiguous (a stale link can be checked by clicking it) + and clickable when this file is viewed on GitHub. - **Parallelize independent work.** The steps below are written in the order they're normally reasoned about, but that doesn't mean everything has to run one at a time. Where two pieces of work don't depend on each other's output, hand them to separate subagents and let them run concurrently - instead of sequentially -- e.g. the two review efforts in **Launch API - Documentation Review Agents** already run as parallel background agents, - and **Tag and Push pgxntool** / **Stamp, Tag, and Push pgxntool-test** are - two independent repos that can likewise run at the same time rather than - one after the other. Anything that reads or depends on another step's - output (e.g. **Sanity-Check bin/version Output** needs the stamp from - **Update HISTORY.asc and Commit** to already exist) must still wait for - that dependency first. + instead of sequentially -- e.g. the two review efforts in + [Launch API Documentation Review Agents](#launch-api-documentation-review-agents) + already run as parallel background agents, and + [Tag and Push pgxntool](#tag-and-push-pgxntool) / + [Stamp, Tag, and Push pgxntool-test](#stamp-tag-and-push-pgxntool-test) + are two independent repos that can likewise run at the same time rather + than one after the other. Anything that reads or depends on another + step's output (e.g. + [Sanity-Check bin/version Output](#sanity-check-binversion-output) needs + the stamp from + [Update HISTORY.asc and Commit](#update-historyasc-and-commit) to already + exist) must still wait for that dependency first. --- @@ -95,13 +105,15 @@ The script checks: Immediately after pre-flight passes, launch the review agents below via the Agent tool, running in the background. This happens early so the review has -time to finish while the version number is determined and HISTORY.asc is -confirmed (below). +time to finish while +[Determine Version Number](#determine-version-number) and +[Confirm HISTORY.asc](#confirm-historyasc) (below) are worked through. -**Gate: do not proceed past Update HISTORY.asc and Commit (below) — i.e. do -not make any release-related change to git — until both sets of findings -below have been retrieved and inspected.** See Inspect API Documentation -Review Findings. +**Gate: do not proceed past +[Update HISTORY.asc and Commit](#update-historyasc-and-commit) — i.e. do not +make any release-related change to git — until both sets of findings below +have been retrieved and inspected.** See +[Inspect API Documentation Review Findings](#inspect-api-documentation-review-findings). Launch two independent review efforts. Each may be one agent or a small set of agents if splitting the surface area (e.g. by file) makes sense; give @@ -165,9 +177,11 @@ Read `../pgxntool/HISTORY.asc` and show the user what's in the STABLE section. ## Inspect API Documentation Review Findings -Retrieve the results from both review efforts launched in **Launch API -Documentation Review Agents** (wait for them if they haven't finished). This -is a hard gate: **do not proceed to Update HISTORY.asc and Commit until this +Retrieve the results from both review efforts launched in +[Launch API Documentation Review Agents](#launch-api-documentation-review-agents) +(wait for them if they haven't finished). This is a hard gate: **do not +proceed to +[Update HISTORY.asc and Commit](#update-historyasc-and-commit) until this step is complete** — that's the first release step that changes git state, and the whole point of launching the reviews early was to have their findings in hand before that happens. @@ -195,7 +209,8 @@ Summarize what was found and how each item was resolved before moving on. ### Reorder the STABLE section entries by importance -By this point (the gate in **Inspect API Documentation Review Findings**) +By this point (the gate in +[Inspect API Documentation Review Findings](#inspect-api-documentation-review-findings)) every entry this release needs is already in place. Before stamping, sort those entries -- most important first -- into: @@ -247,7 +262,8 @@ directly against the just-stamped, unmodified `../pgxntool` checkout: ``` **The output must be exactly VERSION.** If it's `STABLE`, an old version, or -an error instead, the stamp in **Update HISTORY.asc and Commit** didn't take +an error instead, the stamp in +[Update HISTORY.asc and Commit](#update-historyasc-and-commit) didn't take effect correctly (or `bin/version` itself regressed). Stop and fix it -- do not tag or push a release that doesn't agree with this. @@ -276,8 +292,10 @@ git push PGXNTOOL_TEST_UPSTREAM master git push PGXNTOOL_TEST_UPSTREAM VERSION ``` -This is independent of **Tag and Push pgxntool** (separate repo, separate -remote) — see Process Notes above on running the two as parallel subagents. +This is independent of [Tag and Push pgxntool](#tag-and-push-pgxntool) +(separate repo, separate remote) — see +[Process Notes](#process-notes) above on running the two as parallel +subagents. ## Update the release Tag From 7ec86dbe7c221a7f2055a5ba1b278ae78b48330a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 14:56:11 -0500 Subject: [PATCH 07/12] release skill: use a PR instead of pushing straight to master The old flow committed the stamp directly to local master and pushed straight to upstream master/tags. pgxntool's CI only triggers on pull_request, so that path never ran CI at all -- a release could ship with a broken stamp, a regressed bin/version, or anything else CI would normally catch, and nobody would know until later. Both repos now get a release-VERSION branch and a real PR. Using the same branch name on the same (upstream) account for both repos lets pgxntool-test's CI automatically pair its release PR with pgxntool's, which is normally enough to satisfy pgxntool's check-test-pr gate without needing the commit-with-no-tests label -- though the skill still tells the user to apply it if that pairing doesn't take for some reason, since this skill is not allowed to apply that label itself. Tagging moves to after the human merges both PRs, and re-reads HISTORY.asc from the merged master rather than the pre-merge branch, since a squash or rebase merge can change what actually lands. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 150 +++++++++++++++++++++++++------- 1 file changed, 117 insertions(+), 33 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 4865cc0..3f3eb1b 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -5,7 +5,7 @@ description: | HISTORY.asc updates, and pushing to the main Postgres-Extensions GitHub repos. Use when user says "release", "create release", "tag version", or "/release" -allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Read, Edit +allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Bash(git merge:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(.claude/skills/ci/scripts/monitor-ci.sh:*), Read, Edit --- # /release @@ -18,6 +18,11 @@ Create a release for pgxntool and pgxntool-test. - **STABLE section**: The heading in `HISTORY.asc` where unreleased changes are documented. During a release, this heading is replaced with the version number. This has nothing to do with git branches. - **UPSTREAM_REMOTE**: The local git remote pointing to the main project repos at `https://github.com/Postgres-Extensions/`. Releases must be pushed here -- never to a fork. The remote name varies; it is identified by URL pattern in the pre-flight script. +- **Release branch**: `release-VERSION` (e.g. `release-2.2.0`), created fresh + off master in both repos once the version number is known (see + [Determine Version Number](#determine-version-number)). The stamp commit + lands here, not on master directly -- see + [Open Release Pull Requests](#open-release-pull-requests) for why. - **User-facing API surface**: what the review agents launched in [Launch API Documentation Review Agents](#launch-api-documentation-review-agents) treat as "the documented API" of pgxntool. The canonical, evolving @@ -64,16 +69,19 @@ Create a release for pgxntool and pgxntool-test. output, hand them to separate subagents and let them run concurrently instead of sequentially -- e.g. the two review efforts in [Launch API Documentation Review Agents](#launch-api-documentation-review-agents) - already run as parallel background agents, and - [Tag and Push pgxntool](#tag-and-push-pgxntool) / - [Stamp, Tag, and Push pgxntool-test](#stamp-tag-and-push-pgxntool-test) - are two independent repos that can likewise run at the same time rather - than one after the other. Anything that reads or depends on another - step's output (e.g. + already run as parallel background agents, and the pgxntool / + pgxntool-test halves of + [Open Release Pull Requests](#open-release-pull-requests) and + [Tag Both Repos](#tag-both-repos) are two independent repos that can + likewise run at the same time rather than one after the other. Anything + that reads or depends on another step's output (e.g. [Sanity-Check bin/version Output](#sanity-check-binversion-output) needs the stamp from [Update HISTORY.asc and Commit](#update-historyasc-and-commit) to already exist) must still wait for that dependency first. +- **This skill never merges its own release PRs, and never applies the + `commit-with-no-tests` label.** Both are explicit human actions -- see + [Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review). --- @@ -207,6 +215,18 @@ Summarize what was found and how each item was resolved before moving on. ## Update HISTORY.asc and Commit +### Create the release branch + +Both repos get a same-named branch (see **Release branch** in Terminology). +Using the same name on both, pushed to the same (upstream) account, is what +lets pgxntool-test's CI automatically pair its release PR with pgxntool's -- +see [Open Release Pull Requests](#open-release-pull-requests). + +```bash +cd ../pgxntool && git checkout -b release-VERSION +cd ../pgxntool-test && git checkout -b release-VERSION +``` + ### Reorder the STABLE section entries by importance By this point (the gate in @@ -265,37 +285,93 @@ directly against the just-stamped, unmodified `../pgxntool` checkout: an error instead, the stamp in [Update HISTORY.asc and Commit](#update-historyasc-and-commit) didn't take effect correctly (or `bin/version` itself regressed). Stop and fix it -- do -not tag or push a release that doesn't agree with this. +not open a release PR that doesn't agree with this. -## Tag and Push pgxntool +## Open Release Pull Requests -**CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** +**Never commit or push straight to master.** pgxntool's CI only triggers on +`pull_request` -- a direct push to master's `master` branch (which earlier +versions of this skill did) never runs CI at all, silently skipping every +check this repo relies on. Instead, push the release branch and open a PR +in each repo, same as any other change. ```bash cd ../pgxntool -git tag VERSION -git push PGXNTOOL_UPSTREAM master -git push PGXNTOOL_UPSTREAM VERSION +git push PGXNTOOL_UPSTREAM release-VERSION +gh pr create --repo Postgres-Extensions/pgxntool \ + --base master --head release-VERSION \ + --title "Release VERSION" \ + --body "Stamps HISTORY.asc for VERSION. Companion: pgxntool-test release-VERSION." ``` -## Stamp, Tag, and Push pgxntool-test +```bash +cd ../pgxntool-test +git commit --allow-empty -m "Stamp VERSION" +git push PGXNTOOL_TEST_UPSTREAM release-VERSION +gh pr create --repo Postgres-Extensions/pgxntool-test \ + --base master --head release-VERSION \ + --title "Release VERSION" \ + --body "Companion stamp for pgxntool release-VERSION." +``` + +Push both branches directly to the Postgres-Extensions remotes, never a +fork -- this is also what makes the branch pairing below work, since +pgxntool-test's CI matches a paired pgxntool branch by *account* as well as +name. + +**Open the pgxntool-test PR first, or as close to simultaneously as +possible.** pgxntool's `check-test-pr` CI job looks for a pgxntool-test PR +with the same branch name on the same account; if pgxntool's CI runs before +that PR exists, the pairing lookup can miss it. + +## Wait for CI, Then Hand Off for Review -**CRITICAL: Push to the Postgres-Extensions remote, not to a fork.** +Monitor both PRs' CI (see the `/ci` skill / `monitor-ci.sh`, or +`gh pr checks`). This skill does not merge either PR and does not apply any +label -- both are explicit human actions. Report to the user: -Create a stamp commit to match pgxntool's, then tag and push: +- Both PR URLs +- CI status for each + +**If pgxntool's `check-test-pr` job fails** (the automatic pairing above +should normally prevent this, but isn't guaranteed -- e.g. if the diff +wasn't purely doc-only and the pgxntool-test PR wasn't visible in time): +tell the user a maintainer needs to apply the `commit-with-no-tests` label +to the pgxntool PR themselves. **This skill must never apply that label -- +it's maintainer-gated.** + +**Then stop and wait.** Do not proceed to +[Tag Both Repos](#tag-both-repos) until the user confirms both PRs are +merged. + +## Tag Both Repos + +Only after the user has confirmed both release PRs are merged. The tag must +point at whatever actually landed on master -- not the pre-merge branch tip, +which may differ if the PR was squashed or rebased on merge. + +```bash +cd ../pgxntool +git fetch PGXNTOOL_UPSTREAM master +git checkout master +git merge --ff-only PGXNTOOL_UPSTREAM/master +head -n1 HISTORY.asc # must read exactly VERSION -- stop if it doesn't +git tag VERSION +git push PGXNTOOL_UPSTREAM VERSION +``` ```bash cd ../pgxntool-test -git commit --allow-empty -m "Stamp VERSION" +git fetch PGXNTOOL_TEST_UPSTREAM master +git checkout master +git merge --ff-only PGXNTOOL_TEST_UPSTREAM/master git tag VERSION -git push PGXNTOOL_TEST_UPSTREAM master git push PGXNTOOL_TEST_UPSTREAM VERSION ``` -This is independent of [Tag and Push pgxntool](#tag-and-push-pgxntool) -(separate repo, separate remote) — see -[Process Notes](#process-notes) above on running the two as parallel -subagents. +The `head -n1 HISTORY.asc` check exists because a squash/rebase merge can +alter file content in ways a pre-merge check never saw -- confirm the merged +result, not just the pre-merge branch, agrees with VERSION before tagging. ## Update the release Tag @@ -316,9 +392,13 @@ git push PGXNTOOL_TEST_UPSTREAM -f refs/tags/release ## Verify and Report +Both repos are already on master with the release branch merged in (see +[Tag Both Repos](#tag-both-repos)); delete the now-merged local release +branches: + ```bash -cd ../pgxntool && git checkout master -cd ../pgxntool-test && git checkout master +cd ../pgxntool && git branch -d release-VERSION +cd ../pgxntool-test && git branch -d release-VERSION ``` Output: @@ -328,12 +408,12 @@ Release VERSION complete! pgxntool: - HISTORY.asc stamped with VERSION -- Tag VERSION created and pushed to PGXNTOOL_UPSTREAM +- Release PR merged, tag VERSION created and pushed to PGXNTOOL_UPSTREAM - release tag updated to VERSION pgxntool-test: - Stamp commit created -- Tag VERSION created and pushed to PGXNTOOL_TEST_UPSTREAM +- Release PR merged, tag VERSION created and pushed to PGXNTOOL_TEST_UPSTREAM - release tag updated to VERSION Verify releases: @@ -352,14 +432,18 @@ Verify releases: - Provide recovery instructions - Note which repo failed and what state the other repo is in +**If a release PR's CI fails:** fix the problem on the `release-VERSION` +branch and push again (`git commit --amend` or a follow-up commit) -- do not +close it and open a new PR, and do not fall back to pushing straight to +master. + **Rollback guidance if partial failure:** -- If pgxntool push succeeded but pgxntool-test failed (or vice versa -- - if these ran as parallel subagents, either can fail independently of - the other): - - Note which repo already succeeded - - Provide commands to manually complete the other repo's release -- If failure during push: - - Local state is complete, just need to retry push +- If one repo's PR is merged but the other isn't yet: this is expected with + a manual-merge workflow, not a failure -- wait for the user to merge the + second one before running + [Tag Both Repos](#tag-both-repos). Don't tag one repo without the other. +- If pushing the release branch itself fails: local state is complete, just + need to retry the push. **Common issues:** - "Push rejected": Upstream has changes. Need to pull first. From f47fcbbf64c6eb778b7044495a9bcc0db53d6c00 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 16:29:53 -0500 Subject: [PATCH 08/12] release skill: address PR #43 review feedback - Confirm HISTORY.asc: a missing STABLE section no longer has a dead-end 'continue anyway' path -- the user must either add a STABLE section now or stop, since later steps can't proceed without one (r3669108534 et al). - Fix Markdown fences lacking blank lines/language identifiers (markdownlint MD031/MD040) in the HISTORY.asc example and the final report output (r3669108538). - Open Release Pull Requests: the pgxntool-test push/PR block now actually comes first in the instructions, matching the prose that says it must (r3669108544). Each push now has an explicit monitor-ci.sh invocation immediately after it, and the CI-retry path in Error Handling does the same (r3669108551). - Tag Both Repos: run the crossref-audit script before rebasing onto a freshly fetched master, per this repo's own CLAUDE.md rule (r3669108556). - Process Notes: added a note on why the commits in this flow don't need a separate confirmation gate -- invoking /release is itself the explicit instruction, and nothing here touches master without a human-merged PR in between (r3669108542). test/standard/pgxntool-version.bats: use the normalized $PGXNREPO (set by load_test_env) instead of $TOPDIR/../pgxntool, so the test follows the same branch/path resolution as the rest of the suite instead of a hardcoded relative path (r3669108562). Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 91 +++++++++++++++++++++-------- test/standard/pgxntool-version.bats | 10 ++-- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 3f3eb1b..5913b78 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -82,6 +82,12 @@ Create a release for pgxntool and pgxntool-test. - **This skill never merges its own release PRs, and never applies the `commit-with-no-tests` label.** Both are explicit human actions -- see [Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review). +- **The commits in this skill don't need a separate per-commit confirmation + gate.** Invoking `/release` at all is the user's explicit instruction to + run this whole documented flow, commits included -- and none of those + commits touch master directly; they land on `release-VERSION` and only + reach master through a human-reviewed, human-merged PR (see + [Open Release Pull Requests](#open-release-pull-requests)). --- @@ -171,6 +177,7 @@ Use AskUserQuestion: - Provide options based on current version in pgxntool's HISTORY.asc **Then re-run pre-flight** with the chosen version to validate it: + ```bash .claude/skills/release/scripts/release-preflight.sh VERSION ``` @@ -179,9 +186,19 @@ Use AskUserQuestion: Read `../pgxntool/HISTORY.asc` and show the user what's in the STABLE section. -**If no STABLE section exists:** -- Warn: "No STABLE section found. No changes are documented for this release." -- Ask user if they want to continue using AskUserQuestion. +**If no STABLE section exists:** later, +[Stamp the version](#stamp-the-version) can only replace an existing +`STABLE` heading -- there's nothing to continue *to* without one, and +[Sanity-Check bin/version Output](#sanity-check-binversion-output) will +fail if it's missing. Ask the user (via AskUserQuestion) to pick one: +- Add a `STABLE` section to `HISTORY.asc` now (even if it ends up + documenting nothing but a header for this release), then continue. +- Stop the release here. + +Do not proceed to +[Inspect API Documentation Review Findings](#inspect-api-documentation-review-findings) +without a `STABLE` section actually in hand -- "continue" only means +something once one exists. ## Inspect API Documentation Review Findings @@ -252,15 +269,19 @@ genuinely unclear, ask the user rather than guessing. Edit `../pgxntool/HISTORY.asc`: replace the `STABLE` heading with the version number. Replace: -``` + +```text STABLE ------ ``` + With: -``` + +```text VERSION ------- ``` + (Adjust dashes to match version string length) ### Commit @@ -295,6 +316,30 @@ versions of this skill did) never runs CI at all, silently skipping every check this repo relies on. Instead, push the release branch and open a PR in each repo, same as any other change. +**Open the pgxntool-test PR first.** pgxntool's `check-test-pr` CI job looks +for a pgxntool-test PR with the same branch name on the same account; if +pgxntool's CI runs before that PR exists, the pairing lookup can miss it. + +```bash +cd ../pgxntool-test +git commit --allow-empty -m "Stamp VERSION" +git push PGXNTOOL_TEST_UPSTREAM release-VERSION +gh pr create --repo Postgres-Extensions/pgxntool-test \ + --base master --head release-VERSION \ + --title "Release VERSION" \ + --body "Companion stamp for pgxntool release-VERSION." +``` + +Per this project's CI-monitoring rule, immediately start a background +monitor for that push (exact SHA, not `--branch`, to avoid a race with any +other concurrent push on this branch name): + +```bash +bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool-test release-VERSION "" +``` + +Then push pgxntool and open its PR: + ```bash cd ../pgxntool git push PGXNTOOL_UPSTREAM release-VERSION @@ -305,30 +350,20 @@ gh pr create --repo Postgres-Extensions/pgxntool \ ``` ```bash -cd ../pgxntool-test -git commit --allow-empty -m "Stamp VERSION" -git push PGXNTOOL_TEST_UPSTREAM release-VERSION -gh pr create --repo Postgres-Extensions/pgxntool-test \ - --base master --head release-VERSION \ - --title "Release VERSION" \ - --body "Companion stamp for pgxntool release-VERSION." +bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool release-VERSION "" ``` Push both branches directly to the Postgres-Extensions remotes, never a -fork -- this is also what makes the branch pairing below work, since +fork -- this is also what makes the branch pairing work, since pgxntool-test's CI matches a paired pgxntool branch by *account* as well as name. -**Open the pgxntool-test PR first, or as close to simultaneously as -possible.** pgxntool's `check-test-pr` CI job looks for a pgxntool-test PR -with the same branch name on the same account; if pgxntool's CI runs before -that PR exists, the pairing lookup can miss it. - ## Wait for CI, Then Hand Off for Review -Monitor both PRs' CI (see the `/ci` skill / `monitor-ci.sh`, or -`gh pr checks`). This skill does not merge either PR and does not apply any -label -- both are explicit human actions. Report to the user: +Wait for the results from the two `monitor-ci.sh` background runs started in +[Open Release Pull Requests](#open-release-pull-requests). This skill does +not merge either PR and does not apply any label -- both are explicit human +actions. Report to the user: - Both PR URLs - CI status for each @@ -350,6 +385,15 @@ Only after the user has confirmed both release PRs are merged. The tag must point at whatever actually landed on master -- not the pre-merge branch tip, which may differ if the PR was squashed or rebased on merge. +This step rebases local state onto a freshly fetched master in both repos, +so per this repo's CLAUDE.md, run the cross-reference audit first: + +```bash +bash .claude/skills/crossref-audit/scripts/audit.sh ../pgxntool . +``` + +Follow its output if it flags anything; only continue once it's clean. + ```bash cd ../pgxntool git fetch PGXNTOOL_UPSTREAM master @@ -403,7 +447,7 @@ cd ../pgxntool-test && git branch -d release-VERSION Output: -``` +```text Release VERSION complete! pgxntool: @@ -435,7 +479,8 @@ Verify releases: **If a release PR's CI fails:** fix the problem on the `release-VERSION` branch and push again (`git commit --amend` or a follow-up commit) -- do not close it and open a new PR, and do not fall back to pushing straight to -master. +master. Immediately re-run `monitor-ci.sh` for that push, same as in +[Open Release Pull Requests](#open-release-pull-requests). **Rollback guidance if partial failure:** - If one repo's PR is merged but the other isn't yet: this is expected with diff --git a/test/standard/pgxntool-version.bats b/test/standard/pgxntool-version.bats index 5fe121f..723032f 100644 --- a/test/standard/pgxntool-version.bats +++ b/test/standard/pgxntool-version.bats @@ -18,7 +18,7 @@ setup_file() { load_test_env "pgxntool-version" ensure_foundation "$TEST_DIR" - export VERSION_SCRIPT="$TOPDIR/../pgxntool/bin/version" + export VERSION_SCRIPT="$PGXNREPO/bin/version" export SCRATCH_DIR="$TEST_DIR/pgxntool-version-tests" } @@ -70,9 +70,9 @@ setup() { } # CRITICAL: this test must run bin/version directly against the real, -# unmodified $TOPDIR/../pgxntool checkout -- never a scratch file, an rsync'd -# copy, or a cached foundation snapshot. A release PR stamps HISTORY.asc with -# the real version and relies on CI running this exact check to catch a bad +# unmodified $PGXNREPO checkout -- never a scratch file, an rsync'd copy, or +# a cached foundation snapshot. A release PR stamps HISTORY.asc with the +# real version and relies on CI running this exact check to catch a bad # stamp before it's tagged and pushed -- that guarantee only holds if the # check reads the literal, current pgxntool/HISTORY.asc. This matters most # precisely when the top line is NOT "STABLE": that's the release commit @@ -81,7 +81,7 @@ setup() { # non-empty check. @test "bin/version matches the current pgxntool/HISTORY.asc with no modifications" { local expected - expected=$(head -n1 "$TOPDIR/../pgxntool/HISTORY.asc") + expected=$(head -n1 "$PGXNREPO/HISTORY.asc") run "$VERSION_SCRIPT" assert_success From 0f78deefba5bc4d19a72b51ff6e8d3886e63a0a4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 16:46:21 -0500 Subject: [PATCH 09/12] release skill: push order doesn't protect check-test-pr, drop it The 'open pgxntool-test first' instruction from the previous commit doesn't actually solve anything: check-test-pr checks doc-only status from the PR's own file list before it ever looks for a paired PR, and a release-stamp diff (HISTORY.asc/README.asc/README.html) is always doc-only, so the bypass fires regardless of push order. Worse, the ordering requirement directly contradicted this file's own Process Notes guidance to run the two repos' pushes as parallel subagents. Also note the underlying pairing-search race is a general latent issue in check-test-pr for any paired PR, not release-specific -- just more likely to bite a release PR since those aren't iterated on the way a normal PR is. Not something this skill needs to work around, since it never relies on that search succeeding. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 53 ++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 5913b78..ed12a3b 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -316,9 +316,25 @@ versions of this skill did) never runs CI at all, silently skipping every check this repo relies on. Instead, push the release branch and open a PR in each repo, same as any other change. -**Open the pgxntool-test PR first.** pgxntool's `check-test-pr` CI job looks -for a pgxntool-test PR with the same branch name on the same account; if -pgxntool's CI runs before that PR exists, the pairing lookup can miss it. +**Push order doesn't matter here, and pushing pgxntool-test first is not a +safeguard** -- an earlier draft of this step claimed it was, which was +wrong. pgxntool's `check-test-pr` job checks doc-only status *first*, purely +from this PR's own changed-file list -- no cross-repo lookup, no ordering +or timing dependency at all -- and only falls through to searching for a +paired pgxntool-test PR if that's false. A release-stamp PR only ever +touches `HISTORY.asc`/`README.asc`/`README.html`, all covered by +`DOC_EXTENSIONS`, so it should hit that bypass every time regardless of +which repo's PR goes up first. (The pairing search existing at all is a +general, latent race in `check-test-pr` for *any* paired PR, release or +not -- ordinary PRs just tend to survive it better since a failed first CI +run gets fixed by a follow-up push, whereas a release PR is pushed once and +not iterated on nearly as much. Worth its own issue if it ever actually +bites; not something this skill needs to work around, since it never +depends on the pairing search succeeding.) + +Push both branches directly to the Postgres-Extensions remotes, never a +fork, and open both PRs -- these can run as parallel subagents (see Process +Notes): ```bash cd ../pgxntool-test @@ -330,16 +346,6 @@ gh pr create --repo Postgres-Extensions/pgxntool-test \ --body "Companion stamp for pgxntool release-VERSION." ``` -Per this project's CI-monitoring rule, immediately start a background -monitor for that push (exact SHA, not `--branch`, to avoid a race with any -other concurrent push on this branch name): - -```bash -bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool-test release-VERSION "" -``` - -Then push pgxntool and open its PR: - ```bash cd ../pgxntool git push PGXNTOOL_UPSTREAM release-VERSION @@ -349,14 +355,18 @@ gh pr create --repo Postgres-Extensions/pgxntool \ --body "Stamps HISTORY.asc for VERSION. Companion: pgxntool-test release-VERSION." ``` +Per this project's CI-monitoring rule, immediately start a background +monitor for each push (exact SHA, not `--branch`, to avoid a race with any +other concurrent push on this branch name -- one monitor per repo, not run +sequentially): + ```bash -bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool release-VERSION "" +bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool-test release-VERSION "" ``` -Push both branches directly to the Postgres-Extensions remotes, never a -fork -- this is also what makes the branch pairing work, since -pgxntool-test's CI matches a paired pgxntool branch by *account* as well as -name. +```bash +bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool release-VERSION "" +``` ## Wait for CI, Then Hand Off for Review @@ -368,9 +378,10 @@ actions. Report to the user: - Both PR URLs - CI status for each -**If pgxntool's `check-test-pr` job fails** (the automatic pairing above -should normally prevent this, but isn't guaranteed -- e.g. if the diff -wasn't purely doc-only and the pgxntool-test PR wasn't visible in time): +**If pgxntool's `check-test-pr` job fails** (shouldn't happen for a +doc-only release diff -- see +[Open Release Pull Requests](#open-release-pull-requests) -- but isn't +impossible, e.g. if this release ends up needing a non-doc file change): tell the user a maintainer needs to apply the `commit-with-no-tests` label to the pgxntool PR themselves. **This skill must never apply that label -- it's maintainer-gated.** From 711fe9c89a9e9d58eda19e53c5b34dcfbebc1c8d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 17:23:25 -0500 Subject: [PATCH 10/12] release skill: pgxntool-test PR must never be merged, exists only for CI Re-examined pgxntool's check-test-pr logic precisely: for a doc-only PR (which a release stamp always is), it skips the actual Postgres test matrix unconditionally, with no fallback to testing against master -- regardless of whether a paired pgxntool-test PR exists. So without a companion PR, the release content would get zero CI test coverage. What actually provides that coverage: an empty commit is NOT doc-only by pgxntool-test's own check (zero changed files trips a different guard there), so its own test job runs and resolves the paired pgxntool branch -- exercising the real Postgres suite against the actual release content via pgxntool-test's CI instead of pgxntool's. Since that PR never has anything worth landing, it must never be merged -- title/body now say so explicitly, and the flow closes it (gh pr close --delete-branch) instead of merging it. pgxntool-test's tag goes directly on its master, which never changed. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 128 ++++++++++++++++++++++---------- 1 file changed, 88 insertions(+), 40 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index ed12a3b..4d33234 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -5,7 +5,7 @@ description: | HISTORY.asc updates, and pushing to the main Postgres-Extensions GitHub repos. Use when user says "release", "create release", "tag version", or "/release" -allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Bash(git merge:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(.claude/skills/ci/scripts/monitor-ci.sh:*), Read, Edit +allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Bash(git merge:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr close:*), Bash(.claude/skills/ci/scripts/monitor-ci.sh:*), Read, Edit --- # /release @@ -20,9 +20,12 @@ Create a release for pgxntool and pgxntool-test. - **UPSTREAM_REMOTE**: The local git remote pointing to the main project repos at `https://github.com/Postgres-Extensions/`. Releases must be pushed here -- never to a fork. The remote name varies; it is identified by URL pattern in the pre-flight script. - **Release branch**: `release-VERSION` (e.g. `release-2.2.0`), created fresh off master in both repos once the version number is known (see - [Determine Version Number](#determine-version-number)). The stamp commit - lands here, not on master directly -- see - [Open Release Pull Requests](#open-release-pull-requests) for why. + [Determine Version Number](#determine-version-number)). The pgxntool + stamp commit lands here, not on master directly -- see + [Open Release Pull Requests](#open-release-pull-requests) for why. The + pgxntool-test branch of the same name carries only an empty commit and + its PR is **never merged** -- see the same section for why it still + needs to exist. - **User-facing API surface**: what the review agents launched in [Launch API Documentation Review Agents](#launch-api-documentation-review-agents) treat as "the documented API" of pgxntool. The canonical, evolving @@ -79,9 +82,12 @@ Create a release for pgxntool and pgxntool-test. the stamp from [Update HISTORY.asc and Commit](#update-historyasc-and-commit) to already exist) must still wait for that dependency first. -- **This skill never merges its own release PRs, and never applies the +- **This skill never merges the pgxntool release PR, and never applies the `commit-with-no-tests` label.** Both are explicit human actions -- see [Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review). + The pgxntool-test PR is different again: it's never meant to be merged at + all, by anyone -- see + [Open Release Pull Requests](#open-release-pull-requests). - **The commits in this skill don't need a separate per-commit confirmation gate.** Invoking `/release` at all is the user's explicit instruction to run this whole documented flow, commits included -- and none of those @@ -316,21 +322,31 @@ versions of this skill did) never runs CI at all, silently skipping every check this repo relies on. Instead, push the release branch and open a PR in each repo, same as any other change. -**Push order doesn't matter here, and pushing pgxntool-test first is not a -safeguard** -- an earlier draft of this step claimed it was, which was -wrong. pgxntool's `check-test-pr` job checks doc-only status *first*, purely -from this PR's own changed-file list -- no cross-repo lookup, no ordering -or timing dependency at all -- and only falls through to searching for a -paired pgxntool-test PR if that's false. A release-stamp PR only ever -touches `HISTORY.asc`/`README.asc`/`README.html`, all covered by -`DOC_EXTENSIONS`, so it should hit that bypass every time regardless of -which repo's PR goes up first. (The pairing search existing at all is a -general, latent race in `check-test-pr` for *any* paired PR, release or -not -- ordinary PRs just tend to survive it better since a failed first CI -run gets fixed by a follow-up push, whereas a release PR is pushed once and -not iterated on nearly as much. Worth its own issue if it ever actually -bites; not something this skill needs to work around, since it never -depends on the pairing search succeeding.) +**Why pgxntool-test needs a PR too, even though it has nothing to stamp.** +pgxntool's release PR is doc-only (`HISTORY.asc`/`README.asc`/`README.html` +only), and pgxntool's `check-test-pr` job skips the actual Postgres test +matrix *unconditionally* for a doc-only PR -- no fallback to testing against +master, regardless of whether a paired pgxntool-test PR exists. So without +a companion PR, the real content of this release would get **zero** CI test +coverage before it merges. What actually provides that coverage: an empty +commit is *not* doc-only by pgxntool-test's own check (zero changed files +trips a different guard), so its `test` job runs and resolves the paired +pgxntool branch (`release-VERSION`, matched by name+account) -- exercising +the real Postgres suite against the actual release content, just via +pgxntool-test's CI instead of pgxntool's. + +**This means the pgxntool-test PR exists purely to trigger that test run -- +it must never be merged.** It changes nothing pgxntool-test actually wants +on its master. Say so explicitly in the PR itself so nobody merges it by +habit; close it (don't merge it) once its CI is green -- see +[Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review). + +(Push order between the two repos doesn't matter for pgxntool's own +`check-test-pr` gate -- it checks doc-only status purely from this PR's own +changed-file list, with no cross-repo lookup, before it would ever look for +a pairing. That pairing-search race is a real, general latent issue in +`check-test-pr` for any paired PR, but this skill doesn't depend on it +succeeding either way.) Push both branches directly to the Postgres-Extensions remotes, never a fork, and open both PRs -- these can run as parallel subagents (see Process @@ -342,8 +358,12 @@ git commit --allow-empty -m "Stamp VERSION" git push PGXNTOOL_TEST_UPSTREAM release-VERSION gh pr create --repo Postgres-Extensions/pgxntool-test \ --base master --head release-VERSION \ - --title "Release VERSION" \ - --body "Companion stamp for pgxntool release-VERSION." + --title "DO NOT MERGE: Release VERSION (CI trigger only)" \ + --body "This PR exists only to trigger a real CI test run of the paired \ +pgxntool release-VERSION branch -- pgxntool's own release PR is doc-only \ +and skips the Postgres test matrix entirely. It changes nothing in \ +pgxntool-test (empty commit) and must NOT be merged. Close it once its CI \ +is green." ``` ```bash @@ -352,7 +372,7 @@ git push PGXNTOOL_UPSTREAM release-VERSION gh pr create --repo Postgres-Extensions/pgxntool \ --base master --head release-VERSION \ --title "Release VERSION" \ - --body "Stamps HISTORY.asc for VERSION. Companion: pgxntool-test release-VERSION." + --body "Stamps HISTORY.asc for VERSION. Companion (do-not-merge, CI trigger only): pgxntool-test release-VERSION." ``` Per this project's CI-monitoring rule, immediately start a background @@ -371,13 +391,21 @@ bash .claude/skills/ci/scripts/monitor-ci.sh pgxntool release-VERSION "" Date: Tue, 28 Jul 2026 17:52:35 -0500 Subject: [PATCH 11/12] release skill: more PR #43 review feedback; merge two bats suites release/SKILL.md: - Since-last-release findings: always ask the user how to document a missing STABLE entry instead of letting the agent silently write it -- a missing entry there means something already went wrong upstream of the release. - Create the release branch: fix stale/wrong claim that matching branch names 'lets pgxntool-test's CI automatically pair its release PR with pgxntool's' (debunked last commit -- doc-only bypass never reaches the pairing search). State plainly that the pgxntool-test PR is strictly a CI trigger. - Reorder the STABLE section entries: back to a numbered list. This one's small, compact, and never cross-referenced elsewhere in the doc, unlike the top-level Step N numbering that was ditched -- no renumbering-churn risk here. - Closing the pgxntool-test CI-trigger PR is now an explicit, separately-confirmed human action (point the user at the CI run and wait for their go-ahead) rather than an automatic step tied to CI going green. test/standard/: merged base-mk-include-guard.bats and pgxntool-version.bats into base-mk-misc.bats. Each was its own suite file for no real reason -- both are small, both only need a plain foundation checkout, and every additional suite file costs real setup overhead. No reason to keep paying that twice. Co-Authored-By: Claude Sonnet 5 --- test/standard/base-mk-include-guard.bats | 46 ---------------- ...gxntool-version.bats => base-mk-misc.bats} | 54 +++++++++++++++---- 2 files changed, 43 insertions(+), 57 deletions(-) delete mode 100755 test/standard/base-mk-include-guard.bats rename test/standard/{pgxntool-version.bats => base-mk-misc.bats} (58%) diff --git a/test/standard/base-mk-include-guard.bats b/test/standard/base-mk-include-guard.bats deleted file mode 100755 index e74166b..0000000 --- a/test/standard/base-mk-include-guard.bats +++ /dev/null @@ -1,46 +0,0 @@ -#!/usr/bin/env bats - -# Test: base.mk double-inclusion safety (issue #50) -# -# base.mk can end up included twice in one `make` run: an extension's own -# .mk module includes it, and the extension's Makefile includes it directly -# too. Without a guard, every target gets redefined, producing -# overriding-recipe/ignoring-old-recipe warnings at parse time. This is -# reproduced here by writing a throwaway Makefile that includes -# pgxntool/base.mk directly AND via an intermediate module -- mirroring the -# real-world scenario from the issue -- and confirming make parses it -# without those warnings. (Confirmed by hand that removing the ifndef/endif -# guard reproduces dozens of these warnings for this exact setup.) - -load ../lib/helpers - -setup_file() { - setup_topdir - - load_test_env "base-mk-include-guard" - ensure_foundation "$TEST_DIR" -} - -setup() { - load_test_env "base-mk-include-guard" - cd_test_env -} - -@test "base.mk tolerates being included twice in one make run" { - cat > double-include-module.mk <<'EOF' -include pgxntool/base.mk -EOF - cat > double-include-test.mk <<'EOF' -include pgxntool/base.mk -include double-include-module.mk -EOF - - run make -f double-include-test.mk -n all 2>&1 - assert_success - assert_not_contains "$output" "overriding recipe" - assert_not_contains "$output" "ignoring old recipe" - - rm -f double-include-module.mk double-include-test.mk -} - -# vi: expandtab sw=2 ts=2 diff --git a/test/standard/pgxntool-version.bats b/test/standard/base-mk-misc.bats similarity index 58% rename from test/standard/pgxntool-version.bats rename to test/standard/base-mk-misc.bats index 723032f..d176a41 100644 --- a/test/standard/pgxntool-version.bats +++ b/test/standard/base-mk-misc.bats @@ -1,34 +1,66 @@ #!/usr/bin/env bats -# Test: pgxntool-version +# Test: misc small base.mk behaviors # -# Tests bin/version's HISTORY.asc parsing in isolation (a stamped -# version, an unreleased "STABLE" checkout, a missing file, an empty file, -# malformed first lines), that `make pgxntool-version` is correctly wired to -# it inside a real embedded pgxntool copy, and (CRITICAL -- see comment -# below) that it matches the actual, unmodified pgxntool checkout this test -# suite is running against. +# Grouped into one suite because each is small enough that a dedicated +# suite file isn't worth the per-suite overhead -- every test below only +# needs a plain foundation checkout, no suite-specific fixture. +# +# - base.mk double-inclusion safety (issue #50): base.mk can end up +# included twice in one `make` run: an extension's own .mk module +# includes it, and the extension's Makefile includes it directly too. +# Without a guard, every target gets redefined, producing +# overriding-recipe/ignoring-old-recipe warnings at parse time. This is +# reproduced here by writing a throwaway Makefile that includes +# pgxntool/base.mk directly AND via an intermediate module -- mirroring +# the real-world scenario from the issue -- and confirming make parses it +# without those warnings. (Confirmed by hand that removing the +# ifndef/endif guard reproduces dozens of these warnings for this exact +# setup.) +# - bin/version's HISTORY.asc parsing (in isolation -- a stamped version, +# an unreleased "STABLE" checkout, a missing file, an empty file, +# malformed first lines), that `make pgxntool-version` is correctly +# wired to it inside a real embedded pgxntool copy, and (CRITICAL -- see +# comment below) that it matches the actual, unmodified pgxntool checkout +# this test suite is running against. load ../lib/helpers setup_file() { setup_topdir - # Independent test - gets its own isolated environment with foundation TEST_REPO - load_test_env "pgxntool-version" + load_test_env "base-mk-misc" ensure_foundation "$TEST_DIR" export VERSION_SCRIPT="$PGXNREPO/bin/version" - export SCRATCH_DIR="$TEST_DIR/pgxntool-version-tests" + export SCRATCH_DIR="$TEST_DIR/base-mk-misc-scratch" } setup() { - load_test_env "pgxntool-version" + load_test_env "base-mk-misc" + cd_test_env rm -rf "$SCRATCH_DIR" mkdir -p "$SCRATCH_DIR" } +@test "base.mk tolerates being included twice in one make run" { + cat > double-include-module.mk <<'EOF' +include pgxntool/base.mk +EOF + cat > double-include-test.mk <<'EOF' +include pgxntool/base.mk +include double-include-module.mk +EOF + + run make -f double-include-test.mk -n all 2>&1 + assert_success + assert_not_contains "$output" "overriding recipe" + assert_not_contains "$output" "ignoring old recipe" + + rm -f double-include-module.mk double-include-test.mk +} + @test "bin/version prints a stamped version number" { echo "2.1.0" > "$SCRATCH_DIR/HISTORY.asc" From a436d548e6901eabd6c87a54fad8c53e6b604aea Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 28 Jul 2026 18:13:59 -0500 Subject: [PATCH 12/12] release skill: add explicit check that CI actually runs test-all This exact bug was just found live: run-tests.yml's 'Run tests' step runs 'make test', silently excluding everything in test/extra/. The release process leans on pgxntool-test's own CI (triggered by the companion PR) to give the release content real Postgres test coverage -- that guarantee is only as good as the workflow actually running the full suite, and nothing was checking it. New step checks both the workflow definition (grep for 'make test-all' in run-tests.yml) and a recent real run (make test/test-extra print a 'Tip: ... test-all for everything' banner that test-all never does -- its absence from actual CI logs confirms the YAML and the executed run agree). Placed early, right after pre-flight, since everything downstream assumes this guarantee holds. Co-Authored-By: Claude Sonnet 5 --- .claude/skills/release/SKILL.md | 99 ++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 4d33234..2e704cd 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -5,7 +5,7 @@ description: | HISTORY.asc updates, and pushing to the main Postgres-Extensions GitHub repos. Use when user says "release", "create release", "tag version", or "/release" -allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Bash(git merge:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr close:*), Bash(.claude/skills/ci/scripts/monitor-ci.sh:*), Read, Edit +allowed-tools: Bash(.claude/skills/release/scripts/release-preflight.sh:*), Bash(git tag:*), Bash(git commit:*), Bash(git push:*), Bash(git checkout:*), Bash(git status:*), Bash(git log:*), Bash(git remote:*), Bash(git rev-parse:*), Bash(git branch:*), Bash(git fetch:*), Bash(git diff:*), Bash(git merge:*), Bash(gh pr create:*), Bash(gh pr view:*), Bash(gh pr checks:*), Bash(gh pr close:*), Bash(gh run list:*), Bash(gh run view:*), Bash(grep:*), Bash(.claude/skills/ci/scripts/monitor-ci.sh:*), Read, Edit --- # /release @@ -121,6 +121,54 @@ The script checks: - `PGXNTOOL_UPSTREAM` - remote name for pgxntool (e.g., "upstream") - `PGXNTOOL_TEST_UPSTREAM` - remote name for pgxntool-test (e.g., "upstream") +## Verify CI Runs test-all + +This whole release process leans on pgxntool-test's own CI (triggered by the +companion PR in +[Open Release Pull Requests](#open-release-pull-requests)) to give the +release content real Postgres test coverage -- see that section for why. +That guarantee is only as good as `run-tests.yml` actually running the full +suite, and this has silently regressed before (it was found running only +`make test`, silently excluding everything in `test/extra/`). Check both the +workflow definition and a recent real run before trusting it -- don't rely +on the YAML alone. + +**Check the workflow definition:** + +```bash +grep -n "run: make" ../pgxntool-test/.github/workflows/run-tests.yml +``` + +The `Run tests` step must read `make test-all`. If it reads `make test` or +`make test-extra`, that's the same bug recurring -- stop and fix it (or +confirm someone already is) before continuing. + +**Check a recent actual run agrees with the code.** `make test` and `make +test-extra` both print a `Tip: Use 'make test-extra' ... or 'make test-all' +for everything` banner before and after the suite; `make test-all` never +prints it. Pull the most recent successful CI run's logs and confirm that +banner is absent: + +```bash +run_id=$(gh run list --repo Postgres-Extensions/pgxntool-test --workflow CI \ + --status success --limit 1 --json databaseId --jq '.[0].databaseId') +gh run view "$run_id" --repo Postgres-Extensions/pgxntool-test --log \ + | grep -i "Use 'make test-extra'" +``` + +Any match means the actual run did NOT use `test-all`, regardless of what +the checked-in YAML says -- the workflow the runner executed and the +workflow on record can differ (e.g. `run-tests.yml` is a reusable workflow +pulled from a specific ref -- see its own `CROSS-REPO REUSABLE WORKFLOW` +comment in `../pgxntool/.github/workflows/ci.yml`). No match is a good sign +but confirm it's actually checking the right step's output, not silently +matching nothing due to a `gh` error. + +**If either check fails:** STOP. This is a testing-infrastructure gap, not +something to route around release-by-release. Fix `run-tests.yml` (or hand +this off, e.g. to a dedicated follow-up PR/agent) and only continue the +release once both checks pass. + ## Launch API Documentation Review Agents Immediately after pre-flight passes, launch the review agents below via the @@ -220,11 +268,13 @@ findings in hand before that happens. For each finding: - **Since-last-release findings (review A):** a behavior change without a - STABLE entry MUST be fixed before continuing. Either add the missing entry - to the STABLE section now (folded into the HISTORY.asc edit below), or ask - the user how they want it documented — do not release with an undocumented - behavior change. API items added/removed by these commits but missing from - README.asc must also be fixed (edit README.asc) before continuing. + STABLE entry means something already went wrong upstream of this release + (it should have been documented when it merged) -- **always ask the user** + how they want it documented; never silently write the entry yourself. Fold + their answer into the STABLE section as part of the HISTORY.asc edit below. + Do not release with an undocumented behavior change. The same goes for API + items added/removed by these commits but missing from README.asc: ask, then + edit README.asc, before continuing. - **Comprehensive findings (review B):** these may include pre-existing drift unrelated to this release. Show the findings to the user and ask whether to fix now (as part of this release), file as follow-up work, or @@ -240,10 +290,9 @@ Summarize what was found and how each item was resolved before moving on. ### Create the release branch -Both repos get a same-named branch (see **Release branch** in Terminology). -Using the same name on both, pushed to the same (upstream) account, is what -lets pgxntool-test's CI automatically pair its release PR with pgxntool's -- -see [Open Release Pull Requests](#open-release-pull-requests). +Both repos get a same-named branch (see **Release branch** in Terminology; +the empty pgxntool-test PR is strictly to trigger a CI run -- see +[Open Release Pull Requests](#open-release-pull-requests)). ```bash cd ../pgxntool && git checkout -b release-VERSION @@ -257,12 +306,12 @@ By this point (the gate in every entry this release needs is already in place. Before stamping, sort those entries -- most important first -- into: -- Breaking changes -- anything that could break an existing consumer's - build, tests, or behavior -- Non-breaking behavior changes -- existing behavior changed, but nothing - should break as a result (expect this category to be rare) -- New features/additions -- new targets, variables, scripts, etc. -- Bugfixes +1. Breaking changes -- anything that could break an existing consumer's + build, tests, or behavior +2. Non-breaking behavior changes -- existing behavior changed, but nothing + should break as a result (expect this category to be rare) +3. New features/additions -- new targets, variables, scripts, etc. +4. Bugfixes An entry that fits more than one category goes under the highest (earliest) one that applies. This is a one-time pass over *this release's* entries @@ -338,7 +387,10 @@ pgxntool-test's CI instead of pgxntool's. **This means the pgxntool-test PR exists purely to trigger that test run -- it must never be merged.** It changes nothing pgxntool-test actually wants on its master. Say so explicitly in the PR itself so nobody merges it by -habit; close it (don't merge it) once its CI is green -- see +habit. Closing it is not an automatic action just because its CI went +green -- point the user at the CI run and wait for their explicit +go-ahead, same as merging pgxntool's PR is a human action, not this +skill's -- see [Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review). (Push order between the two repos doesn't matter for pgxntool's own @@ -487,9 +539,16 @@ git push PGXNTOOL_TEST_UPSTREAM -f refs/tags/release pgxntool is already on master with the release branch merged in (see [Tag Both Repos](#tag-both-repos)); delete its now-merged local release -branch. pgxntool-test's release branch was never merged (see -[Open Release Pull Requests](#open-release-pull-requests)) -- close its -CI-trigger PR and delete the branch on both ends: +branch. + +pgxntool-test's release branch was never merged (see +[Open Release Pull Requests](#open-release-pull-requests)). Before closing +its CI-trigger PR, point the user at its CI run and ask them to confirm +it's OK to close as part of finishing the release -- this is a distinct +confirmation from the CI-passed check in +[Wait for CI, Then Hand Off for Review](#wait-for-ci-then-hand-off-for-review), +not implied by it. Once confirmed, close it and delete the branch on both +ends: ```bash cd ../pgxntool && git branch -d release-VERSION