release: add API documentation review agents to the process

.claude/skills/release/SKILL.md

@@ -18,6 +18,29 @@ 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.
+- **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
+  and generated targets are easy to miss that way. Run it from a scratch
+  directory containing nothing but a `Makefile` with
+  `include <path-to-pgxntool>/base.mk`. Two things to watch for:
+  - The output includes harmless noise from make's own recursive-submake
+    chatter (literal lines `Makefile`, `make[1]`, etc.) — filter these out,
+    they aren't real targets.
+  - Targets gated behind `ifeq`/`ifdef` conditionals that depend on files
+    not present in a bare scratch directory (e.g. `test-build`,
+    `clean-test-build`, which only appear when `test/build/*.sql` or
+    `test/install/*.sql` exist) won't show up this way. Read `base.mk`
+    directly for these rather than relying on `make list` alone.
 
 ---
 
@@ -45,7 +68,56 @@ 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: Determine Version Number
+## Step 2: 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.
+
+**Gate: do not proceed past Step 6 (Update HISTORY.asc 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 Step 5.
+
+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.
+
+**A. Since-last-release review** (focus: what MUST be documented in
+`HISTORY.asc`)
+
+- Scope: commits in `../pgxntool` between the `release` tag and `HEAD`
+  (`git log release..HEAD`, `git diff release..HEAD`), restricted to changes
+  that touch the user-facing API surface (see Terminology). **Commit
+  titles are not a reliable filter** — a commit can touch API-surface
+  behavior without saying so in its subject line. Always check the actual
+  file-level diff, don't just scan `git log --oneline`.
+- For every such change, compare against:
+  - `../pgxntool/HISTORY.asc` STABLE section — is the behavior change called
+    out there?
+  - `../pgxntool/README.asc` — if the change added, removed, renamed, or
+    changed the default/semantics of a documented item, is README.asc
+    updated to match?
+- Report: (1) behavior changes in the diff not mentioned in the STABLE
+  section, (2) API items added or removed by these commits but not reflected
+  in README.asc, (3) anything encountered that's ambiguously in/out of the
+  user-facing API surface.
+
+**B. Comprehensive review** (focus: current-state drift, regardless of
+history)
+
+- Scope: the full user-facing API surface (see Terminology) as it exists in
+  `../pgxntool` right now, compared against everything documented in
+  `../pgxntool/README.asc`.
+- Report: (1) documented items no longer present in code, (2) code-level
+  items in the user-facing API surface not documented in README.asc, (3)
+  documented behavior that no longer matches the code (wrong defaults,
+  wrong prerequisites, wrong descriptions), (4) anything encountered that's
+  ambiguously in/out of the user-facing API surface.
+- This review ignores git history entirely — it only compares the README
+  against the code as they exist right now.
+
+## Step 3: Determine Version Number
 
 If VERSION was not provided as an argument, ask the user:
 
@@ -59,15 +131,42 @@ Use AskUserQuestion:
 .claude/skills/release/scripts/release-preflight.sh VERSION
 ```
 
-## Step 3: Confirm HISTORY.asc
+## Step 4: Confirm HISTORY.asc
 
 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.
 
-## Step 4: Update HISTORY.asc and Commit
+## Step 5: 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.
+
+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.
+- **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
+  so future reviews don't re-flag it.
+
+Summarize what was found and how each item was resolved before moving on.
+
+## Step 6: Update HISTORY.asc and Commit
 
 1. Edit `../pgxntool/HISTORY.asc`: Replace the `STABLE` heading with the version number
 
@@ -88,7 +187,7 @@ Read `../pgxntool/HISTORY.asc` and show the user what's in the STABLE section.
    cd ../pgxntool && git commit -am "Stamp VERSION"
    ```
 
-## Step 5: Tag and Push pgxntool
+## Step 7: Tag and Push pgxntool
 
 **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.**
 
@@ -99,7 +198,7 @@ git push PGXNTOOL_UPSTREAM master
 git push PGXNTOOL_UPSTREAM VERSION
 ```
 
-## Step 6: Stamp, Tag, and Push pgxntool-test
+## Step 8: Stamp, Tag, and Push pgxntool-test
 
 **CRITICAL: Push to the Postgres-Extensions remote, not to a fork.**
 
@@ -113,7 +212,7 @@ git push PGXNTOOL_TEST_UPSTREAM master
 git push PGXNTOOL_TEST_UPSTREAM VERSION
 ```
 
-## Step 7: Update `release` Tag
+## Step 9: 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.
@@ -130,7 +229,7 @@ git tag -f release VERSION
 git push PGXNTOOL_TEST_UPSTREAM -f refs/tags/release
 ```
 
-## Step 8: Verify and Report
+## Step 10: Verify and Report
 
 ```bash
 cd ../pgxntool && git checkout master

CLAUDE.md

@@ -32,6 +32,25 @@ work on that, or did you mean to send this to a different session?"
 This applies to: editing PR branches, pushing to them, closing/reopening them,
 adding commits, modifying PR descriptions, or any other PR-level action.
 
+## Check Master Sync Before Branching
+
+**Before creating a new branch or worktree** in either repo, fetch the
+upstream remote and confirm local master isn't behind it — don't just check
+`git status`/branch name, actually compare the SHAs:
+
+```bash
+git fetch upstream master --quiet
+git rev-parse master upstream/master  # compare the two SHAs
+```
+
+If local master is behind, sync it before branching off it — don't branch
+from a stale base. Branching from a stale master risks redoing work that's
+already been fixed upstream.
+
+This is separate from (and broader than) the `/release` skill's own
+pre-flight sync check (Step 1) — that one only runs right before a release;
+this applies to *any* new branch or worktree in either repo.
+
 ## Git Commit Guidelines
 
 **CRITICAL**: Never attempt to commit changes on your own initiative. Always wait for explicit user instruction to commit. Even if you detect issues (like out-of-date files), inform the user and let them decide when to commit.
@@ -110,6 +129,67 @@ This repository contains template extension files in the `template/` directory w
 
 **Where it belongs**: `../pgxntool/.gitattributes` is the correct location - it controls what gets excluded from distributions when extension developers run `make dist`.
 
+### User-Facing API Surface of pgxntool
+
+This defines what counts as pgxntool's "public API" for the purposes of the
+`/release` skill's API documentation review (`.claude/skills/release/SKILL.md`,
+Step 2): the surface that must be kept in sync between the code and
+`../pgxntool/README.asc`, and whose behavior changes must be called out in
+`../pgxntool/HISTORY.asc`. It's a working definition, expected to evolve:
+when something doesn't clearly fit, don't guess — raise it and update this
+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.).
+   - 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
+     another target's lifecycle rather than being invoked directly (e.g.
+     `clean-test-build`, which only runs as a hook off `clean`; the
+     install-schedule file target that `installcheck` depends on). The
+     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
+     `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
+   reference or override them.
+3. **Variables prefixed `PGXNTOOL_` that are designed for override** —
+   defaulted with `?=`, or normalized via the validate/override pattern in
+   `lib.sh` (e.g. `pgxntool_validate_yesno`). Excludes pure internal
+   plumbing like `PGXNTOOL_DIR`, `PGXNTOOL_CONTROL_FILES`,
+   `PGXNTOOL_EXTENSIONS` — never meant to be set by users.
+4. **Scripts a user is realistically expected to invoke by hand**:
+   `setup.sh`, `pgxntool-sync.sh`, `update-setup-files.sh`, and `pgtle.sh`
+   (including its own CLI flags, not just the make targets that wrap it).
+5. **`DEBUG` is a special case**: its existence may be documented (it's
+   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.
+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*
+   consumer repos — it's effectively part of pgxntool's product, the same
+   way `README.asc` is. Treat substantive changes to it like changes to
+   `README.asc`: they should be reviewed for accuracy, and if they describe
+   a behavior change, call it out in `HISTORY.asc` too. (For other files
+   that are genuinely just internal dev documentation with no bearing on
+   consumer projects, doc-only changes don't need a `HISTORY.asc` entry.)
+7. Everything else is internal by default unless there's a specific
+   indication otherwise.
+
 ## Running Skills and Scripts
 
 **CRITICAL**: Always run skill scripts using relative paths from the repo root, never absolute paths. Absolute paths cause permission issues.