release skill: own PGXS-target modifications, pin DEBUG exception, auto-sync preflight

.claude/skills/release/SKILL.md

@@ -109,11 +109,21 @@ The script checks:
 1. Upstream remotes exist (pointing to Postgres-Extensions)
 2. Both working directories are clean
 3. Both repos are on master
-4. Local master is in sync with upstream
+4. Local master is in sync with upstream — if local is simply behind (a
+   clean fast-forward), the script self-heals: it fast-forwards local
+   master to `upstream/master` (`git merge --ff-only`, never a plain merge
+   or rebase) and pushes the result to `origin` (the fork), no user
+   decision needed. Master is only ever updated by fast-forward — it must
+   never gain a merge commit. If a true fast-forward isn't possible for any
+   reason (including local having commits upstream lacks, or `--ff-only`
+   itself refusing), the script does not merge, rebase, or force anything;
+   it stops and reports a hard error requiring manual resolution.
 5. Version format is valid and tag doesn't already exist
 6. HISTORY.asc has a STABLE section
 
 **If the script exits with errors:** STOP and show the errors to the user.
+This includes genuine master divergence from check 4 — that must be
+resolved by hand, not auto-merged.
 
 **If there are warnings:** Show them and ask the user how to proceed.
 
@@ -183,6 +193,12 @@ 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).
 
