diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000..e2e74fe --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,28 @@ +# Claude Development Notes + +This file contains guidance for Claude Code when working in this repository. +It is excluded from distributions via `.gitattributes export-ignore`. + +## CI Monitoring After Every Push + +**REQUIRED**: After every `git push`, immediately start a background task to +monitor the CI run for that push. If you pushed to both pgxntool and +pgxntool-test, start a background task for each repo — do not monitor them +sequentially. + +Use `gh run watch` or poll with `gh run list` / `gh pr checks` in the +background task. Report failures to the user as soon as they are detected; +do not wait for all jobs to finish before reporting. + +## Multiple Concurrent Sessions + +It is common to have multiple Claude Code sessions open simultaneously across +pgxntool and pgxntool-test. To avoid cross-session interference: + +**If you are asked to do something on an existing PR that you did not open or +are not already working on in this session, immediately ask for confirmation +before proceeding.** For example: "I see PR #32 exists. Were you asking me to +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. diff --git a/.gitattributes b/.gitattributes index a94d824..8dc1599 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,5 +1,6 @@ .gitattributes export-ignore .claude/ export-ignore +.github/ export-ignore *.md export-ignore .DS_Store export-ignore *.asc export-ignore diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..5a94aa7 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,321 @@ +name: CI + +on: + pull_request: + # We use 'pull_request' (not 'pull_request_target') deliberately. + # 'pull_request_target' runs with write access to the base repo, which is + # a security risk for untrusted fork code. Since this workflow only reads + # from other public repos (no secrets needed), 'pull_request' is correct + # and safe even for fork PRs. + +jobs: + check-test-pr: + name: Check for paired pgxntool-test PR + runs-on: ubuntu-latest + # This is a fast check — no polling, just a single API call. + # If it takes longer than 2 minutes something is wrong with the runner. + timeout-minutes: 2 + outputs: + run-tests: ${{ steps.check.outputs.run_tests }} + test-ref: ${{ steps.check.outputs.test_ref }} + + steps: + - name: Find paired pgxntool-test PR or check commit-with-no-tests label + id: check + uses: actions/github-script@v7 + with: + # GITHUB_TOKEN is sufficient for reading public repos. If these repos + # are ever made private, replace with a PAT stored as a secret with + # 'repo' scope on both repos. Note: PAT expiration causes silent + # failures here — the API returns 401 and the job errors out instead + # of failing gracefully with a useful message. + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const branch = context.payload.pull_request.head.ref; + const prNumber = context.payload.pull_request.number; + + // master-to-master PRs have no paired test PR by convention. + // Run tests against pgxntool-test/master directly. + if (branch === 'master') { + core.setOutput('run_tests', 'true'); + core.setOutput('test_ref', 'master'); + return; + } + + // Look for an existing open pgxntool-test PR with the SAME branch + // name. No waiting — the test PR must already exist when this check + // runs. This keeps the check fast (seconds, not minutes). + // + // Branch names must match exactly. If the branch is named 'fix-foo' + // in pgxntool, the paired test PR must also be on 'fix-foo'. + // + // The GitHub API's 'head' filter requires "owner:branch" format and + // requires knowing the fork owner. Since a contributor's pgxntool-test + // fork may differ from their pgxntool fork, we list all open PRs and + // filter locally — a safe approach for repos with few open PRs. + const { data: prs } = await github.rest.pulls.list({ + owner: context.repo.owner, + repo: 'pgxntool-test', + state: 'open', + per_page: 100 + }); + + const matching = prs.filter(pr => pr.head.ref === branch); + let testPR = null; + + if (matching.length > 1) { + // Multiple open PRs with the same branch name from different + // forks. Prefer the one from the same fork owner as this PR. + // If no owner match, fall back to the first result. + const prOwner = context.payload.pull_request.head.repo?.owner?.login; + testPR = matching.find( + pr => pr.head.repo?.owner?.login === prOwner + ) ?? matching[0]; + core.info( + `Multiple pgxntool-test PRs match branch '${branch}'; ` + + `using #${testPR.number} from ${testPR.head.repo?.owner?.login}` + ); + } else if (matching.length === 1) { + testPR = matching[0]; + } + + if (testPR) { + // A paired test PR exists. Verify its CI passed for the exact + // current HEAD SHA and that the run is recent enough to be valid. + const sha = testPR.head.sha; + const testPRUrl = + `https://github.com/${context.repo.owner}/pgxntool-test/pull/${testPR.number}`; + const recheckUrl = + `https://github.com/${context.repo.owner}/${context.repo.repo}/pull/${prNumber}/checks`; + + core.info(`Found pgxntool-test PR #${testPR.number} (${sha.slice(0, 7)})`); + + // Fetch check runs for the exact HEAD SHA of the test PR. + // Using 'ref: sha' (not branch name) ensures we only see runs + // that used this specific commit — never stale runs from an older + // push on the same branch. + const { data: checks } = await github.rest.checks.listForRef({ + owner: context.repo.owner, + repo: 'pgxntool-test', + ref: sha, + per_page: 100 + }); + const runs = checks.check_runs; + + if (runs.length === 0) { + core.setFailed( + `pgxntool-test PR #${testPR.number} has no CI runs for its ` + + `current HEAD (${sha.slice(0, 7)}).\n\n` + + `Push a commit (or re-run CI) on the test PR to trigger its ` + + `CI, then re-run this check:\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + return; + } + + // If CI is still running, tell the user to wait and re-run. + const incomplete = runs.filter(r => r.status !== 'completed'); + if (incomplete.length > 0) { + const names = incomplete.map(r => r.name).join(', '); + core.setFailed( + `pgxntool-test PR #${testPR.number} CI is still running ` + + `for SHA ${sha.slice(0, 7)}: ${names}\n\n` + + `Wait for the test PR CI to finish, then re-run this check:\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + return; + } + + // All checks complete — look for failures. + // 'success', 'skipped', 'neutral' are non-blocking. + const failed = runs.filter( + r => !['success', 'skipped', 'neutral'].includes(r.conclusion) + ); + if (failed.length > 0) { + const names = failed.map(r => `${r.name} (${r.conclusion})`).join(', '); + core.setFailed( + `pgxntool-test PR #${testPR.number} CI failed for ` + + `SHA ${sha.slice(0, 7)}: ${names}\n\n` + + `Fix the test PR CI, then re-run this check:\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + return; + } + + // Verify the passing run is recent (within 7 days). + // Stale CI results may not reflect the current state of + // dependencies or the pgxn-tools container image. + const MAX_AGE_DAYS = 7; + const mostRecent = runs.reduce((a, b) => + new Date(a.completed_at) > new Date(b.completed_at) ? a : b + ); + const completedAt = new Date(mostRecent.completed_at); + const ageDays = (Date.now() - completedAt.getTime()) / 86400000; + + if (ageDays > MAX_AGE_DAYS) { + core.setFailed( + `pgxntool-test PR #${testPR.number} CI passed for ` + + `SHA ${sha.slice(0, 7)} but the run is ${Math.round(ageDays)} ` + + `days old (completed: ${completedAt.toISOString()}).\n\n` + + `CI results older than ${MAX_AGE_DAYS} days are not accepted.\n` + + `Push a commit or re-run CI on the test PR to refresh:\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + return; + } + + const ageHours = Math.round(ageDays * 24); + core.info( + `pgxntool-test PR #${testPR.number} CI passed for ` + + `SHA ${sha.slice(0, 7)} (${ageHours}h ago) — tests run there, not here.` + ); + core.setOutput('run_tests', 'false'); + core.setOutput('test_ref', sha); + return; + } + + // No paired test PR found. Check for the 'commit-with-no-tests' + // label, which a maintainer can apply when a pgxntool change + // genuinely needs no test changes (unusual). + // + // We make a live API call rather than reading from the event + // payload. The payload is a snapshot from when this workflow was + // triggered — a maintainer may have added the label after that. + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber + }); + + if (pr.labels.some(l => l.name === 'commit-with-no-tests')) { + core.info( + "'commit-with-no-tests' label is present; running tests " + + "against pgxntool-test/master. The protect-label workflow " + + "ensures only maintainers can apply this label." + ); + core.setOutput('run_tests', 'true'); + core.setOutput('test_ref', 'master'); + return; + } + + // Neither a paired test PR nor the override label was found. + // Fail with a clear, actionable message. + core.setFailed( + `No paired pgxntool-test PR found for branch '${branch}', ` + + `and no 'commit-with-no-tests' label on this PR.\n\n` + + `pgxntool changes should always be paired with matching test\n` + + `changes in pgxntool-test. This check enforces that pairing.\n\n` + + `To resolve:\n` + + ` 1. Open a PR in pgxntool-test from a branch ALSO named '${branch}'.\n` + + ` Branch names must match exactly for the pairing to work.\n\n` + + ` 2. If this pgxntool change truly needs no test updates (unusual),\n` + + ` ask a maintainer to apply the 'commit-with-no-tests' label.\n` + + ` Only maintainers can apply this label. It is not a normal\n` + + ` shortcut — most pgxntool changes require test updates.\n\n` + + `See: https://github.com/Postgres-Extensions/pgxntool-test#ci-and-contributing` + ); + + test: + name: 🐘 PostgreSQL ${{ matrix.pg }} + needs: check-test-pr + # Only run tests when there is no paired test PR. When a paired PR exists, + # tests run in pgxntool-test's own CI — we don't duplicate them here. + if: needs.check-test-pr.outputs.run-tests == 'true' + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + strategy: + matrix: + pg: [17, 16, 15, 14, 13, 12] + + steps: + - name: Start PostgreSQL ${{ matrix.pg }} + run: pg-start ${{ matrix.pg }} + + - name: Report CI context + run: | + # Print both repos and exact refs at the top of every job so failures + # are easy to correlate to the right code, especially cross-repo issues. + echo "=== BRANCHES: pgxntool=${{ github.head_ref }} pgxntool-test=${{ needs.check-test-pr.outputs.test-ref }} ===" + + - name: Check out pgxntool + uses: actions/checkout@v4 + with: + path: pgxntool + submodules: false # pgxntool has no submodules + # REQUIRED: must be a full clone. 'git subtree add' refuses to work + # with shallow clones and fails with "shallow roots are not allowed + # to be updated" — an error that looks like a remote/ref problem + # rather than a depth issue. + fetch-depth: 0 + + - name: Check out pgxntool-test + uses: actions/checkout@v4 + with: + repository: Postgres-Extensions/pgxntool-test + ref: ${{ needs.check-test-pr.outputs.test-ref }} + path: pgxntool-test + # REQUIRED: pgxntool-test includes BATS as a git submodule at + # test/bats/. Without this, every test invocation fails with + # "bats: command not found" — an error that looks like a PATH issue. + submodules: recursive + + - name: Configure git for CI + run: | + # The container may run as a different UID than the checkout owner. + # Without safe.directory, git refuses to operate with "fatal: dubious + # ownership" — causing test failures that look like code bugs. + git config --global --add safe.directory "$GITHUB_WORKSPACE/pgxntool" + git config --global --add safe.directory "$GITHUB_WORKSPACE/pgxntool-test" + + # Required for git operations that create commits inside tests. + git config --global user.email "ci@github-actions" + git config --global user.name "GitHub Actions" + + # actions/checkout leaves HEAD detached. Tests that call 'git subtree + # add' need a real local branch, not a detached HEAD. -B force-creates + # the branch even if it already exists. + cd pgxntool + git checkout -B "${{ github.head_ref }}" + + - name: Install dependencies + run: | + apt-get update -qq + apt-get install -y -qq rsync jq ruby + gem install asciidoctor --no-document --quiet + # rsync: used throughout test infrastructure; absent rsync causes + # failures deep in BATS output that look like test logic bugs. + # asciidoctor via gem (not apt ruby-asciidoctor): the apt package + # installs the gem but its binary is not on PATH in pgxn-tools + # containers. gem install puts it in /usr/local/bin which IS on PATH. + # jq: required by assert_valid_meta_json(). + + - name: Pre-install pgtap + run: | + # Pre-install pgtap before running tests to prevent a race condition + # in the concurrent-make-test.bats suite. + # + # The test "concurrent make test succeeds for both projects" runs two + # `make test` processes simultaneously. Both check for + # $(datadir)/extension/pgtap.control and, finding it absent, both + # invoke `pgxn install pgtap --sudo` at the same time. Their + # concurrent `gmake install` calls race to write files into the + # shared /usr/share/postgresql//extension/ directory, causing + # "File exists" and "No such file or directory" errors. + # + # This test step only runs in the 'commit-with-no-tests' case (when + # there is no paired pgxntool-test PR). When a paired test PR exists, + # that PR's own CI handles the pre-install. + pgxn install pgtap --sudo + + - name: Run tests + working-directory: pgxntool-test + env: + # Bypass test infrastructure's branch auto-detection. In CI, pgxntool + # is checked out at a specific ref, not necessarily a remote branch + # tip. PGXNBRANCH tells the test infra which branch to treat it as. + PGXNBRANCH: ${{ github.head_ref }} + run: make test diff --git a/.github/workflows/protect-label.yml b/.github/workflows/protect-label.yml new file mode 100644 index 0000000..18c0004 --- /dev/null +++ b/.github/workflows/protect-label.yml @@ -0,0 +1,139 @@ +name: Protect 'commit-with-no-tests' label + +on: + # IMPORTANT: Must use pull_request_target, NOT pull_request. + # + # 'pull_request' from a fork runs with a read-only GITHUB_TOKEN scoped to + # the fork. It cannot add or remove labels on the upstream repo (write + # operation), and cannot call getCollaboratorPermissionLevel (requires write + # permission to the target repo). + # + # 'pull_request_target' runs in the base repo's context with a token that + # has write access — exactly what we need here. + # + # Security: because pull_request_target has write access, never check out + # or execute code from the PR head in this workflow. This workflow only calls + # the GitHub API via actions/github-script and is safe. + pull_request_target: + types: [labeled, unlabeled] + +jobs: + protect: + # Only fire for the label we care about. All other label changes are + # unaffected by this workflow. + if: github.event.label.name == 'commit-with-no-tests' + runs-on: ubuntu-latest + permissions: + pull-requests: write # To add/remove labels + issues: write # GitHub label API goes through the issues endpoint + + steps: + - name: Enforce write-access-only on 'commit-with-no-tests' label + uses: actions/github-script@v7 + with: + script: | + const actor = context.actor; + const prNumber = context.payload.pull_request.number; + const action = context.payload.action; // 'labeled' or 'unlabeled' + + // When this workflow re-adds or removes the label itself, that fires + // this event again with actor = 'github-actions[bot]'. Without this + // guard the job loops forever. We match any '[bot]' suffix to also + // cover other automation (Dependabot, Renovate, etc.). + if (actor.endsWith('[bot]')) { + core.info(`Actor is a bot (${actor}); skipping permission check`); + return; + } + + // Check the actor's effective permission level in this repo. + // + // EDGE CASE — 404 for non-collaborators: This API returns 404 when + // the user is not an explicit collaborator. This is the normal case + // for contributors who forked and opened a PR. If we don't catch + // this error, the job crashes with an unhandled exception and the + // label stays in whatever state the contributor put it in — + // defeating the entire protection. + // + // EDGE CASE — org team members: Users with write access via org + // team membership (not a direct collaborator invite) correctly show + // as 'write' here because the API returns effective permission. + // Exception: if the org has "private member visibility" set and the + // token can't enumerate team membership, they may get a 404 instead. + // If that becomes an issue, add a fallback to + // github.rest.orgs.getMembershipForUser(). + // + // EDGE CASE — other errors: Network blips, API outages, and rate + // limiting all throw here. We fail safe by treating any unexpected + // error as "no write access" and logging for debugging. + let hasWrite = false; + try { + const { data: perm } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: context.repo.owner, + repo: context.repo.repo, + username: actor + }); + hasWrite = ['admin', 'write'].includes(perm.permission); + } catch (e) { + if (e.status === 404) { + // Not a collaborator — no write access. Expected and normal. + hasWrite = false; + } else { + core.warning( + `Unexpected error checking permissions for ${actor} ` + + `(HTTP ${e.status}): ${e.message}. Treating as no write access.` + ); + hasWrite = false; + } + } + + if (action === 'labeled' && !hasWrite) { + core.info(`${actor} lacks write access; removing 'commit-with-no-tests' label`); + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + name: 'commit-with-no-tests' + }); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: + `@${actor} The \`commit-with-no-tests\` label can only be applied by ` + + `maintainers with write access to this repository.\n\n` + + `If you believe no test changes are needed for this PR, please ask a ` + + `maintainer to apply the label after reviewing. Note that most pgxntool ` + + `changes do require paired test updates — this label should be used sparingly.` + }); + + } else if (action === 'unlabeled' && !hasWrite) { + // Non-writer removed the label. Put it back. + // + // EDGE CASE — brief label-absent window: There is a short window + // between removal and this workflow re-adding the label. During + // that window the label genuinely does not exist. This is harmless + // in practice: the ci.yml workflow reads labels via a live API + // call (not from its cached payload), so a re-run after the label + // is restored will pick it up correctly. + core.info(`${actor} lacks write access; re-adding 'commit-with-no-tests' label`); + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + labels: ['commit-with-no-tests'] + }); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: + `@${actor} The \`commit-with-no-tests\` label can only be removed by ` + + `maintainers with write access to this repository.\n\n` + + `Contact a maintainer if you believe this label was applied in error.` + }); + + } else if (hasWrite) { + core.info( + `${actor} has write access; '${action}' on 'commit-with-no-tests' label is approved` + ); + }