From cef9b733b92fdff2aa504b63490bcb02ad98b18e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 12 Oct 2026 18:21:44 -0500 Subject: [PATCH 1/2] Reframe commit guidance around PR branches Commits land on a PR branch, never on master; what reaches master is the merge result, which for a squash merge is the PR title and body. - CLAUDE.md, `/commit` skill and its format guide: describe composing commits on a PR branch for review. `/commit` now stops and asks if either repo is on `master`/`main`. The two-phase hash injection stays, as a review-time pointer; the cross-reference that must survive the merge goes in the PR description, preferably as a PR URL. - Force-push rules: master is never an agent's call (sole exception: `crossref-audit` Step 4); a PR branch is lower risk but still needs a literal ask in this repo pair, a deliberate exception to `../ai/PR.md`. The two duplicate statements in the `/commit` skill are merged into one. - `crossref-audit`: note that with squash merges it audits the PR description; fixing a pushed PR commit now means a new commit or asking. - `/commit` "After Pushing" points at the `/ci` skill instead of raw `gh run` commands. Changes only in pgxntool-test. No related changes in pgxntool. Fixes #93. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/commit/SKILL.md | 40 +++++++++++-------- .../commit/guides/commit-message-format.md | 11 +++-- .claude/skills/crossref-audit/SKILL.md | 9 +++-- CLAUDE.md | 15 ++++++- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md index cf0a6ca..9de9592 100644 --- a/.claude/skills/commit/SKILL.md +++ b/.claude/skills/commit/SKILL.md @@ -1,9 +1,9 @@ --- name: commit description: | - Create git commits across pgxntool and pgxntool-test repositories following project - standards. Preprocesses repo info, runs tests, drafts cross-referenced commit messages, - and executes two-phase commits with hash injection. + Compose git commits on PR branches across pgxntool and pgxntool-test for review, + following project standards. Preprocesses repo info, runs tests, drafts cross-referenced + commit messages, and executes two-phase commits with hash injection. Use when: "commit", "create commit", "commit changes", "/commit" allowed-tools: Bash(git status:*), Bash(git log:*), Bash(git add:*), Bash(git diff:*), Bash(git commit:*), Bash(git branch:*), Bash(bash .claude/skills/commit/scripts/*), Read, Edit, Task, Skill(test) @@ -11,13 +11,15 @@ allowed-tools: Bash(git status:*), Bash(git log:*), Bash(git add:*), Bash(git di # Commit Skill -Create git commits following project standards and safety protocols for pgxntool-test. +Compose commits on a PR branch for review, following project standards and safety +protocols. Commits never land on master directly; what reaches master is the merge +result (see CLAUDE.md "Git Commit Guidelines"). ## Critical Requirements | Rule | Details | |------|---------| -| **Git Safety** | Never update git config. Never force push -- not even to your own already-open/draft PR branch, not even when a task (a rebase, a restructure) seems to structurally require rewriting history. A task *implying* a rewrite is a signal to stop and ask, never a substitute for asking. Before any force-push, ask a literal question ("this needs a force-push to `` -- go ahead?") and wait for a real reply -- inferring a yes from the wording of the request is exactly what NOT to do. Add a new commit instead unless/until told otherwise. Never skip hooks unless requested. | +| **Git Safety** | Never update git config. Never skip hooks unless requested. Never force-push without asking -- see Restrictions. | | **Attribution** | No "Generated with Claude Code" in body. Co-Authored-By trailer is OK. | | **Multi-Repo** | Commit BOTH repos if both have changes (unless told otherwise). No empty commits. Never create a branch on a repo that has no changes. | | **Testing** | ALL tests must pass. ANY failure = STOP and ask user. No rationalizing failures. | @@ -39,7 +41,8 @@ Run the preprocessing script to collect all git data in one call: bash .claude/skills/commit/scripts/gather-repo-info.sh ../pgxntool . ``` -Review output. Identify which repos have changes. +Review output. Identify which repos have changes. If either repo with changes is on +`master`/`main`, STOP and ask -- never commit there. ### 2b. Check Debug Level Distribution (pgxntool .sh changes only) @@ -128,7 +131,9 @@ After tests pass AND receiving user approval: - Verify: `git status` 2. **Phase 2: Commit pgxntool-test** (if it has changes) - - Replace `[PGXNTOOL_COMMIT_HASH]` with captured hash + - Replace `[PGXNTOOL_COMMIT_HASH]` with captured hash -- a pointer during review only, + since merging creates new commits; the reference that must survive the merge goes in + the PR description (see the guide) - Stage, verify, commit, verify (same pattern) For detailed execution steps including pre-commit hook handling, read: @@ -144,19 +149,20 @@ EOF ## After Pushing: Monitor CI -After any `git push`, monitor GitHub Actions for failures: - -```bash -gh run list --limit 5 # list recent runs -gh run watch # watch the latest run in real time -gh run view # view a specific run's details -``` - -Check both repos if both were pushed. Report any CI failures to the user immediately — do not assume they are pre-existing or unrelated. +After any `git push`, monitor CI with the `/ci` skill, per CLAUDE.md "CI Monitoring +After Every Push". Report any CI failures to the user immediately — do not assume they are pre-existing or unrelated. ## Restrictions - DO NOT push unless explicitly asked -- DO NOT force-push a PR branch unless the user explicitly asks you to -- including your own unreviewed draft PR, and including when a rebase/restructure task seems to require it. Ask first, literally, every time; don't infer a yes from the task's wording. If a pushed commit needs correcting, add a new commit instead unless/until told otherwise. +- Force-pushing master/the default branch is never an agent's call (sole exception: + `crossref-audit` Step 4). +- DO NOT force-push a PR branch without asking first. It is lower risk (unmerged history + belonging only to that PR), but in this repo pairing ask a literal question ("this needs a + force-push to `` -- go ahead?") and wait for a real reply, every time -- including + for your own unreviewed draft PR, and including when a rebase/restructure task seems to + require rewriting history. A task *implying* a rewrite is a signal to ask, never a + substitute for asking. Otherwise add a new commit. This is a deliberate exception to + `../ai/PR.md`'s "Force-push / history rewriting" section. - DO NOT commit files with secrets (.env, credentials.json) - Never use `-i` flags (git commit -i, git rebase -i) diff --git a/.claude/skills/commit/guides/commit-message-format.md b/.claude/skills/commit/guides/commit-message-format.md index 5ee9281..a9aaff7 100644 --- a/.claude/skills/commit/guides/commit-message-format.md +++ b/.claude/skills/commit/guides/commit-message-format.md @@ -79,9 +79,9 @@ Add tests/updates for pgxntool commit [PGXNTOOL_COMMIT_HASH] (brief description) 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. +- Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution). The hash is a pointer for review only: squash and rebase merges both create new commits, so no branch commit's hash reaches master. +- **The cross-reference that has to survive the merge goes in the PR description, preferably as the pgxntool PR's URL** (e.g. `https://github.com/Postgres-Extensions/pgxntool/pull/63`). With squash merges the PR body becomes master's commit message, and a PR link stays valid when the referenced commit is rebased or amended; a bare hash goes stale silently. The same URL is also the preferred substitute for the hash in the commit message once the PR exists. +- If a raw hash was used and the referenced pgxntool commit is rebased/amended before your pgxntool-test PR merges, update the reference to the new hash. - Include brief summary (2-3 bullets) of pgxntool changes near top - Wrap code references in backticks @@ -98,6 +98,8 @@ If only one repo has changes: ## Two-Phase Commit Execution +Both phases commit to PR branches for review (the skill's step 2 stops on `master`/`main`). + ### Phase 1: Commit pgxntool 1. `cd ../pgxntool` @@ -155,4 +157,5 @@ The two repositories: - **pgxntool-test** (this repo) - Test harness with template files in `template/` directory. Commit pgxntool first (no placeholder), capture hash, then commit pgxntool-test (inject hash). -Result: pgxntool-test commit references the pgxntool commit it corresponds to. +Result: during review, the pgxntool-test commit references the pgxntool commit it +corresponds to; the PR description carries that reference through the merge. diff --git a/.claude/skills/crossref-audit/SKILL.md b/.claude/skills/crossref-audit/SKILL.md index f7a5b68..cbd438c 100644 --- a/.claude/skills/crossref-audit/SKILL.md +++ b/.claude/skills/crossref-audit/SKILL.md @@ -24,7 +24,8 @@ 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. +branch's own commits *before* merge. With squash merges, the master commit +body this audits is the PR description. ## Running it cheaply, every round @@ -105,8 +106,8 @@ tells you to for that specific commit. - 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. + open PR's own commits as part of the audit itself. To fix an open PR's + commit, amend freely before it is pushed; once pushed, add a new commit + or ask before force-pushing (see the `/commit` skill's Restrictions). - 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.md b/CLAUDE.md index e2ebd94..7032b86 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,9 +61,22 @@ this repo pairing. 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. +Commits land on a PR branch, never on master. What reaches master is the +merge result. For a squash merge, both repos use the PR title and body as +the commit message, so the PR description is what master records. + +**Force-push**: on master, never an agent's call (the sole exception is +`crossref-audit`'s Step 4, below). A PR branch is lower risk -- it is +unmerged history belonging only to that PR -- but in this repo pairing +still ask literally ("this needs a force-push to `` -- go ahead?") +and wait for a real reply before force-pushing one; otherwise add a new +commit. This is a deliberate exception to `../ai/PR.md`'s "Force-push / +history rewriting" section, which allows PR-branch force-pushes without +asking. + ## 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). +**This project uses a GitHub PR workflow; Claude never commits directly to master.** The `/commit` skill composes a PR branch's commits for review, including the two-phase cross-reference process (commit pgxntool, capture its hash, commit pgxntool-test referencing it). Squash and rebase merges both create new commits on master ([GitHub docs](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github)), so a branch commit's hash is only a pointer during review; the cross-reference that has to survive the merge goes in the PR description, preferably as a PR URL. Merging happens via the GitHub website, by the user, outside AI control, and Claude 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. From 77650fe0fabac504312fcc35e63876d3d4932516 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 12 Oct 2026 18:50:49 -0500 Subject: [PATCH 2/2] Retire /commit; move PR-prep guidance into /pr With squash merges (both repos allow only squash and rebase), the PR title and body become master's commit message, so branch commit messages and hashes don't reach master and the paired-repo cross-reference belongs in the PR description as a PR URL. - Delete .claude/skills/commit/ (skill, format guide, gather script). - /pr gains HISTORY.asc handling (entry plus the fixed-issues line, kept in sync with the PR's Fixes #N set), the debug-level advisory, a /test PASS-with-zero-skips gate before pushing, and URL-only cross-references placed after the opening paragraph. Step 4 uses --body-file, since gh pr edit has no --add-body flag. - CLAUDE.md drops the two-phase hash injection, names /pr, and holds the PR-branch force-push rule unchanged. - crossref-audit and release point at /pr instead of /commit. --- .claude/commands/pr.md | 44 ++++- .claude/skills/commit/SKILL.md | 168 ------------------ .../commit/guides/commit-message-format.md | 161 ----------------- .../skills/commit/scripts/gather-repo-info.sh | 75 -------- .claude/skills/crossref-audit/SKILL.md | 19 +- .claude/skills/release/SKILL.md | 9 +- CLAUDE.md | 16 +- 7 files changed, 57 insertions(+), 435 deletions(-) delete mode 100644 .claude/skills/commit/SKILL.md delete mode 100644 .claude/skills/commit/guides/commit-message-format.md delete mode 100755 .claude/skills/commit/scripts/gather-repo-info.sh diff --git a/.claude/commands/pr.md b/.claude/commands/pr.md index 3b9d9ec..eb77a72 100644 --- a/.claude/commands/pr.md +++ b/.claude/commands/pr.md @@ -22,11 +22,15 @@ Create pull requests for pgxntool and pgxntool-test changes, following the two-r - pgxntool-test: `--repo Postgres-Extensions/pgxntool-test` - **NEVER** target fork (jnasbyupgrade) -4. **Cross-reference PRs:** - - pgxntool-test PR: Include "Related pgxntool PR: [URL]" at top +4. **Cross-reference PRs by URL, never by commit hash:** + - pgxntool-test PR: "Related pgxntool PR: [URL]" - After creating test PR: Update pgxntool PR to add "Related pgxntool-test PR: [URL]" + - Single-repo PR: "Changes only in [repo]. No related changes in [other repo]." + - Put these lines after the opening paragraph, not first: the opening has to work standalone as the squash commit message (`../ai/PR.md` "PR descriptions"). - **Always use full URLs** (cross-repo #number doesn't work) +5. **Before pushing or creating a PR, run `/test`** and proceed only on `PASS` with zero skips (CLAUDE.md "Testing"). + --- ## PR Title Rules @@ -48,6 +52,23 @@ issue after it, with no error or warning.** This caused real drift in the auto-closed #7 -- the other four issues were genuinely fixed but stayed open on GitHub until a manual post-release audit caught it. +## HISTORY.asc (pgxntool PRs only) + +Read `../pgxntool/HISTORY.asc` and handle two separate things: + +1. **Entry**, only for significant user-visible changes (new features, behavior changes, bug fixes users would notice; not refactoring, test changes, cleanup, or doc fixes). Propose the entry in AsciiDoc (`==` heading) and get confirmation before adding it to the `STABLE` section at the top of the file (create the section if missing): + ``` + STABLE + ------ + == [Entry heading] + [Entry description] + ``` +2. **Fixed-issues line**, always: every pgxntool issue in the PR's `Fixes #N` / `Closes #N` / `Resolves #N` set goes on `Issues fixed in this release: #7, #14, #19`, the last line of the `STABLE` section, even when the change is too minor for an entry. Create the line if missing; keep it deduped and in ascending order. pgxntool issues only; pgxntool-test has no equivalent. Re-check it whenever a push to an open PR changes that set. + +## Debug Levels (pgxntool `.sh` changes only) + +For each changed `../pgxntool/*.sh` file, flag it if it contains loops (`for`, `while`) and every `debug N` call uses a single-digit level, or if it has 5+ `debug` calls all at the same level. Advisory only: present findings and let the user decide; say nothing when nothing is flagged. + ## PR Description Guidelines **Think: "Someone in 2 years reading this in the commit log - what do they need to know?"** @@ -110,18 +131,23 @@ gh pr create \ --base master \ --head jnasbyupgrade:BRANCH \ --title "[Title]" \ - --body "Related pgxntool PR: [URL] + --body "[Opening paragraph] + +Related pgxntool PR: [URL] -[Body]" +[Rest of body]" ``` ### 4. Update pgxntool PR with cross-reference -```bash -cd ../pgxntool -gh pr edit [NUMBER] --add-body " +`gh pr edit` has no append flag, so read the current body, append, and write it back: -Related pgxntool-test PR: [URL]" +```bash +body=$(mktemp) && +gh pr view [NUMBER] --repo Postgres-Extensions/pgxntool --json body --jq .body > "$body" && +printf '\nRelated pgxntool-test PR: %s\n' "[URL]" >> "$body" && +gh pr edit [NUMBER] --repo Postgres-Extensions/pgxntool --body-file "$body" +rm -f "$body" ``` --- @@ -130,7 +156,7 @@ Related pgxntool-test PR: [URL]" **Good PR Description:** ``` -Add pg_tle support and template consolidation +Add pg_tle support, with a test-extra target that runs the full suite across pg_tle versions, and move the template extension into pgxntool-test. Related pgxntool-test PR: https://github.com/Postgres-Extensions/pgxntool-test/pull/2 diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md deleted file mode 100644 index 9de9592..0000000 --- a/.claude/skills/commit/SKILL.md +++ /dev/null @@ -1,168 +0,0 @@ ---- -name: commit -description: | - Compose git commits on PR branches across pgxntool and pgxntool-test for review, - following project standards. Preprocesses repo info, runs tests, drafts cross-referenced - commit messages, and executes two-phase commits with hash injection. - - Use when: "commit", "create commit", "commit changes", "/commit" -allowed-tools: Bash(git status:*), Bash(git log:*), Bash(git add:*), Bash(git diff:*), Bash(git commit:*), Bash(git branch:*), Bash(bash .claude/skills/commit/scripts/*), Read, Edit, Task, Skill(test) ---- - -# Commit Skill - -Compose commits on a PR branch for review, following project standards and safety -protocols. Commits never land on master directly; what reaches master is the merge -result (see CLAUDE.md "Git Commit Guidelines"). - -## Critical Requirements - -| Rule | Details | -|------|---------| -| **Git Safety** | Never update git config. Never skip hooks unless requested. Never force-push without asking -- see Restrictions. | -| **Attribution** | No "Generated with Claude Code" in body. Co-Authored-By trailer is OK. | -| **Multi-Repo** | Commit BOTH repos if both have changes (unless told otherwise). No empty commits. Never create a branch on a repo that has no changes. | -| **Testing** | ALL tests must pass. ANY failure = STOP and ask user. No rationalizing failures. | -| **HISTORY.asc** | Update for significant user-visible pgxntool changes. Propose entry, get confirmation. Separately and always: any `Fixes #N`/`Closes #N`/`Resolves #N` for a pgxntool issue must be added to the running "Issues fixed" line, even when the change is too minor for its own bullet (see step 3b). | - -## Workflow - -### 1. Launch Tests (Background) - -Invoke the `/test` skill (runs `test-all` by default). -The test skill runs in background, parses TAP output, and enforces strict pass/fail. -Skip only if user explicitly says to. - -### 2. Gather Repository Info - -Run the preprocessing script to collect all git data in one call: - -```bash -bash .claude/skills/commit/scripts/gather-repo-info.sh ../pgxntool . -``` - -Review output. Identify which repos have changes. If either repo with changes is on -`master`/`main`, STOP and ask -- never commit there. - -### 2b. Check Debug Level Distribution (pgxntool .sh changes only) - -If `../pgxntool/*.sh` files are in the diff, scan each changed file: - -1. Collect all `debug N` call levels -2. Note whether the file contains loops (`for`, `while`) -3. Flag if: file has loops AND every debug call uses a single-digit level (1-9) -4. Flag if: file has 5+ debug calls AND they all use the exact same level - -This is **advisory only** — present any findings in Step 5 and let the user decide. -If nothing looks off, say nothing (don't add noise to the common case). - -### 3. Check HISTORY.asc (pgxntool changes only) - -Read `../pgxntool/HISTORY.asc`. Determine if changes are significant and user-visible: -- Yes: New features, behavior changes, bug fixes users would notice -- No: Internal refactoring, test changes, cleanup, documentation fixes - -If update needed: -1. Propose entry in AsciiDoc format (use `==` for heading) -2. Ask for confirmation BEFORE proceeding -3. Add to STABLE section at TOP of file (create section if missing): - ``` - STABLE - ------ - == [Entry heading] - [Entry description] - - [existing content...] - ``` - -### 3b. Update the "Issues fixed" line (pgxntool changes only, whenever an issue is fixed) - -Independent of the bullet-entry decision above: if this commit's message -contains a `Fixes #N` / `Closes #N` / `Resolves #N` for a pgxntool issue, that -issue number belongs on the STABLE section's running fixed-issues line -- -even when the change is too minor to warrant its own `==` entry. Don't defer -this to release time; maintain it incrementally, one commit at a time. (The -`/release` skill's own "Compile the complete fixed-issues line" step is a -final audit/safety net for anything that slipped through this per-commit -step -- e.g. a commit made outside this skill -- not the primary mechanism.) - -- Line lives at the end of the STABLE section (last line before the next - version's heading), format: `Issues fixed in this release: #7, #14, #19` -- If it doesn't exist yet, create it. If it exists, add the new number(s), - keep the list deduped and in ascending numeric order. -- pgxntool issues only -- pgxntool-test has no equivalent list. - -### 4. Draft Commit Messages - -Read the format guide for detailed templates and rules: -**`.claude/skills/commit/guides/commit-message-format.md`** - -Key principles: -- Order items by **decreasing importance** (impact x likelihood someone cares) -- pgxntool message includes relevant test changes (1-3 bullets) -- pgxntool-test message uses `[PGXNTOOL_COMMIT_HASH]` placeholder -- Single-repo: note "Changes only in [repo]. No related changes in [other repo]." -- Wrap code references in backticks - -### 5. Present for Approval - -Show proposed commit messages. Wait for user approval. -Mention any files intentionally excluded and why. -If only one repo has changes, show only that message (with note about other repo). - -### 6. Verify Tests Passed (MANDATORY) - -Check test skill output for completion. -- Verify STATUS is `PASS` with zero skips -- **If STATUS is `FAIL` or `PASS_WITH_SKIPS`: STOP. Do NOT commit. Ask user what to do.** -- There is NO such thing as an "acceptable" failing or skipped test -- Do NOT rationalize failures or skips as "pre-existing" or "unrelated" -- Only proceed if STATUS is `PASS` with zero skips - -### 7. Execute Two-Phase Commit - -After tests pass AND receiving user approval: - -1. **Phase 1: Commit pgxntool** (if it has changes) - - Stage files: `git add` specific files (include ALL new files for the feature) - - Verify staging: `git status` (STOP if mismatch) - - Commit with approved message using HEREDOC - - Capture hash: `PGXNTOOL_HASH=$(git log -1 --format=%h)` - - Verify: `git status` - -2. **Phase 2: Commit pgxntool-test** (if it has changes) - - Replace `[PGXNTOOL_COMMIT_HASH]` with captured hash -- a pointer during review only, - since merging creates new commits; the reference that must survive the merge goes in - the PR description (see the guide) - - Stage, verify, commit, verify (same pattern) - -For detailed execution steps including pre-commit hook handling, read: -**`.claude/skills/commit/guides/commit-message-format.md`** -> "Two-Phase Commit Execution" - -Always use HEREDOC format: -```bash -git commit -m "$(cat <<'EOF' -[message] -EOF -)" -``` - -## After Pushing: Monitor CI - -After any `git push`, monitor CI with the `/ci` skill, per CLAUDE.md "CI Monitoring -After Every Push". Report any CI failures to the user immediately — do not assume they are pre-existing or unrelated. - -## Restrictions - -- DO NOT push unless explicitly asked -- Force-pushing master/the default branch is never an agent's call (sole exception: - `crossref-audit` Step 4). -- DO NOT force-push a PR branch without asking first. It is lower risk (unmerged history - belonging only to that PR), but in this repo pairing ask a literal question ("this needs a - force-push to `` -- go ahead?") and wait for a real reply, every time -- including - for your own unreviewed draft PR, and including when a rebase/restructure task seems to - require rewriting history. A task *implying* a rewrite is a signal to ask, never a - substitute for asking. Otherwise add a new commit. This is a deliberate exception to - `../ai/PR.md`'s "Force-push / history rewriting" section. -- DO NOT commit files with secrets (.env, credentials.json) -- Never use `-i` flags (git commit -i, git rebase -i) diff --git a/.claude/skills/commit/guides/commit-message-format.md b/.claude/skills/commit/guides/commit-message-format.md deleted file mode 100644 index a9aaff7..0000000 --- a/.claude/skills/commit/guides/commit-message-format.md +++ /dev/null @@ -1,161 +0,0 @@ -# Commit Message Format Guide - -## Closing GitHub Issues (CRITICAL) - -When a commit fixes one or more issues, give each its own `Fixes #N` / -`Closes #N` / `Resolves #N` line -- never combine them into a single -comma-separated list like `Fixes #7, #14, #19`. **GitHub's auto-close only -recognizes the first issue number in that pattern and silently drops every -issue after it, with no error or warning.** This caused real drift in the -2.2.0 release: one commit's `Fixes #7, #14, #19, #28, #50, #53` line only -auto-closed #7 -- the other four issues were genuinely fixed but stayed -open on GitHub until a manual post-release audit caught it. - -Correct: -``` -Fixes #7. -Fixes #14. -Fixes #19. -``` - -Incorrect: -``` -Fixes #7, #14, #19. -``` - -## Maintaining the HISTORY.asc fixed-issues line (pgxntool changes) - -Whenever a commit's `Fixes #N` / `Closes #N` / `Resolves #N` refers to a -pgxntool issue, add that issue number to the STABLE section's fixed-issues -line in `../pgxntool/HISTORY.asc` -- regardless of whether the change is -significant enough for its own narrative `==` entry. These are two separate -decisions: "does this need a bullet explaining what changed" (maybe not) vs -"was a numbered issue closed" (if yes, it always goes on the list). See the -commit skill's step 3b for the mechanics. - -## Item Ordering (CRITICAL) - -Order all items (changes, bullet points) by **decreasing importance**: -- Importance = impact of the change x likelihood someone reading history will care -- Most impactful/interesting changes first, minor details last -- Think: "What would I want to see first when reading this in git log 2 years from now?" - -## pgxntool Commit Format - -``` -Subject line - -[Main changes in pgxntool, ordered by decreasing importance...] - -Related changes in pgxntool-test: -- [RELEVANT test change 1] -- [RELEVANT test change 2] - -Co-Authored-By: Claude -``` - -**Relevance filter for test changes:** - -| Include | Exclude | -|---------|---------| -| Tests for new features | Test refactoring | -| Template updates | Infrastructure changes | -| User documentation | Internal improvements | - -Keep to 1-3 bullets max. Wrap code references in backticks (e.g., `helpers.bash`, `make test`). -No hash placeholder needed - pgxntool doesn't reference test hash. - -## pgxntool-test Commit Format - -``` -Subject line - -Add tests/updates for pgxntool commit [PGXNTOOL_COMMIT_HASH] (brief description): -- [Key pgxntool change 1] -- [Key pgxntool change 2] - -[pgxntool-test specific changes, ordered by decreasing importance...] - -Co-Authored-By: Claude -``` - -- Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution). The hash is a pointer for review only: squash and rebase merges both create new commits, so no branch commit's hash reaches master. -- **The cross-reference that has to survive the merge goes in the PR description, preferably as the pgxntool PR's URL** (e.g. `https://github.com/Postgres-Extensions/pgxntool/pull/63`). With squash merges the PR body becomes master's commit message, and a PR link stays valid when the referenced commit is rebased or amended; a bare hash goes stale silently. The same URL is also the preferred substitute for the hash in the commit message once the PR exists. -- If a raw hash was used and the referenced pgxntool commit is rebased/amended before your pgxntool-test PR merges, update the reference to the new hash. -- Include brief summary (2-3 bullets) of pgxntool changes near top -- Wrap code references in backticks - -## Single-Repo Case - -If only one repo has changes: -- Skip the repo with no changes -- In the commit message for the repo that has changes, add before Co-Authored-By: - ``` - Changes only in [repo]. No related changes in [other repo]. - ``` - ---- - -## Two-Phase Commit Execution - -Both phases commit to PR branches for review (the skill's step 2 stops on `master`/`main`). - -### Phase 1: Commit pgxntool - -1. `cd ../pgxntool` -2. Stage changes: `git add` specific files - - Check `git status` for untracked files - - ALL untracked files that are part of the feature/change MUST be staged - - New scripts, documentation, helper files should all be included - - Do NOT leave new files uncommitted unless explicitly told to exclude them -3. Verify staged files: `git status` - - Confirm ALL modified AND untracked files are staged - - STOP and ask user if staging doesn't match intent -4. Commit: - ```bash - git commit -m "$(cat <<'EOF' - [approved pgxntool message] - EOF - )" - ``` -5. Capture hash: `PGXNTOOL_HASH=$(git log -1 --format=%h)` -6. Verify: `git status` -7. Handle pre-commit hooks if needed: - - Check if hooks modified files - - Check authorship: `git log -1 --format='%an %ae'` - - Check branch status - - Amend if safe or create new commit - -### Phase 2: Commit pgxntool-test - -1. Return to pgxntool-test directory -2. Replace `[PGXNTOOL_COMMIT_HASH]` in approved message with `$PGXNTOOL_HASH` - - Keep everything else EXACTLY the same -3. Stage changes: `git add` specific files (include ALL new files) -4. Verify staged files: `git status` -5. Commit: - ```bash - git commit -m "$(cat <<'EOF' - [approved message with actual pgxntool hash] - EOF - )" - ``` -6. Capture hash: `TEST_HASH=$(git log -1 --format=%h)` -7. Verify: `git status` -8. Handle pre-commit hooks if needed - -### Important Notes - -- If only one repo has changes, skip the phase for the other repo -- Always use HEREDOC format for commit messages -- Never use `-i` flags (git commit -i, git rebase -i) - -## Repository Context - -The two repositories: -- **pgxntool** (`../pgxntool/`) - The framework being tested. Main Makefile is `base.mk`, scripts in root, docs in `README.asc`. -- **pgxntool-test** (this repo) - Test harness with template files in `template/` directory. - -Commit pgxntool first (no placeholder), capture hash, then commit pgxntool-test (inject hash). -Result: during review, the pgxntool-test commit references the pgxntool commit it -corresponds to; the PR description carries that reference through the merge. diff --git a/.claude/skills/commit/scripts/gather-repo-info.sh b/.claude/skills/commit/scripts/gather-repo-info.sh deleted file mode 100755 index 4147c30..0000000 --- a/.claude/skills/commit/scripts/gather-repo-info.sh +++ /dev/null @@ -1,75 +0,0 @@ -#!/usr/bin/env bash -set -e - -# gather-repo-info.sh - Collect git info for commit preparation -# -# Gathers branch, status, diff stats, and recent log for each repo in one -# call, reducing multiple git commands to a single preprocessed summary. -# -# Usage: gather-repo-info.sh [ ...] -# Output: Structured text summary per repository - -gather_info() { - local repo_path="$1" - local repo_name - repo_name=$(basename "$(cd "$repo_path" && pwd)") - - echo "## $repo_name" - echo "Path: $(cd "$repo_path" && pwd)" - - ( - cd "$repo_path" || exit 1 - - echo "Branch: $(git branch --show-current 2>/dev/null || echo 'detached')" - echo - - local status - status=$(git status -s 2>/dev/null) - if [ -z "$status" ]; then - echo "Status: CLEAN (no changes)" - echo - return - fi - - echo "### Changes" - echo "$status" - echo - - local diff_stat - diff_stat=$(git diff --stat 2>/dev/null) - if [ -n "$diff_stat" ]; then - echo "### Unstaged Diff" - echo "$diff_stat" - echo - fi - - local cached_stat - cached_stat=$(git diff --cached --stat 2>/dev/null) - if [ -n "$cached_stat" ]; then - echo "### Staged Diff" - echo "$cached_stat" - echo - fi - - echo "### Recent Commits" - git log -10 --oneline 2>/dev/null - echo - ) -} - -if [ $# -eq 0 ]; then - echo "Usage: gather-repo-info.sh [ ...]" >&2 - exit 1 -fi - -for repo in "$@"; do - if ! git -C "$repo" rev-parse --git-dir &>/dev/null; then - echo "## $(basename "$repo")" - echo "WARNING: Not a git repository: $repo" - echo - continue - fi - gather_info "$repo" - echo "---" - echo -done diff --git a/.claude/skills/crossref-audit/SKILL.md b/.claude/skills/crossref-audit/SKILL.md index cbd438c..19f0f21 100644 --- a/.claude/skills/crossref-audit/SKILL.md +++ b/.claude/skills/crossref-audit/SKILL.md @@ -16,15 +16,15 @@ allowed-tools: Bash(git:*), Bash(gh:*), Bash(bash .claude/skills/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`. +the other repo, per `/pr` (`.claude/commands/pr.md`). -## Why this exists, and how it differs from `/commit` +## Why this exists, and how it differs from `/pr` 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. With squash merges, the master commit +a different problem from `/pr`, which only covers writing the cross-reference +into a PR description *before* merge. With squash merges, the master commit body this audits is the PR description. ## Running it cheaply, every round @@ -64,11 +64,10 @@ 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. +the other repo. The pgxntool-test side is expected to reference pgxntool by +PR URL (per `/pr`); a raw commit hash is also accepted, only because +historical master commits used one. By convention the pgxntool side only +needs to describe related pgxntool-test changes in prose. ## Step 4: Handle what you find @@ -108,6 +107,6 @@ tells you to for that specific commit. the single tip commit under the conditions above. It never touches an open PR's own commits as part of the audit itself. To fix an open PR's commit, amend freely before it is pushed; once pushed, add a new commit - or ask before force-pushing (see the `/commit` skill's Restrictions). + or ask before force-pushing (see CLAUDE.md "Git Commit Guidelines"). - 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/release/SKILL.md b/.claude/skills/release/SKILL.md index fe288d6..8ea4b79 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -343,13 +343,12 @@ genuinely unclear, ask the user rather than guessing. ### Compile the complete fixed-issues line -The `/commit` skill (step 3b) now maintains this line incrementally, one -commit at a time, whenever a commit's message carries a `Fixes #N` for a -pgxntool issue -- so by release time the STABLE section's fixed-issues line +`/pr` ("HISTORY.asc") maintains this line per PR, from each pgxntool PR's +`Fixes #N` set -- so by release time the STABLE section's fixed-issues line should already list most or all of what belongs here. This step is the **audit/safety net**, not the primary mechanism: it catches anything that -slipped through the per-commit step (a commit made outside `/commit`, a -`Fixes #N` added to a PR body after the commit landed, manual edits, etc.) +slipped through the per-PR step (a PR prepared outside `/pr`, a `Fixes #N` +added to a PR body without re-checking the line, manual edits, etc.) and re-derives the complete, authoritative list from `release..HEAD` history to verify against what's already in the file. diff --git a/CLAUDE.md b/CLAUDE.md index 7032b86..c606201 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,14 +69,16 @@ the commit message, so the PR description is what master records. `crossref-audit`'s Step 4, below). A PR branch is lower risk -- it is unmerged history belonging only to that PR -- but in this repo pairing still ask literally ("this needs a force-push to `` -- go ahead?") -and wait for a real reply before force-pushing one; otherwise add a new -commit. This is a deliberate exception to `../ai/PR.md`'s "Force-push / -history rewriting" section, which allows PR-branch force-pushes without -asking. +and wait for a real reply before force-pushing one, every time: including +your own unreviewed draft PR, and including when a rebase/restructure task +seems to require rewriting history. A task *implying* a rewrite is a signal +to ask, never a substitute for asking. Otherwise add a new commit. This is +a deliberate exception to `../ai/PR.md`'s "Force-push / history rewriting" +section, which allows PR-branch force-pushes without asking. ## PR-Based Workflow: Merges Happen Outside AI Control -**This project uses a GitHub PR workflow; Claude never commits directly to master.** The `/commit` skill composes a PR branch's commits for review, including the two-phase cross-reference process (commit pgxntool, capture its hash, commit pgxntool-test referencing it). Squash and rebase merges both create new commits on master ([GitHub docs](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github)), so a branch commit's hash is only a pointer during review; the cross-reference that has to survive the merge goes in the PR description, preferably as a PR URL. Merging happens via the GitHub website, by the user, outside AI control, and Claude 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). +**This project uses a GitHub PR workflow; Claude never commits directly to master.** Squash and rebase merges both create new commits on master ([GitHub docs](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github)), so a branch commit's hash is not the hash that lands. With squash merges, the PR description becomes master's commit message, so the paired-PR cross-reference goes there, as a PR URL (see `/pr`). Merging happens via the GitHub website, by the user, outside AI control, and Claude 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. @@ -119,10 +121,10 @@ These subagents are already available in your context - you don't need to discov ## Claude Skills and Commands -The `/commit` skill lives in `.claude/skills/commit/` with a preprocessing script and format guide. +The `/pr` command (`.claude/commands/pr.md`) prepares paired PRs: cross-references, HISTORY.asc, and pre-push checks. 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/`. +Other commands (worktree, pgxntool-update) remain in `.claude/commands/`. ## What This Repo Is