+Before launching, read (and fold into each agent's prompt) the "User-Facing
+API Surface of pgxntool" section in `CLAUDE.md` — it records standing
+exceptions (e.g. `DEBUG` being intentionally undocumented) that must not be
+re-flagged as findings; don't duplicate that list here, just point agents at
+it.
+
 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
 every agent concrete file paths, not a vague "review the code" instruction.
@@ -462,7 +478,10 @@ gh pr create --repo Postgres-Extensions/pgxntool-test \
 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."
+is green.
+
+Companion PR (should be merged normally once CI is green): \
+Postgres-Extensions/pgxntool#<pgxntool-pr-number>"
 ```
 
 ```bash
@@ -471,9 +490,25 @@ 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 (do-not-merge, CI trigger only): pgxntool-test release-VERSION."
+  --body "Stamps HISTORY.asc for VERSION. This PR should be merged normally.
+
+Companion PR (must NOT be merged -- exists only to trigger a real CI test \
+run): Postgres-Extensions/pgxntool-test#<pgxntool-test-pr-number>"
 ```
 
+**Why the wording matters:** an earlier release's pgxntool PR body read
+"Companion (do-not-merge, CI trigger only): pgxntool-test release-VERSION"
+-- a single run-on sentence where the do-not-merge parenthetical, read out
+of context, could be misread as applying to the PR you're looking at
+instead of its companion. State plainly, as its own sentence, whether
+*this* PR should or shouldn't be merged before ever mentioning the other
+one. Backfill the actual PR numbers into `<pgxntool-pr-number>` /
+`<pgxntool-test-pr-number>` once both PRs exist (each is opened after the
+other in the commands above, so the second `gh pr create` can reference
+the first PR's already-known number; if opening both concurrently via
+parallel subagents, edit the missing cross-reference in with `gh pr edit
+--body` immediately after both numbers are known).
+
 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

.claude/skills/release/scripts/release-preflight.sh

@@ -94,33 +94,78 @@ echo "pgxntool-test: $pgxntool_test_branch"
 echo
 
 # 4. Fetch and check sync
-echo "--- Sync Status ---"
-if [ -n "$PGXNTOOL_UPSTREAM" ]; then
-    git -C "$PGXNTOOL_DIR" fetch "$PGXNTOOL_UPSTREAM" 2>/dev/null
-    local_head=$(git -C "$PGXNTOOL_DIR" rev-parse HEAD)
-    upstream_head=$(git -C "$PGXNTOOL_DIR" rev-parse "$PGXNTOOL_UPSTREAM/master" 2>/dev/null || echo "unknown")
+#
+# If local master is simply behind upstream (a clean fast-forward is
+# possible - local has no commits upstream lacks), self-heal: fast-forward
+# local master to upstream/master and push the result to the fork remote
+# (origin), rather than just warning and leaving it to the user. Genuine
+# divergence (local has commits upstream doesn't have) is NOT auto-merged
+# or force-pushed - that's left as a hard error requiring manual resolution,
+# since silently merging/rebasing here could be lossy.
+sync_repo() {
+    local repo_path="$1"
+    local repo_label="$2"
+    local upstream_remote="$3"
+    local branch="$4"
+
+    local local_head upstream_head
+    local_head=$(git -C "$repo_path" rev-parse HEAD)
+    upstream_head=$(git -C "$repo_path" rev-parse "$upstream_remote/master" 2>/dev/null || echo "unknown")
+
     if [ "$local_head" = "$upstream_head" ]; then
-        echo "pgxntool: in sync with $PGXNTOOL_UPSTREAM/master ($local_head)"
-    else
-        echo "pgxntool: DIVERGED from $PGXNTOOL_UPSTREAM/master"
+        echo "$repo_label: in sync with $upstream_remote/master ($local_head)"
+        return
+    fi
+
+    if [ "$branch" != "master" ]; then
+        echo "$repo_label: DIVERGED from $upstream_remote/master"
         echo "  local:    $local_head"
         echo "  upstream: $upstream_head"
-        warnings+=("pgxntool: local master diverges from $PGXNTOOL_UPSTREAM/master")
+        echo "  not on master ('$branch') - skipping auto-sync"
+        errors+=("$repo_label: local master diverges from $upstream_remote/master and current branch is not master")
+        return
     fi
-fi
 
-if [ -n "$PGXNTOOL_TEST_UPSTREAM" ]; then
-    git -C "$PGXNTOOL_TEST_DIR" fetch "$PGXNTOOL_TEST_UPSTREAM" 2>/dev/null
-    local_head=$(git -C "$PGXNTOOL_TEST_DIR" rev-parse HEAD)
-    upstream_head=$(git -C "$PGXNTOOL_TEST_DIR" rev-parse "$PGXNTOOL_TEST_UPSTREAM/master" 2>/dev/null || echo "unknown")
-    if [ "$local_head" = "$upstream_head" ]; then
-        echo "pgxntool-test: in sync with $PGXNTOOL_TEST_UPSTREAM/master ($local_head)"
+    # Is local a strict ancestor of upstream (i.e. purely behind, with
+    # nothing of its own ahead)? Guarded with if/else so a non-zero exit
+    # from --is-ancestor (the expected "not an ancestor" case) doesn't trip
+    # set -e.
+    if git -C "$repo_path" merge-base --is-ancestor "$local_head" "$upstream_head"; then
+        echo "$repo_label: BEHIND $upstream_remote/master - fast-forwarding"
+        echo "  local:    $local_head"
+        echo "  upstream: $upstream_head"
+        # NEVER use a plain merge here -- master must only ever fast-forward,
+        # never gain a merge commit. --ff-only makes git refuse (non-zero
+        # exit) instead of creating a merge commit if a true fast-forward
+        # somehow isn't possible (e.g. a race with a concurrent push landing
+        # between the ancestor check above and this merge). Guarded with
+        # if/else, not `set -e`, so that refusal is caught and reported as
+        # the same divergence error below -- never retried with a different
+        # strategy (rebase, plain merge, force-push) and never forced.
+        if git -C "$repo_path" merge --ff-only "$upstream_remote/master"; then
+            git -C "$repo_path" push origin master
+            echo "$repo_label: fast-forwarded master $local_head -> $upstream_head and pushed to origin"
+        else
+            echo "$repo_label: fast-forward merge unexpectedly failed"
+            errors+=("$repo_label: git merge --ff-only failed even though local was expected to be a strict ancestor of $upstream_remote/master - resolve manually")
+        fi
     else
-        echo "pgxntool-test: DIVERGED from $PGXNTOOL_TEST_UPSTREAM/master"
+        echo "$repo_label: DIVERGED from $upstream_remote/master (local has commits upstream lacks)"
         echo "  local:    $local_head"
         echo "  upstream: $upstream_head"
-        warnings+=("pgxntool-test: local master diverges from $PGXNTOOL_TEST_UPSTREAM/master")
+        errors+=("$repo_label: local master has diverged from $upstream_remote/master (not a clean fast-forward) - resolve manually")
     fi
+}
+
+echo "--- Sync Status ---"
+if [ -n "$PGXNTOOL_UPSTREAM" ]; then
+    git -C "$PGXNTOOL_DIR" fetch "$PGXNTOOL_UPSTREAM" 2>/dev/null
+    sync_repo "$PGXNTOOL_DIR" "pgxntool" "$PGXNTOOL_UPSTREAM" "$pgxntool_branch"
+fi
+
+if [ -n "$PGXNTOOL_TEST_UPSTREAM" ]; then
+    git -C "$PGXNTOOL_TEST_DIR" fetch "$PGXNTOOL_TEST_UPSTREAM" 2>/dev/null
+    sync_repo "$PGXNTOOL_TEST_DIR" "pgxntool-test" "$PGXNTOOL_TEST_UPSTREAM" "$pgxntool_test_branch"
 fi
 echo
 

CLAUDE.md

@@ -178,8 +178,21 @@ section once resolved.
 1. **Make targets pgxntool defines**, including dev-helper targets like
    `list` and `print-%` (they exist specifically to help users introspect
    the Makefile). Excludes:
-   - Targets pgxntool inherits from PGXS unmodified (`install`,
-     `installcheck`, `submake-*`, etc.).
+   - Targets pgxntool inherits from PGXS **unmodified** (`install`,
+     `submake-*`, etc.) — but only to the extent they're genuinely
+     unmodified. The moment pgxntool adds or changes a prerequisite,
+     recipe body, or variable (e.g. `EXTRA_CLEAN`) on a PGXS-named target,
+     that modification is IN SCOPE and must be tracked like any other API
+     surface item — the underlying PGXS behavior pgxntool didn't touch
+     stays out of scope, but pgxntool's own authored changes to a
+     shared-name target don't get to hide behind "it's a PGXS target."
+     This is settled policy: it has already caused two real misses when
+     treated as excluded-by-name-alone — a stale README claim about
+     `test`'s prerequisites, and `installcheck: install` (issue #79, a
+     genuine ordering-bug fix to the PGXS-named `installcheck` target)
+     nearly being waved through as "out of scope" during 2.3.0 release
+     prep. Both should have been (and now are) tracked the same as any
+     other API surface change.
    - Pure generated-file targets (`META.json`, `meta.mk`, `control.mk`) —
      build plumbing, not something a user intentionally runs.
    - Conditionally-defined helper targets that exist purely to support
@@ -189,15 +202,10 @@ section once resolved.
      primary target they support (e.g. `test-build` itself) remains in
      scope if it's meant to be invoked directly and is independently
      documented.
-   - **Known gap**: pgxntool's own modifications to shared-name PGXS
-     targets (e.g. `test`'s prerequisites, `clean`'s `EXTRA_CLEAN`
-     additions) are currently excluded along with the rest of that
-     target's PGXS lineage, since the exclusion is by name alone. This has
-     already hidden real drift once (a stale README claim about `test`'s
-     prerequisites) — revisit if it keeps happening.
    - Finding these requires reading the actual source, not just running
-     `make list` — conditionally-gated targets, and anything else a
-     discovery tool can't fully enumerate, only show up by inspecting
+     `make list` — conditionally-gated targets, pgxntool's own additions
+     to PGXS-named targets, and anything else a discovery tool can't
+     fully enumerate, only show up by inspecting
      `base.mk`/`control.mk.sh`/`meta.mk.sh` directly.
 2. **Target prerequisites worth documenting by name** even when not
    invoked directly (e.g. `testdeps`), since extension authors may
@@ -220,6 +228,16 @@ section once resolved.
    fine for users to know it exists), but the specific level numbers are
    an internal implementation detail, not a documented contract — changing
    them is not a behavior change that needs a `HISTORY.asc` entry.
+   `DEBUG`'s absence from `README.asc`/`CLAUDE.md` documentation is NOT
+   itself a finding — API-documentation review agents (used by the
+   `/release` skill, see `.claude/skills/release/SKILL.md`) must not flag
+   "`DEBUG` exists but isn't documented" as a gap, regardless of how many
+   scripts reference it. This is the first entry in what should be treated
+   as a general pattern: when a documentation gap is judged intentional
+   rather than an oversight, record that decision here as a standing
+   exception so later reviews don't re-flag and re-litigate the same
+   question release after release — add new entries to this list rather
+   than raising them fresh each time.
 6. **`../pgxntool/CLAUDE.md` is in scope, not exempt as "doc-only."** Unlike
    ordinary dev-only documentation, this file ships into every consumer
    project via subtree and is written for AI agents working in *those*