diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index f94dac6..972d306 100644 --- a/.claude/skills/release/SKILL.md +++ b/.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 /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,7 +131,7 @@ 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. @@ -67,7 +139,34 @@ 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 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 diff --git a/CLAUDE.md b/CLAUDE.md index 2331ac0..dd12a9a 100644 --- a/CLAUDE.md +++ b/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.