diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 6310664..fe288d6 100644 --- a/.claude/skills/release/SKILL.md +++ b/.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 + /` 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#" ``` ```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#" ``` +**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 `` / +`` 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 diff --git a/.claude/skills/release/scripts/release-preflight.sh b/.claude/skills/release/scripts/release-preflight.sh index a756327..96e2bfd 100755 --- a/.claude/skills/release/scripts/release-preflight.sh +++ b/.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 /` 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 diff --git a/CLAUDE.md b/CLAUDE.md index a8d7885..cc376fe 100644 --- a/CLAUDE.md +++ b/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*