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

.claude/skills/release/SKILL.md

@@ -109,11 +109,27 @@ 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 AND the fork (`origin`) are both in sync with upstream —
+   the script self-heals both independently, using `gh repo sync` (the CLI
+   equivalent of GitHub's "Sync fork" button) rather than hand-rolled git
+   plumbing: first `gh repo sync` (no args, from inside the local clone) to
+   fast-forward local master from its upstream parent, then `gh repo sync
+   <owner>/<fork>` to fast-forward the fork on GitHub directly. Both are
+   checked and fixed unconditionally, not just when local was found behind
+   — the fork can go stale independently of local (e.g. a prior run
+   updated local but was interrupted before reaching the fork sync, or
+   local already matched upstream so there was never a reason to touch the
+   fork). `gh repo sync` only ever fast-forwards — it is never invoked with
+   `--force` here, so it fails (non-zero exit) rather than creating a merge
+   commit or a hard reset if a true fast-forward isn't possible on either
+   side. That failure is treated as a hard error requiring manual
+   resolution, never auto-merged, auto-rebased, or forced.
 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 +199,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 +484,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 +496,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

@@ -93,35 +93,68 @@ echo "pgxntool-test: $pgxntool_test_branch"
 [ "$pgxntool_test_branch" = "master" ] || errors+=("pgxntool-test: on branch '$pgxntool_test_branch', not master")
 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_head" = "$upstream_head" ]; then
-        echo "pgxntool: in sync with $PGXNTOOL_UPSTREAM/master ($local_head)"
+# 4. Sync local master and the fork from upstream
+#
+# Uses `gh repo sync` -- the CLI equivalent of GitHub's "Sync fork" button
+# -- rather than hand-rolled git fetch/merge/push plumbing. By default it
+# performs a fast-forward-only update and FAILS (non-zero exit) rather than
+# creating a merge commit or rewriting history if a true fast-forward isn't
+# possible. NEVER pass --force to either call below -- that switches gh to
+# a hard reset instead of refusing, which could destroy commits on
+# whichever side gets reset.
+#
+# Two independent syncs per repo, since either can go stale independently
+# of the other (e.g. a prior run updated local but was interrupted before
+# reaching the fork sync, or local already matched upstream from an earlier
+# session so there was never a reason to touch the fork):
+#   1. `gh repo sync` (no destination-repository argument, run from inside
+#      the local clone) updates local master from its upstream parent.
+#   2. `gh repo sync <owner>/<fork>` updates the fork on GitHub directly
+#      (the actual "Sync fork" button equivalent), independent of #1.
+# Both calls resolve their source (the upstream parent) via GitHub's own
+# fork-parent metadata, not our locally-configured remote names -- so this
+# works regardless of what the upstream remote happens to be named locally.
+fork_slug() {
+    local repo_path="$1"
+    git -C "$repo_path" remote get-url origin \
+        | sed -E 's#^(https://github\.com/|git@github\.com:)##; s#\.git$##'
+}
+
+sync_repo() {
+    local repo_path="$1"
+    local repo_label="$2"
+    local slug output
+
+    if ! slug=$(fork_slug "$repo_path"); then
+        errors+=("$repo_label: could not determine fork slug from 'origin' remote URL")
+        return
+    fi
+
+    echo "$repo_label: syncing local master from upstream (gh repo sync)..."
+    if output=$(cd "$repo_path" && gh repo sync 2>&1); then
+        if [ -n "$output" ]; then
+            echo "$output" | sed 's/^/  /'
+        fi
     else
-        echo "pgxntool: DIVERGED from $PGXNTOOL_UPSTREAM/master"
-        echo "  local:    $local_head"
-        echo "  upstream: $upstream_head"
-        warnings+=("pgxntool: local master diverges from $PGXNTOOL_UPSTREAM/master")
+        echo "$output" | sed 's/^/  /'
+        errors+=("$repo_label: gh repo sync (local) failed -- local master may have diverged from upstream in a way that isn't a clean fast-forward; resolve manually")
+        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)"
+    echo "$repo_label: syncing fork ($slug) from upstream (gh repo sync $slug)..."
+    if output=$(gh repo sync "$slug" 2>&1); then
+        if [ -n "$output" ]; then
+            echo "$output" | sed 's/^/  /'
+        fi
     else
-        echo "pgxntool-test: DIVERGED from $PGXNTOOL_TEST_UPSTREAM/master"
-        echo "  local:    $local_head"
-        echo "  upstream: $upstream_head"
-        warnings+=("pgxntool-test: local master diverges from $PGXNTOOL_TEST_UPSTREAM/master")
+        echo "$output" | sed 's/^/  /'
+        errors+=("$repo_label: gh repo sync (fork $slug) failed -- the fork's master may have diverged from upstream in a way that isn't a clean fast-forward; this is unexpected since the fork should be a pure mirror, and needs manual investigation rather than an automatic overwrite")
     fi
-fi
+}
+
+echo "--- Sync Status ---"
+sync_repo "$PGXNTOOL_DIR" "pgxntool"
+sync_repo "$PGXNTOOL_TEST_DIR" "pgxntool-test"
 echo
 
 # 5. Version checks

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*