From 52945a7ee14c7b91eed614e65acc1db0c763e42e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 15:10:05 -0500 Subject: [PATCH 1/7] Add CI workflows and multi-session PR guard - .github/workflows/ci.yml: wait for corresponding pgxntool-test PR CI to pass before running tests; falls back to pgxntool-test/master after 5 minutes if no test PR exists (requires no-test-pr label override) - .github/workflows/protect-label.yml: enforce write-access-only on the no-test-pr label; handles non-collaborator 404, bot loop prevention, and org team membership edge cases - .claude/CLAUDE.md: warn before touching existing PRs not opened in this session (multiple concurrent Claude sessions are common) Co-Authored-By: Claude Sonnet 4.6 --- .claude/CLAUDE.md | 17 ++ .github/workflows/ci.yml | 249 ++++++++++++++++++++++++++++ .github/workflows/protect-label.yml | 137 +++++++++++++++ 3 files changed, 403 insertions(+) create mode 100644 .claude/CLAUDE.md create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/protect-label.yml diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 100644 index 0000000..dafc32a --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1,17 @@ +# Claude Development Notes + +This file contains guidance for Claude Code when working in this repository. +It is excluded from distributions via `.gitattributes export-ignore`. + +## 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/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..ccff23b --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,249 @@ +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: + resolve-test-ref: + name: Wait for pgxntool-test PR + runs-on: ubuntu-latest + # Hard cap on the job. Our polling logic exits after 5 minutes internally, + # but this prevents the job from hanging indefinitely if something goes wrong + # in the script itself. + timeout-minutes: 8 + outputs: + test-ref: ${{ steps.wait.outputs.test_ref }} + + steps: + - name: Find matching pgxntool-test PR and wait for its CI + id: wait + 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; + + // Master-to-master PRs don't need a corresponding test PR. + if (branch === 'master') { + core.setOutput('test_ref', 'master'); + return; + } + + const deadline = Date.now() + 5 * 60 * 1000; // 5-minute window + let testPR = null; + + core.info(`Searching for pgxntool-test PR with head branch: ${branch}`); + + // Poll for a matching test PR. We wait because contributors typically + // open the pgxntool PR first, then the test PR shortly after. Without + // this wait, CI would immediately fail for every paired PR. + while (!testPR && Date.now() < deadline) { + // The GitHub API's 'head' filter requires "owner:branch" format, + // but we don't know which fork account the contributor used for + // pgxntool-test (it may differ from their pgxntool fork). So we + // list all open PRs and filter locally by branch name. + 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); + + 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. + const prOwner = context.payload.pull_request.head.repo?.owner?.login; + const sameOwner = matching.find( + pr => pr.head.repo?.owner?.login === prOwner + ); + testPR = sameOwner ?? 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) { + const remaining = Math.round((deadline - Date.now()) / 1000); + core.info(`No matching PR yet; retrying in 30s (${remaining}s remaining)`); + await new Promise(r => setTimeout(r, 30000)); + } + } + + if (!testPR) { + // No test PR found. Make a live API call for current labels rather + // than reading from the event payload. The payload is a snapshot + // from when the workflow was triggered — a maintainer may have + // added 'no-test-pr' during our 5-minute wait window. + const { data: pr } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + }); + const hasLabel = pr.labels.some(l => l.name === 'no-test-pr'); + + if (hasLabel) { + core.info( + "'no-test-pr' label is set; proceeding with pgxntool-test master. " + + "The protect-label workflow ensures only maintainers can set this label." + ); + core.setOutput('test_ref', 'master'); + return; + } + + core.setFailed( + `No matching pgxntool-test PR found for branch '${branch}' after 5 minutes, ` + + `and no 'no-test-pr' label on this PR.\n\n` + + `To fix:\n` + + ` - Open a PR in pgxntool-test from a branch also named '${branch}', OR\n` + + ` - Ask a maintainer to add the 'no-test-pr' label if no test changes are needed.` + ); + return; + } + + core.info( + `Found pgxntool-test PR #${testPR.number} at ${testPR.head.sha}. ` + + `Waiting for its CI to complete...` + ); + + const sha = testPR.head.sha; + + while (Date.now() < deadline) { + 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) { + // CI hasn't started yet on the test PR. Normal — GitHub Actions + // queueing can take 10-30 seconds. Keep waiting rather than + // treating empty check_runs as "all passed". + core.info('pgxntool-test CI not yet started; waiting...'); + await new Promise(r => setTimeout(r, 30000)); + continue; + } + + const incomplete = runs.filter(r => r.status !== 'completed'); + if (incomplete.length > 0) { + core.info( + `${incomplete.length} check(s) still running on ` + + `pgxntool-test PR #${testPR.number}; waiting...` + ); + await new Promise(r => setTimeout(r, 30000)); + continue; + } + + // All checks completed. Evaluate results. + // 'success', 'skipped', 'neutral' are non-blocking. + // 'failure', 'cancelled', 'timed_out', 'action_required' block. + // We do NOT treat 'action_required' as passing — it means a human + // must approve something, and auto-unblocking would defeat the + // purpose of that gate. + 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: ${names}\n` + + `Fix the test PR CI before this PR can be merged.` + ); + return; + } + + core.info(`pgxntool-test PR #${testPR.number} CI passed`); + core.setOutput('test_ref', sha); + return; + } + + core.setFailed( + `Timed out waiting for pgxntool-test PR #${testPR.number} CI to complete. ` + + `The test PR may have a stuck or very slow CI run.` + ); + + test: + name: 🐘 PostgreSQL ${{ matrix.pg }} + needs: resolve-test-ref + 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: Check out pgxntool + uses: actions/checkout@v4 + with: + path: pgxntool + submodules: false # pgxntool has no submodules + + - name: Check out pgxntool-test + uses: actions/checkout@v4 + with: + repository: Postgres-Extensions/pgxntool-test + ref: ${{ needs.resolve-test-ref.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 \ + ruby-asciidoctor \ + jq + # rsync: used throughout test infrastructure; absent rsync causes + # failures deep in BATS output that look like test logic bugs. + # ruby-asciidoctor: required by check-readme (called during test-setup). + # jq: required by assert_valid_meta_json(). + + - 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..6834a51 --- /dev/null +++ b/.github/workflows/protect-label.yml @@ -0,0 +1,137 @@ +name: Protect 'no-test-pr' 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 == 'no-test-pr' + 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 'no-test-pr' 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 'no-test-pr' label`); + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + name: 'no-test-pr' + }); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: + `@${actor} The \`no-test-pr\` label can only be applied by maintainers ` + + `with write access to this repository. ` + + `Please ask a maintainer to apply it if no test changes are needed for this PR.` + }); + + } 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), and the window is typically + // under 30 seconds — far less than the 5-minute polling interval. + core.info(`${actor} lacks write access; re-adding 'no-test-pr' label`); + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + labels: ['no-test-pr'] + }); + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: + `@${actor} The \`no-test-pr\` label can only be removed by maintainers ` + + `with write access to this repository. ` + + `Contact a maintainer if you believe this label was applied in error.` + }); + + } else if (hasWrite) { + core.info( + `${actor} has write access; '${action}' on 'no-test-pr' label is approved` + ); + } From 39a3fce49adf10310ee45db57f521605919f71d3 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 15:17:50 -0500 Subject: [PATCH 2/7] fix: use full clone for pgxntool checkout (git subtree requires it) git subtree add refuses to work with shallow clones; fails with "shallow roots are not allowed to be updated" which looks like a remote/ref problem rather than a depth issue. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ccff23b..6589b49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -197,6 +197,11 @@ jobs: 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 From 8390e290c5be9e9d85aa5ea1ca4a7a51ecca3be6 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 15:20:13 -0500 Subject: [PATCH 3/7] docs: require CI monitoring background task after every push Co-Authored-By: Claude Sonnet 4.6 --- .claude/CLAUDE.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index dafc32a..e2e74fe 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -3,6 +3,17 @@ 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 From e8b1abaadc7d778d98f5a6ca793d052fdc41ddc5 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 15:25:46 -0500 Subject: [PATCH 4/7] fix: install asciidoctor via gem; add branch context reporting - gem install asciidoctor instead of apt ruby-asciidoctor (same fix as pgxntool-test) - Print '=== BRANCHES: ===' line at the start of each test job so both the pgxntool and pgxntool-test refs are visible in CI logs Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6589b49..d3e94f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -192,6 +192,12 @@ jobs: - 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.resolve-test-ref.outputs.test-ref }} ===" + - name: Check out pgxntool uses: actions/checkout@v4 with: @@ -235,13 +241,13 @@ jobs: - name: Install dependencies run: | apt-get update -qq - apt-get install -y -qq \ - rsync \ - ruby-asciidoctor \ - jq + 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. - # ruby-asciidoctor: required by check-readme (called during test-setup). + # 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: Run tests From c5f753aa484334e77056ecac81fa97613ec5e1a8 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 15:41:04 -0500 Subject: [PATCH 5/7] fix: exclude .github/ from distribution archives GitHub Actions workflows are development infrastructure and should not be included in PGXN distribution packages. Without this exclusion, `make dist` (which uses `git archive`) includes .github/workflows/, causing the dist test to fail: not ok 22 distribution contains exact expected files # ERROR: Distribution contents differ from expected manifest # > pgxntool/.github/ # > pgxntool/.github/workflows/ # > pgxntool/.github/workflows/ci.yml # > pgxntool/.github/workflows/protect-label.yml The .gitattributes `export-ignore` attribute controls what `git archive` excludes. Adding `.github/ export-ignore` ensures CI workflow files are stripped from distributions, matching the existing treatment of .claude/, .gitattributes, and documentation files. Co-Authored-By: Claude Sonnet 4.6 --- .gitattributes | 1 + 1 file changed, 1 insertion(+) 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 From 121f04d34f484cd888ab27d2a90ad6d4c23d153a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 16:25:12 -0500 Subject: [PATCH 6/7] refactor: redesign CI to check for paired test PR immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key changes: 1. Replace 5-minute polling 'resolve-test-ref' job with a fast 'check-test-pr' job (seconds, not minutes). The test PR must exist when you push — no waiting. 2. When a paired pgxntool-test PR is found, pgxntool CI passes immediately without running tests. Tests run in the test PR's own CI — no duplication. 3. When no paired test PR exists and no override label is present, CI fails immediately with an actionable error message pointing to the README docs. 4. Rename 'no-test-pr' label to 'commit-with-no-tests'. This name better conveys that it's a merge-time decision (not just a CI skip), and that it's unusual and maintainer-only. 5. Add 'Pre-install pgtap' step to the test job (only runs in the commit-with-no-tests case). Prevents a concurrent-install race condition in concurrent-make-test.bats. 6. Update protect-label.yml to guard the new label name. The failure message now links to README#ci-and-contributing which explains branch naming requirements and how to request the label. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 251 +++++++++++++--------------- .github/workflows/protect-label.yml | 32 ++-- 2 files changed, 130 insertions(+), 153 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3e94f9..368cebd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,19 +9,19 @@ on: # and safe even for fork PRs. jobs: - resolve-test-ref: - name: Wait for pgxntool-test PR + check-test-pr: + name: Check for paired pgxntool-test PR runs-on: ubuntu-latest - # Hard cap on the job. Our polling logic exits after 5 minutes internally, - # but this prevents the job from hanging indefinitely if something goes wrong - # in the script itself. - timeout-minutes: 8 + # 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: - test-ref: ${{ steps.wait.outputs.test_ref }} + run-tests: ${{ steps.check.outputs.run_tests }} + test-ref: ${{ steps.check.outputs.test_ref }} steps: - - name: Find matching pgxntool-test PR and wait for its CI - id: wait + - 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 @@ -32,156 +32,113 @@ jobs: 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 don't need a corresponding test PR. + // 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; } - const deadline = Date.now() + 5 * 60 * 1000; // 5-minute window + // 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; - core.info(`Searching for pgxntool-test PR with head branch: ${branch}`); - - // Poll for a matching test PR. We wait because contributors typically - // open the pgxntool PR first, then the test PR shortly after. Without - // this wait, CI would immediately fail for every paired PR. - while (!testPR && Date.now() < deadline) { - // The GitHub API's 'head' filter requires "owner:branch" format, - // but we don't know which fork account the contributor used for - // pgxntool-test (it may differ from their pgxntool fork). So we - // list all open PRs and filter locally by branch name. - 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); - - 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. - const prOwner = context.payload.pull_request.head.repo?.owner?.login; - const sameOwner = matching.find( - pr => pr.head.repo?.owner?.login === prOwner - ); - testPR = sameOwner ?? 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) { - const remaining = Math.round((deadline - Date.now()) / 1000); - core.info(`No matching PR yet; retrying in 30s (${remaining}s remaining)`); - await new Promise(r => setTimeout(r, 30000)); - } + 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) { - // No test PR found. Make a live API call for current labels rather - // than reading from the event payload. The payload is a snapshot - // from when the workflow was triggered — a maintainer may have - // added 'no-test-pr' during our 5-minute wait window. - const { data: pr } = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number - }); - const hasLabel = pr.labels.some(l => l.name === 'no-test-pr'); - - if (hasLabel) { - core.info( - "'no-test-pr' label is set; proceeding with pgxntool-test master. " + - "The protect-label workflow ensures only maintainers can set this label." - ); - core.setOutput('test_ref', 'master'); - return; - } - - core.setFailed( - `No matching pgxntool-test PR found for branch '${branch}' after 5 minutes, ` + - `and no 'no-test-pr' label on this PR.\n\n` + - `To fix:\n` + - ` - Open a PR in pgxntool-test from a branch also named '${branch}', OR\n` + - ` - Ask a maintainer to add the 'no-test-pr' label if no test changes are needed.` + if (testPR) { + // A paired test PR exists. Its own CI handles test coverage for + // this pgxntool change — no need to run tests here too. + // This keeps pgxntool CI fast and avoids duplicating test runs. + core.info( + `Found paired pgxntool-test PR #${testPR.number} ` + + `(${testPR.head.sha.slice(0, 7)}) — tests run there, not here.` ); + core.setOutput('run_tests', 'false'); + core.setOutput('test_ref', testPR.head.sha); return; } - core.info( - `Found pgxntool-test PR #${testPR.number} at ${testPR.head.sha}. ` + - `Waiting for its CI to complete...` - ); - - const sha = testPR.head.sha; - - while (Date.now() < deadline) { - 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) { - // CI hasn't started yet on the test PR. Normal — GitHub Actions - // queueing can take 10-30 seconds. Keep waiting rather than - // treating empty check_runs as "all passed". - core.info('pgxntool-test CI not yet started; waiting...'); - await new Promise(r => setTimeout(r, 30000)); - continue; - } - - const incomplete = runs.filter(r => r.status !== 'completed'); - if (incomplete.length > 0) { - core.info( - `${incomplete.length} check(s) still running on ` + - `pgxntool-test PR #${testPR.number}; waiting...` - ); - await new Promise(r => setTimeout(r, 30000)); - continue; - } - - // All checks completed. Evaluate results. - // 'success', 'skipped', 'neutral' are non-blocking. - // 'failure', 'cancelled', 'timed_out', 'action_required' block. - // We do NOT treat 'action_required' as passing — it means a human - // must approve something, and auto-unblocking would defeat the - // purpose of that gate. - const failed = runs.filter( - r => !['success', 'skipped', 'neutral'].includes(r.conclusion) + // 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." ); - - if (failed.length > 0) { - const names = failed.map(r => `${r.name} (${r.conclusion})`).join(', '); - core.setFailed( - `pgxntool-test PR #${testPR.number} CI failed: ${names}\n` + - `Fix the test PR CI before this PR can be merged.` - ); - return; - } - - core.info(`pgxntool-test PR #${testPR.number} CI passed`); - core.setOutput('test_ref', sha); + 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( - `Timed out waiting for pgxntool-test PR #${testPR.number} CI to complete. ` + - `The test PR may have a stuck or very slow CI run.` + `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: resolve-test-ref + 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: @@ -196,7 +153,7 @@ jobs: 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.resolve-test-ref.outputs.test-ref }} ===" + echo "=== BRANCHES: pgxntool=${{ github.head_ref }} pgxntool-test=${{ needs.check-test-pr.outputs.test-ref }} ===" - name: Check out pgxntool uses: actions/checkout@v4 @@ -213,7 +170,7 @@ jobs: uses: actions/checkout@v4 with: repository: Postgres-Extensions/pgxntool-test - ref: ${{ needs.resolve-test-ref.outputs.test-ref }} + 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 @@ -250,6 +207,24 @@ jobs: # 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: diff --git a/.github/workflows/protect-label.yml b/.github/workflows/protect-label.yml index 6834a51..18c0004 100644 --- a/.github/workflows/protect-label.yml +++ b/.github/workflows/protect-label.yml @@ -1,4 +1,4 @@ -name: Protect 'no-test-pr' label +name: Protect 'commit-with-no-tests' label on: # IMPORTANT: Must use pull_request_target, NOT pull_request. @@ -21,14 +21,14 @@ jobs: protect: # Only fire for the label we care about. All other label changes are # unaffected by this workflow. - if: github.event.label.name == 'no-test-pr' + 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 'no-test-pr' label + - name: Enforce write-access-only on 'commit-with-no-tests' label uses: actions/github-script@v7 with: script: | @@ -87,21 +87,23 @@ jobs: } if (action === 'labeled' && !hasWrite) { - core.info(`${actor} lacks write access; removing 'no-test-pr' label`); + 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: 'no-test-pr' + name: 'commit-with-no-tests' }); await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, body: - `@${actor} The \`no-test-pr\` label can only be applied by maintainers ` + - `with write access to this repository. ` + - `Please ask a maintainer to apply it if no test changes are needed for this PR.` + `@${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) { @@ -111,27 +113,27 @@ jobs: // 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), and the window is typically - // under 30 seconds — far less than the 5-minute polling interval. - core.info(`${actor} lacks write access; re-adding 'no-test-pr' label`); + // 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: ['no-test-pr'] + labels: ['commit-with-no-tests'] }); await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: prNumber, body: - `@${actor} The \`no-test-pr\` label can only be removed by maintainers ` + - `with write access to this repository. ` + + `@${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 'no-test-pr' label is approved` + `${actor} has write access; '${action}' on 'commit-with-no-tests' label is approved` ); } From 93a9eec97f8c271f6a66a5bb5598663fed77d125 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 15 May 2026 16:28:57 -0500 Subject: [PATCH 7/7] feat: verify test PR CI used current SHA and is recent When a paired pgxntool-test PR is found, verify: 1. CI has run for the exact current HEAD SHA (no stale runs from old commits) 2. All check runs completed successfully 3. The most recent passing run is within 7 days Each failure case provides URLs for both the test PR and the pgxntool re-run check, making it clear what to fix and how to re-trigger. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 98 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 368cebd..5a94aa7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,15 +80,101 @@ jobs: } if (testPR) { - // A paired test PR exists. Its own CI handles test coverage for - // this pgxntool change — no need to run tests here too. - // This keeps pgxntool CI fast and avoids duplicating test runs. + // 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( - `Found paired pgxntool-test PR #${testPR.number} ` + - `(${testPR.head.sha.slice(0, 7)}) — tests run there, not here.` + `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', testPR.head.sha); + core.setOutput('test_ref', sha); return; }