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/CLAUDE.md b/.github/workflows/CLAUDE.md new file mode 100644 index 0000000..48c749c --- /dev/null +++ b/.github/workflows/CLAUDE.md @@ -0,0 +1,61 @@ +# .github/workflows — CI Architecture + +## Workflow files + +- **`ci.yml`** — main CI for pgxntool pull requests. Runs `check-test-pr` (verifies + the paired pgxntool-test PR's CI passed), then optionally runs `test` (only for the + commit-with-no-tests path — see below). +- **`protect-label.yml`** — enforces that only maintainers with write access can apply + or remove the `commit-with-no-tests` label. + +## Normal CI flow (paired test PR exists) + +When a pgxntool PR has a corresponding open PR in pgxntool-test with the same branch +name, the `check-test-pr` job polls (up to 20 minutes) for that test PR's CI to +complete and pass. If it passes, pgxntool CI passes — **no tests run here**. Tests run +exactly once, in pgxntool-test's own CI. + +## commit-with-no-tests path + +When a maintainer applies the `commit-with-no-tests` label (and no paired test PR +exists), the `test` job runs tests directly in pgxntool CI against pgxntool-test/master. +This is the rare exception, not the norm. + +## Cross-repo reusable workflow — tradeoffs and constraints + +The `test` job calls a reusable workflow from pgxntool-test: +```yaml +uses: Postgres-Extensions/pgxntool-test/.github/workflows/run-tests.yml@ +``` + +GitHub Actions requires the `uses:` ref to be a **static string** — expressions like +`${{ }}` are not supported in the repo/path portion or the `@ref` suffix in practice. + +### The @branch → @master ref + +While developing on a feature branch where pgxntool-test also has changes, this ref +is set to `@` so CI can find `run-tests.yml` before it lands on master. + +**IMPORTANT**: This ref must be updated to `@master` before pgxntool merges. The +correct merge order is: **pgxntool-test merges first**, then update this ref to +`@master`, then pgxntool merges. + +**For Claude**: Do NOT leave a `@` ref without explicit user approval. The +user merges directly from the PR page — there are no manual steps between merges. +See `.github/workflows/CLAUDE.md` in pgxntool-test for the full picture. + +### Changes to run-tests.yml + +`run-tests.yml` lives in pgxntool-test and is the single source of truth for all test +steps. If it changes, pgxntool's CI uses `@master` — so it won't see the new version +until pgxntool-test merges. This is acceptable because: +- Changes to `run-tests.yml` require a paired test PR (not commit-with-no-tests) +- When a paired test PR exists, pgxntool's `test` job is skipped anyway +- The two scenarios are mutually exclusive in practice + +## Label name + +The label `commit-with-no-tests` is defined as a const (`NO_TEST_LABEL`) in `ci.yml` +and as `LABEL` in `protect-label.yml`. The job-level `if:` condition in +`protect-label.yml` must also use the literal string (YAML can't reference JS consts) +— keep these in sync if the label name ever changes. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..70946b8 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,286 @@ +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. + +permissions: + pull-requests: read + checks: read + +concurrency: + group: ci-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + check-test-pr: + name: Check for paired pgxntool-test PR + runs-on: ubuntu-latest + # This check polls until the paired pgxntool-test CI run completes + # (up to 20 minutes). The job timeout gives a few minutes of headroom. + timeout-minutes: 25 + 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; + // Single source of truth for the label name. Must also match the + // literal string in the protect-label.yml job-level `if:` condition + // (YAML expressions can't reference JS constants). + const NO_TEST_LABEL = 'commit-with-no-tests'; + + // master-to-master PRs have no paired test PR by convention. + // Run tests against pgxntool-test/master directly. + // + // If a fork PR's branch is named 'master', that's almost certainly + // a mistake (contributors should use a feature branch), but we + // don't block it — just warn visibly as an annotation on the run. + // Note: pull_request gives a read-only token for fork PRs, so we + // can't post a PR comment back to the upstream repo from here. + if (branch === 'master') { + const headRepo = context.payload.pull_request.head.repo; + const isBaseRepo = + headRepo?.owner?.login === context.repo.owner && + headRepo?.name === context.repo.repo; + if (!isBaseRepo) { + core.warning( + `PR head branch is named 'master' but comes from a fork ` + + `(${headRepo?.full_name ?? 'unknown'}). Contributors should ` + + `use a feature branch, not master. Proceeding with tests ` + + `against pgxntool-test/master.` + ); + } + core.setOutput('run_tests', 'true'); + core.setOutput('test_ref', 'master'); + return; + } + + // The owner of this PR's head repo — the contributor's fork owner + // for fork PRs, or the base repo owner for maintainer PRs. + // The paired pgxntool-test PR must come from the SAME owner. + // We never cross-match PRs across different contributors' forks. + const prOwner = context.payload.pull_request.head.repo?.owner?.login; + + // Look for open pgxntool-test PRs with the SAME branch name AND + // the same fork owner. Branch names must match exactly. + // + // The GitHub API's 'head' filter requires "owner:branch" format. + // We list all open PRs and filter locally — safe for repos with + // few open PRs, and avoids needing to know the fork repo name. + // paginate() fetches all pages automatically, so this is correct + // even if pgxntool-test ever exceeds 100 open PRs (the per_page cap). + const prs = await github.paginate(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 && + pr.head.repo?.owner?.login === prOwner + ); + if (matching.length > 1) { + core.setFailed( + `Multiple open pgxntool-test PRs from ${prOwner} match branch ` + + `'${branch}'. Cannot determine which one to use.\n\n` + + `Close all but one, then re-run this check.` + ); + return; + } + + const testPR = matching.length === 1 ? matching[0] : null; + + if (testPR) { + // Error if the no-test label is also set — that's contradictory. + // Re-fetch the PR live (not from payload) in case the label was + // added after this workflow was triggered. + const { data: currentPR } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber + }); + if (currentPR.labels.some(l => l.name === NO_TEST_LABEL)) { + core.setFailed( + `PR has the '${NO_TEST_LABEL}' label, but a paired ` + + `pgxntool-test PR #${testPR.number} exists on branch '${branch}'.\n\n` + + `Remove the '${NO_TEST_LABEL}' label — it should only be used ` + + `when there is genuinely no paired test PR.` + ); + return; + } + + // 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)})`); + + // Poll until all check runs for the exact HEAD SHA complete. + // Using 'ref: sha' (not branch name) ensures we only see runs for + // this commit — never stale runs from an older push on the same branch. + // + // We poll rather than fail immediately because both repos are often + // pushed close together. When that happens, pgxntool CI starts while + // pgxntool-test CI may not have queued yet. We wait up to 20 minutes. + const POLL_INTERVAL_MS = 30 * 1000; + const MAX_WAIT_MS = 20 * 60 * 1000; + const waitStart = Date.now(); + let runs; + + while (true) { + // per_page: 100 is intentional here — a single commit will + // not realistically have 100+ CI check runs, so pagination + // is unnecessary. (pulls.list uses paginate() above because + // an active repo could have many open PRs.) + const { data: checks } = await github.rest.checks.listForRef({ + owner: context.repo.owner, + repo: 'pgxntool-test', + ref: sha, + per_page: 100 + }); + runs = checks.check_runs; + + const incomplete = runs.filter(r => r.status !== 'completed'); + if (runs.length > 0 && incomplete.length === 0) break; + + const elapsed = Date.now() - waitStart; + if (elapsed >= MAX_WAIT_MS) { + const mins = Math.round(elapsed / 60000); + if (runs.length === 0) { + core.setFailed( + `pgxntool-test PR #${testPR.number} has no CI runs for ` + + `SHA ${sha.slice(0, 7)} after waiting ${mins} min.\n\n` + + `Push a commit (or manually re-run CI) on the test PR:\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + } else { + const names = incomplete.map(r => r.name).join(', '); + core.setFailed( + `pgxntool-test PR #${testPR.number} CI did not finish within ` + + `${mins} min for SHA ${sha.slice(0, 7)}: ${names}\n\n` + + ` Test PR: ${testPRUrl}\n` + + ` Re-run this check: ${recheckUrl}` + ); + } + return; + } + + if (runs.length === 0) { + core.info(`No CI runs yet for pgxntool-test PR #${testPR.number} (${sha.slice(0, 7)}); waiting 30s...`); + } else { + const names = incomplete.map(r => r.name).join(', '); + core.info(`pgxntool-test CI still running (${names}); waiting 30s...`); + } + await new Promise(resolve => setTimeout(resolve, POLL_INTERVAL_MS)); + } + + // 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; + } + + core.info( + `pgxntool-test PR #${testPR.number} CI passed for ` + + `SHA ${sha.slice(0, 7)} — tests run there, not here.` + ); + core.setOutput('run_tests', 'false'); + core.setOutput('test_ref', sha); + return; + } + + // No paired test PR found. Check for the NO_TEST_LABEL 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 === NO_TEST_LABEL)) { + core.info( + `'${NO_TEST_LABEL}' 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 '${NO_TEST_LABEL}' 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 '${NO_TEST_LABEL}' 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: + needs: check-test-pr + if: needs.check-test-pr.outputs.run-tests == 'true' + # ----------------------------------------------------------------------- + # CROSS-REPO REUSABLE WORKFLOW — READ BEFORE CHANGING THIS REF + # See: .github/workflows/CLAUDE.md for full architecture notes. + # + # The ref (@add-ci / @master) must be a static string — GitHub Actions + # does not support expressions in uses:. During development on a feature + # branch the ref is @ so CI can find run-tests.yml before it + # lands on master. Before this PR merges, two things must happen: + # 1. pgxntool-test/ merges to master first + # 2. This ref is updated from @ to @master + # + # CURRENT REF: @add-ci (temporary — pgxntool-test/add-ci not yet merged) + # ----------------------------------------------------------------------- + uses: Postgres-Extensions/pgxntool-test/.github/workflows/run-tests.yml@add-ci + with: + pgxntool-branch: ${{ github.event.pull_request.head.ref }} + pgxntool-test-ref: master diff --git a/.github/workflows/protect-label.yml b/.github/workflows/protect-label.yml new file mode 100644 index 0000000..0f6d514 --- /dev/null +++ b/.github/workflows/protect-label.yml @@ -0,0 +1,144 @@ +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. + # Note: this literal must match the LABEL const defined in the script below. + 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' + // Single source of truth for the label name within this script. + // Must also match the literal in the job-level `if:` condition above + // (YAML expressions can't reference JS constants). + const LABEL = 'commit-with-no-tests'; + + // 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 '${LABEL}' label`); + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + name: LABEL + }); + 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 '${LABEL}' label`); + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + labels: [LABEL] + }); + 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 '${LABEL}' label is approved` + ); + }