diff --git a/.claude/skills/commit/guides/commit-message-format.md b/.claude/skills/commit/guides/commit-message-format.md index ee8b8cd..1417818 100644 --- a/.claude/skills/commit/guides/commit-message-format.md +++ b/.claude/skills/commit/guides/commit-message-format.md @@ -47,6 +47,8 @@ Co-Authored-By: Claude ``` - Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution) +- **A direct GitHub URL to the pgxntool PR or commit (e.g. `https://github.com/Postgres-Extensions/pgxntool/pull/63`) is an acceptable, and preferred, substitute for a raw commit hash.** A PR link stays valid even if the referenced commit is later rebased or amended (its hash changes, but the PR URL doesn't); a bare hash embedded in already-committed text goes stale silently in that case. Prefer linking the PR once one exists. +- If a raw hash was used and the referenced pgxntool commit is later rebased/amended (changing its hash) before your pgxntool-test PR merges, update the reference to the new hash before merging. This only matters pre-merge — once a commit has landed on master, don't rewrite it to fix a stale reference without explicit instruction from the user; rewriting shared history on a protected branch is a different (and much riskier) class of action than amending your own open PR branch. - Include brief summary (2-3 bullets) of pgxntool changes near top - Wrap code references in backticks diff --git a/.claude/skills/crossref-audit/SKILL.md b/.claude/skills/crossref-audit/SKILL.md new file mode 100644 index 0000000..f7a5b68 --- /dev/null +++ b/.claude/skills/crossref-audit/SKILL.md @@ -0,0 +1,112 @@ +--- +name: crossref-audit +description: | + Audit master in both pgxntool and pgxntool-test for paired commits (a code + change plus its corresponding test coverage) that are missing a + cross-reference to each other. Fixes safely when it's a single, recent, + tip-of-master commit; otherwise stops and asks. + + Use when: at the end of each round of work in this project (not just + session start — sessions run long), before rebasing a branch onto a fresh + master fetch, or when asked to check/audit cross-references. +allowed-tools: Bash(git:*), Bash(gh:*), Bash(bash .claude/skills/crossref-audit/scripts/audit.sh:*), Read +--- + +# /crossref-audit + +Check whether commits that landed on master in pgxntool and pgxntool-test +since the last release properly cross-reference their paired counterpart in +the other repo, per `.claude/skills/commit/guides/commit-message-format.md`. + +## Why this exists, and how it differs from `/commit` + +PRs merge via the GitHub website, by the user, outside AI control — Claude +doesn't see the merge happen, so a missing cross-reference can only be +caught *after the fact* by auditing what actually landed on master. This is +a different problem from `/commit`, which only covers composing a PR +branch's own commits *before* merge. + +## Running it cheaply, every round + +Run `bash .claude/skills/crossref-audit/scripts/audit.sh `. +This does steps 1 and most of step 3 below as a plain script — no LLM +tokens — and caches the last-checked master SHAs in +`/tmp/pgxntool-crossref-audit-state`, updated only on a clean result. That +means most rounds cost nothing more than reading one line of output: + +- `crossref-audit: no new commits on either master since last clean check.` — done, nothing to interpret +- `crossref-audit: clean. Checked N / M commit(s)...` — done +- `crossref-audit: FLAGGED -- ...` — read the flagged list and apply the judgment in Step 2/Step 4 below + +A flagged result is a heuristic, not a verdict — the script correlates +commits via the "(issue #N)" phrasing this project's commits use for +cross-repo issue references (not bare "(#N)", which is usually just the +local PR number), and only checks for the *presence* of a plausible +hash/URL pattern. It can both over-flag (a real pairing that already has a +reference in an unusual phrasing) and under-flag (a pairing it didn't +correlate). Treat "clean" as "nothing obviously wrong", not a guarantee. + +## Step 2: Identify genuinely paired commits (only needed when something is flagged, or you're doing a manual review) + +Not every commit needs a cross-reference. It's only expected when a commit +in one repo is a *functional* code+test pairing with a commit in the +other — e.g. a pgxntool bug fix with dedicated pgxntool-test BATS coverage +for it. It is NOT expected for: +- Single-repo, doc-only changes with no test implications +- Two commits that happen to fix similar-sounding problems independently in + each repo (e.g. each repo has its own separate `claude-code-review.yml` — + fixing both is two unrelated commits, not a pairing) + +Use issue numbers, PR descriptions, and commit content to judge whether a +pairing is real. When genuinely unsure, ask the user rather than guessing. + +## Step 3: Check for a cross-reference (the script does this automatically; use this if reviewing manually) + +For each side of a genuine pairing, confirm the commit message references +the other repo — either a raw commit hash or (preferred, per the +commit-message-format guide) a GitHub PR/commit URL. Note: by convention +the pgxntool side only needs to describe related pgxntool-test changes in +prose (no hash required); the pgxntool-test side is expected to reference +pgxntool via hash or URL. + +## Step 4: Handle what you find + +**Nothing missing:** report that briefly and move on. + +**Exactly one commit missing a reference, and it's the current tip of +master, and it's newer than the last release tag:** you may fix it +directly: +1. Create an isolated worktree tracking `upstream/master` detached — do NOT + edit the shared checkout. +2. `git commit --amend` to add the missing cross-reference. Verify + `git diff ` is empty (message-only change, no content + drift) before pushing. +3. `git fetch upstream master` again immediately before pushing, to catch + any race, then + `git push --force-with-lease=master: upstream HEAD:master`. +4. Report the old and new SHA clearly. +5. Check whether any open PR branch (in either repo) was already rebased + onto the old, now-superseded SHA. If so, it needs re-rebasing onto the + new tip — git will typically recognize the old commit as + patch-equivalent and skip re-applying it cleanly, but re-run the full + test suite on the result before pushing the re-rebase. + +**More than one commit missing a reference:** STOP. Do not fix anything +automatically. Report the full list to the user and wait for direction — +amending multiple non-contiguous commits requires real history surgery +(interactive rebase), which is much higher-risk than touching a single tip +commit. + +**Any affected commit is at or before the last release tag:** NEVER amend +it, regardless of how many commits are affected, unless the user explicitly +tells you to for that specific commit. + +## Constraints + +- This audit only ever touches already-merged master commits, and only ever + the single tip commit under the conditions above. It never touches an + open PR's own commits as part of the audit itself — fixing an open PR's + commit (not yet shared/protected history) is a normal, low-risk edit and + doesn't need this skill's caution, just do it directly. +- Always re-run the full test suite after any amend-and-force-push, and + after any resulting PR-branch rebase, before considering the fix done. diff --git a/.claude/skills/crossref-audit/scripts/audit.sh b/.claude/skills/crossref-audit/scripts/audit.sh new file mode 100755 index 0000000..b7a8492 --- /dev/null +++ b/.claude/skills/crossref-audit/scripts/audit.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# audit.sh [pgxntool-dir] [pgxntool-test-dir] +# +# Cheap, stateful audit for missing cross-references between paired commits +# on master in pgxntool and pgxntool-test. Meant to run every round of a +# long session, not just once at startup, without burning real tokens on +# repeat rounds: it fetches both masters, and if neither has moved since the +# last CLEAN (nothing-flagged) check, exits immediately. State is only +# updated on a clean result -- a flagged issue keeps resurfacing every round +# until it's actually fixed, rather than being silently forgotten. +# +# This is a heuristic aid, not authoritative: it correlates commits via the +# "(issue #N)" phrasing this project's commits use for cross-repo issue +# references (not bare "(#N)", which is usually just the local PR number). +# A flagged result still needs a real judgment call before acting -- see +# SKILL.md. Absence of a flag means "nothing obviously wrong found", not a +# guarantee. +# +# Arguments: +# pgxntool-dir : path to a pgxntool checkout (default: ../pgxntool) +# pgxntool-test-dir : path to a pgxntool-test checkout (default: .) +# +# Exit codes: +# 0 : clean (nothing new since last clean check, or nothing flagged) +# 1 : one or more candidate pairings flagged for review +# 2 : usage/environment error + +set -euo pipefail + +PGXNTOOL_DIR="${1:-${PGXNTOOL_DIR:-../pgxntool}}" +PGXNTOOL_TEST_DIR="${2:-.}" +STATE_FILE="${CROSSREF_AUDIT_STATE:-/tmp/pgxntool-crossref-audit-state}" + +for d in "$PGXNTOOL_DIR" "$PGXNTOOL_TEST_DIR"; do + # -e (not -d): a linked worktree's .git is a FILE pointing at the main + # repo's git-dir, not a directory. + [ -e "$d/.git" ] || { echo "ERROR: '$d' is not a git checkout" >&2; exit 2; } +done + +git -C "$PGXNTOOL_DIR" fetch upstream master --tags -q +git -C "$PGXNTOOL_TEST_DIR" fetch upstream master --tags -q + +PGXN_HEAD=$(git -C "$PGXNTOOL_DIR" rev-parse upstream/master) +TEST_HEAD=$(git -C "$PGXNTOOL_TEST_DIR" rev-parse upstream/master) + +# Derive the org/repo path from the configured remote rather than hardcoding +# it, so this keeps working under a fork or rename. +PGXN_REPO_PATH=$(git -C "$PGXNTOOL_DIR" remote get-url upstream | sed -E 's#^(https://github\.com/|git@github\.com:)##; s#\.git$##') + +if [ -f "$STATE_FILE" ]; then + read -r LAST_PGXN LAST_TEST < "$STATE_FILE" || true + if [ "${LAST_PGXN:-}" = "$PGXN_HEAD" ] && [ "${LAST_TEST:-}" = "$TEST_HEAD" ]; then + echo "crossref-audit: no new commits on either master since last clean check. Nothing to do." + exit 0 + fi +fi + +last_release_tag() { + git -C "$1" tag --sort=-creatordate | grep -vi '^release$' | head -1 +} + +PGXN_TAG=$(last_release_tag "$PGXNTOOL_DIR") +TEST_TAG=$(last_release_tag "$PGXNTOOL_TEST_DIR") + +# An empty tag would make "$TAG"..upstream/master collapse to plain +# ..upstream/master, which git quietly reinterprets as HEAD..upstream/master +# instead of the intended unbounded range. +if [ -z "$PGXN_TAG" ] || [ -z "$TEST_TAG" ]; then + echo "ERROR: no release tag found in one or both repos" >&2 + exit 2 +fi + +pgxn_commits=$(git -C "$PGXNTOOL_DIR" log --format='%H %s' "$PGXN_TAG"..upstream/master) +test_commits=$(git -C "$PGXNTOOL_TEST_DIR" log --format='%H %s' "$TEST_TAG"..upstream/master) + +if [ -z "$pgxn_commits" ] && [ -z "$test_commits" ]; then + echo "crossref-audit: no commits since last release ($PGXN_TAG / $TEST_TAG) in either repo." + printf '%s %s\n' "$PGXN_HEAD" "$TEST_HEAD" > "$STATE_FILE" + exit 0 +fi + +# Extract issue numbers from "(issue #N)"-style phrasing specifically (not +# bare "#N", which is usually just the local repo's own PR number and would +# cause false cross-repo pairings since PR numbering is independent per repo). +# +# `|| true` is required here, not cosmetic: under `set -e`, a `grep` (or a +# pipeline ending in one) that matches nothing exits 1, and since this +# function's return status is that pipeline's status, calling it for a +# subject line with no "issue #N" (the common case -- most commits don't +# reference one) would otherwise abort the whole script immediately. +extract_issue_refs() { grep -oiE 'issue #[0-9]+' <<<"$1" | grep -oE '[0-9]+' | sort -u || true; } + +flagged=0 +report="" + +while IFS= read -r line; do + [ -z "$line" ] && continue + sha=${line%% *} + subj=${line#* } + issues=$(extract_issue_refs "$subj") + [ -z "$issues" ] && continue + for issue in $issues; do + match=$(grep -iE "issue #${issue}([^0-9]|$)" <<<"$test_commits" || true) + [ -z "$match" ] && continue + + body=$(git -C "$PGXNTOOL_DIR" log -1 --format='%b' "$sha") + if ! grep -qi 'pgxntool-test' <<<"$body"; then + report+=$'\n'" PGXNTOOL $sha (issue #$issue): \"$subj\" -- no mention of pgxntool-test in body" + flagged=1 + fi + + # Reuse $match (already bounded to "issue #$issue" exactly, not a numeric + # prefix of it) rather than re-deriving test_sha with a looser pattern. + test_sha=$(head -1 <<<"$match" | cut -d' ' -f1) + if [ -n "$test_sha" ]; then + test_body=$(git -C "$PGXNTOOL_TEST_DIR" log -1 --format='%b' "$test_sha") + if ! grep -qiE "[0-9a-f]{7,40}|github\.com/${PGXN_REPO_PATH}/(pull|commit)/" <<<"$test_body"; then + report+=$'\n'" PGXNTOOL-TEST $test_sha (issue #$issue): no hash or pgxntool PR/commit URL found in body" + flagged=1 + fi + fi + done +done <<<"$pgxn_commits" + +if [ "$flagged" -eq 1 ]; then + echo "crossref-audit: FLAGGED -- possible missing cross-reference(s):" + echo "$report" + echo + echo "Confirm each is a genuine code+test pairing before acting (a shared 'issue #N' string is a strong signal but still worth a sanity check) -- see SKILL.md for the fix-vs-ask rules. State file NOT updated; this will resurface every round until resolved." + exit 1 +fi + +pgxn_count=$(grep -c . <<<"$pgxn_commits" || true) +test_count=$(grep -c . <<<"$test_commits" || true) +echo "crossref-audit: clean. Checked $pgxn_count pgxntool / $test_count pgxntool-test commit(s) since last release ($PGXN_TAG / $TEST_TAG)." +printf '%s %s\n' "$PGXN_HEAD" "$TEST_HEAD" > "$STATE_FILE" +exit 0 + +# vi: expandtab ts=2 sw=2 diff --git a/CLAUDE.md b/CLAUDE.md index 6539512..530e195 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,6 +46,16 @@ adding commits, modifying PR descriptions, or any other PR-level action. `sed -i` (and similar in-place file-rewriting tools) can silently drop a file's executable bit -- this has caused real regressions in this repo (a script losing its `+x` bit cascaded into a wide swath of unrelated-looking test failures before the actual cause was found). After using `sed -i` or similar on any file, check `git diff --summary` for a `100755 => 100644` mode change and restore `chmod +x` before doing anything else with it. +## PR-Based Workflow: Merges Happen Outside AI Control + +**This project uses a GitHub PR workflow, not direct commits to master.** The `/commit` skill's two-phase cross-reference process (commit pgxntool, capture its hash, commit pgxntool-test referencing it) was designed for an earlier direct-commit era and still applies to *composing a PR branch's own commits* before merge — but actually merging a PR happens via the GitHub website, by the user, outside AI control. Claude never commits directly to master and does not control when or how a PR lands, with one narrow, safety-gated exception: `crossref-audit`'s Step 4 may amend and force-push a single tip-of-master commit to add a missing cross-reference, under the specific conditions documented there (never more than one commit, never at or before the last release tag, content-diff verified empty before pushing). + +Because of that, whether a paired PR's cross-reference actually made it onto master can only be verified *after the fact*, once both sides are already merged — see `crossref-audit` below. + +### End of Each Round: Check for Missing Cross-References + +**At the end of each round of work in pgxntool or pgxntool-test (not just once at session start — sessions here run long), and before rebasing any branch onto a fresh master fetch**, run the `crossref-audit` skill's script: `bash .claude/skills/crossref-audit/scripts/audit.sh `. It fetches both masters, caches the last-checked SHAs, and exits immediately with a one-line "nothing new" if neither has moved since the last clean check — so repeating it every round costs near-zero tokens in the common case. Only read further into the skill's rules if it reports something flagged; follow those rules exactly, especially around when it is and isn't safe to amend an already-merged commit. + ## Using Subagents **CRITICAL**: Always use ALL available subagents. Subagents are domain experts that provide specialized knowledge and should be consulted for their areas of expertise. @@ -69,6 +79,7 @@ These subagents are already available in your context - you don't need to discov The `/commit` skill lives in `.claude/skills/commit/` with a preprocessing script and format guide. The `/test` skill lives in `.claude/skills/test/` with a TAP-parsing test runner. +The `/crossref-audit` skill lives in `.claude/skills/crossref-audit/` — audits master in both repos for paired commits missing a cross-reference to each other (see "PR-Based Workflow" above). Other commands (worktree, pr, pgxntool-update) remain in `.claude/commands/`. ## What This Repo Is