From 48a0d34e5174dadb2a627bc294a2e6149b5c548c Mon Sep 17 00:00:00 2001 From: Salah Al-Nawah Date: Tue, 27 Oct 2026 08:25:02 +0000 Subject: [PATCH 1/4] ci(themes): port checkout-react quality gates and report publishing (SAL-3553) Adds the PR gates workflows and scripts, the shared setup action, and a tiered CI with Testing Ready and test reports published to the shared reports worker. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com> --- .github/actions/setup/action.yml | 16 ++ .github/workflows/ci.yml | 178 ++++++++++++---- .github/workflows/pr-gates-merge-group.yml | 86 ++++++++ .github/workflows/pr-gates-rerun.yml | 61 ++++++ .github/workflows/pr-gates.yml | 48 +++++ docs/ci.md | 22 ++ eslint.config.js | 7 + scripts/ci/prGates.test.ts | 232 +++++++++++++++++++++ scripts/ci/prGates.ts | 216 +++++++++++++++++++ scripts/ci/prGatesApi.test.ts | 170 +++++++++++++++ scripts/ci/prGatesApi.ts | 204 ++++++++++++++++++ scripts/ci/prGatesCli.ts | 108 ++++++++++ tsconfig.json | 4 +- vitest.config.ts | 2 +- 14 files changed, 1314 insertions(+), 40 deletions(-) create mode 100644 .github/actions/setup/action.yml create mode 100644 .github/workflows/pr-gates-merge-group.yml create mode 100644 .github/workflows/pr-gates-rerun.yml create mode 100644 .github/workflows/pr-gates.yml create mode 100644 docs/ci.md create mode 100644 scripts/ci/prGates.test.ts create mode 100644 scripts/ci/prGates.ts create mode 100644 scripts/ci/prGatesApi.test.ts create mode 100644 scripts/ci/prGatesApi.ts create mode 100644 scripts/ci/prGatesCli.ts diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml new file mode 100644 index 0000000..ce21c6e --- /dev/null +++ b/.github/actions/setup/action.yml @@ -0,0 +1,16 @@ +name: Set up workspace +description: pnpm through corepack (the packageManager field), Node from .nvmrc, a pnpm store cache and a frozen install. +runs: + using: composite + steps: + # Corepack reads the exact pnpm version from package.json's packageManager field. + - name: Enable corepack + shell: bash + run: corepack enable + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + cache: pnpm + - name: Install dependencies + shell: bash + run: pnpm install --frozen-lockfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9511d2f..4a8c7f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,70 +1,80 @@ -name: CI +name: Theme Raed React — Quality Gates +# A draft runs nothing. A pull request that is ready for review runs every gate on each push; the merge queue (when +# enabled) runs them on the exact result it would merge. Reports are published to the shared tests-reports worker. on: pull_request: branches: - master - feature/quality-gates-twig-parity-audit + types: [opened, synchronize, reopened, ready_for_review, converted_to_draft] + merge_group: + types: [checks_requested] push: branches: - feature/quality-gates-twig-parity-audit +# A draft run skips every job, so it gets its own group: sharing one with the real run lets a late-starting draft run +# cancel the ready_for_review run and leave every gate failed at 0s. +concurrency: + group: quality-gates-${{ github.event.pull_request.number || github.ref }}${{ github.event.pull_request.draft && '-draft' || '' }} + cancel-in-progress: true + permissions: contents: read -concurrency: - group: ci-${{ github.ref }} - cancel-in-progress: true +defaults: + run: + shell: bash jobs: - gates: - name: Quality gates - runs-on: ubuntu-latest - timeout-minutes: 20 + # Required check for the "Tested & Coverage" ruleset: one verdict over every gate below. + testing-result: + name: Testing Ready + needs: [static, unit, build] + # Not on a draft: every gate is skipped there, and skipped counts as passed. + if: always() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + steps: + - uses: SallaApp/.CI/testing-ready@master + with: + needs: ${{ toJSON(needs) }} + + static: + name: Lint, Types & Catalogue + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 15 + env: + NODE_OPTIONS: --max-old-space-size=6144 steps: - name: Checkout uses: actions/checkout@v4 - - # pnpm must be installed before setup-node so its store cache can resolve. - # No `version` input: it is read from the packageManager field. - - name: Set up pnpm - uses: pnpm/action-setup@v4 - - - name: Set up Node - uses: actions/setup-node@v4 - with: - node-version-file: .nvmrc - cache: pnpm - - - name: Print versions - run: | - node -v - pnpm -v - - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Build - run: pnpm build - + - name: Set up pnpm and Node + uses: ./.github/actions/setup - name: Typecheck run: pnpm typecheck - - name: Lint run: pnpm lint - - name: Format check run: pnpm format:check - - - name: Test with coverage - run: pnpm test:coverage - - name: Check manifest run: pnpm check:manifest - - name: Dead code (knip) run: pnpm knip + unit: + name: Unit Tests & Coverage + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Set up pnpm and Node + uses: ./.github/actions/setup + - name: Run unit tests and enforce the coverage floor + run: pnpm test:coverage - name: Upload coverage report if: always() uses: actions/upload-artifact@v4 @@ -73,3 +83,95 @@ jobs: path: coverage/ if-no-files-found: warn retention-days: 7 + - name: Publish the coverage report + if: always() + continue-on-error: true + uses: SallaApp/checkout-tests-reports/publish@master + with: + name: coverage + title: Unit tests and coverage + folders: | + coverage:coverage + entry: coverage/index.html + vitest-json: coverage/test-results.json + status: ${{ job.status }} + access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} + secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} + endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} + + build: + name: Build & Bundle Size + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 15 + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Set up pnpm and Node + uses: ./.github/actions/setup + - name: Build + run: pnpm build + - name: Report the bundle size + run: | + mkdir -p bundle-report + { + echo "Build output size (dist/)" + du -sh dist + echo + find dist -type f \( -name '*.js' -o -name '*.css' \) -printf '%s %p\n' | sort -rn | head -40 | awk '{printf "%8.1f KB %s\n", $1/1024, $2}' + } | tee bundle-report/bundle-size.txt + - name: Publish the bundle size + if: always() + continue-on-error: true + uses: SallaApp/checkout-tests-reports/publish@master + with: + name: size + title: Bundle size + folders: | + bundle-report + entry: bundle-size.txt + status: ${{ job.status }} + access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} + secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} + endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} + + # Not in Testing Ready's needs: a reports problem never blocks a merge. Builds the run index and posts the PR comment. + reports: + name: Test Reports + needs: [static, unit, build] + if: always() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 15 + continue-on-error: true + # One run at a time per pull request, so two runs never post the result comment at the same moment. + concurrency: + group: test-reports-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: false + permissions: + contents: read + pull-requests: write + steps: + # `unit` publishes the coverage row, so its own `needs` row would be a second, empty one: fold it in. + - name: Fold the unit verdict into the coverage row + id: verdicts + if: always() + continue-on-error: true + env: + NEEDS: ${{ toJSON(needs) }} + run: | + { + echo 'needs<> "$GITHUB_OUTPUT" + - name: Build the run index and comment on the pull request + if: always() + continue-on-error: true + uses: SallaApp/checkout-tests-reports/finalize@master + with: + base-url: ${{ secrets.REPORTS_BASE_URL }} + needs: ${{ steps.verdicts.outputs.needs || toJSON(needs) }} + github-token: ${{ github.token }} + access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} + secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} + endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} diff --git a/.github/workflows/pr-gates-merge-group.yml b/.github/workflows/pr-gates-merge-group.yml new file mode 100644 index 0000000..798ef6c --- /dev/null +++ b/.github/workflows/pr-gates-merge-group.yml @@ -0,0 +1,86 @@ +name: Theme Raed React — Merge group verification + +# The jobs below carry the names of the checks the repository ruleset requires and exist for the merge queue only: the group commit never +# gets Greptile or the organisation validators, so each re-verifies the stage 1 result on the head of every pull request of the group +# (docs/merge-queue.md). They are in their own workflow, triggered by `merge_group` alone, because a job skipped by an `if` still +# reports a check run, and a skipped check counts as passed: on a pull request those names must belong to the real apps only. +on: + merge_group: + types: [checks_requested] + +concurrency: + group: pr-gates-merge-group-${{ github.ref }} + cancel-in-progress: false + +permissions: + contents: read + pull-requests: read + checks: read + +defaults: + run: + shell: bash + +jobs: + greptile-review: + name: Greptile Review + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 5 + env: + CHECK_NAME: Greptile Review + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + - name: Verify the stage 1 result + env: + EVENT: ${{ github.event_name }} + REPOSITORY: ${{ github.repository }} + GITHUB_TOKEN: ${{ github.token }} + MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} + MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} + MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} + run: node scripts/ci/prGatesCli.ts + + validate-pr-title: + name: Validate PR title + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 5 + env: + CHECK_NAME: Validate PR title + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + - name: Verify the stage 1 result + env: + EVENT: ${{ github.event_name }} + REPOSITORY: ${{ github.repository }} + GITHUB_TOKEN: ${{ github.token }} + MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} + MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} + MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} + run: node scripts/ci/prGatesCli.ts + + validate-branch-name: + name: Validate branch name + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 5 + env: + CHECK_NAME: Validate branch name + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + - name: Verify the stage 1 result + env: + EVENT: ${{ github.event_name }} + REPOSITORY: ${{ github.repository }} + GITHUB_TOKEN: ${{ github.token }} + MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} + MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} + MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} + run: node scripts/ci/prGatesCli.ts diff --git a/.github/workflows/pr-gates-rerun.yml b/.github/workflows/pr-gates-rerun.yml new file mode 100644 index 0000000..e0f104b --- /dev/null +++ b/.github/workflows/pr-gates-rerun.yml @@ -0,0 +1,61 @@ +name: Theme Raed React — PR gates re-run + +# `PR gates` reads live state, but a pull request run only starts on a pull request event. Greptile, the validators and review threads +# change without one, so this workflow re-runs the `PR gates` run of that head commit when they do. GitHub limits what can be +# used here: `check_run` workflows run from the default branch and attach to its commit, so they cannot report the gate themselves +# (they re-run the pull request's run instead); a check run made with GITHUB_TOKEN never starts a workflow, but Greptile and the +# validators are apps, so theirs do. Resolving a review thread starts no workflow (there is no such event), so the next pull request +# event or a manual re-run picks it up; GitHub's own conversation resolution rule and the merge group jobs hold that gate meanwhile. +# Nothing here is a required check. +on: + check_run: + types: [completed] + pull_request_review: + types: [submitted, dismissed] + +concurrency: + group: pr-gates-rerun-${{ github.event.check_run.head_sha || github.event.pull_request.head.sha }} + cancel-in-progress: false + +permissions: + contents: read + actions: write + +jobs: + rerun: + name: Re-run PR gates + # Only the three checks `PR gates` reads, and only from the apps it trusts (the script re-checks the names). + if: >- + github.event_name == 'pull_request_review' || + contains(fromJSON('["Greptile Review", "Validate PR title", "Validate branch name"]'), github.event.check_run.name) + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 10 + steps: + - name: Re-run PR gates of the head commit + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + HEAD_SHA: ${{ github.event.check_run.head_sha || github.event.pull_request.head.sha }} + run: | + if [[ ! "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then + echo "::error::No head commit to re-run for" + exit 1 + fi + for attempt in 1 2 3 4 5 6 7 8 9 10; do + run=$(gh api "repos/$REPOSITORY/actions/workflows/pr-gates.yml/runs?head_sha=$HEAD_SHA&event=pull_request&per_page=1" \ + --jq '.workflow_runs[0] | "\(.id) \(.status) \(.conclusion)"') + read -r run_id status conclusion <<<"$run" + if [[ -z "${run_id:-}" || "$run_id" == "null" ]]; then + echo "No PR gates run on $HEAD_SHA yet: the next pull request event starts one." + exit 0 + fi + if [[ "$status" == "completed" ]]; then + gh api -X POST "repos/$REPOSITORY/actions/runs/$run_id/rerun" + echo "Re-ran PR gates run $run_id (it was $conclusion): the state it checks changed." + exit 0 + fi + echo "PR gates run $run_id is $status; waiting (attempt $attempt)." + sleep 30 + done + echo "::error::PR gates did not finish in time to be re-run" + exit 1 diff --git a/.github/workflows/pr-gates.yml b/.github/workflows/pr-gates.yml new file mode 100644 index 0000000..4d14816 --- /dev/null +++ b/.github/workflows/pr-gates.yml @@ -0,0 +1,48 @@ +name: Theme Raed React — PR gates + +# Two stages (docs/merge-queue.md). Stage 1 is the pull request: the real Greptile app, the organisation's validators and the fast +# tier report on it. Stage 2 is the merge queue (pr-gates-merge-group.yml). +# `PR gates` runs on both and is the anti-spoof gate: the required checks must come from their real apps, not from a workflow that +# borrows a name. It is re-run by pr-gates-rerun.yml when Greptile or a validator finishes. +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review, edited] + merge_group: + types: [checks_requested] + +concurrency: + group: pr-gates-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +permissions: + contents: read + pull-requests: read + checks: read + +defaults: + run: + shell: bash + +jobs: + # Passes only when `Greptile Review` on the head of the pull request (every pull request of a merge group) was reported by the Greptile + # app (id 867647), the title and branch validators by their real app, no review thread is open and the pull request is not a draft. + pr-gates: + name: PR gates + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + timeout-minutes: 5 + steps: + - name: Checkout + uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + - name: Evaluate the gates + env: + EVENT: ${{ github.event_name }} + REPOSITORY: ${{ github.repository }} + GITHUB_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} + MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} + MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} + run: node scripts/ci/prGatesCli.ts diff --git a/docs/ci.md b/docs/ci.md new file mode 100644 index 0000000..ab697fc --- /dev/null +++ b/docs/ci.md @@ -0,0 +1,22 @@ +# CI quality gates + +Ported from `SallaApp/checkout-react/.github`, limited to what this repository has. + +| Workflow | What it does | +| -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ci.yml` | `static` (typecheck, lint, format, manifest, knip), `unit` (coverage floor), `build` (build + bundle size), `Testing Ready` verdict, `Test Reports` index | +| `pr-gates.yml` | `PR gates`: the Greptile app, the title and branch validators passed on the head commit, no open review thread, not a draft | +| `pr-gates-merge-group.yml` | Merge-queue-only jobs that re-verify the stage 1 result (`Greptile Review`, `Validate PR title`, `Validate branch name`) | +| `pr-gates-rerun.yml` | Re-runs `PR gates` when Greptile, a validator or a review changes | + +Reports (coverage, bundle size) are pushed to the shared tests-reports worker through +`SallaApp/checkout-tests-reports/publish` and `/finalize`. They need the repository secrets +`R2_REPORTS_ACCESS_KEY_ID`, `R2_REPORTS_SECRET_ACCESS_KEY`, `R2_REPORTS_ENDPOINT` and `REPORTS_BASE_URL`; +publishing is `continue-on-error`, so missing secrets never block a merge. `RUNS_ON` (repository variable) selects the +runner and falls back to `ubuntu-latest`. + +Not ported because this repository has no Storybook, Playwright E2E, API contract tests, Lighthouse setup or Cloudflare +checkout Worker: `storybook-baselines`, `figma-visual*`, `e2e-nightly`, `api-drift`, `deploy`. + +Admin setup for the required checks (`Testing Ready`, `Greptile Review`, `Validate PR title`, `Validate branch name`, +`PR gates`) and the merge queue follows checkout-react's `docs/merge-queue.md`. diff --git a/eslint.config.js b/eslint.config.js index b0f135f..b6de273 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -80,6 +80,13 @@ export default tseslint.config( languageOptions: { globals: { ...globals.node } }, }, + // CI scripts run on Node. + { + files: ['scripts/**/*.ts'], + languageOptions: { globals: { ...globals.node } }, + rules: { 'no-restricted-properties': 'off', 'no-restricted-globals': 'off' }, + }, + // Must stay last: turns off formatting rules that conflict with Prettier. prettier ); diff --git a/scripts/ci/prGates.test.ts b/scripts/ci/prGates.test.ts new file mode 100644 index 0000000..81bbeaa --- /dev/null +++ b/scripts/ci/prGates.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, it } from 'vitest'; +import { + ALL_GATES, + evaluatePullRequest, + GREPTILE_APP, + mergeGroupPullRequests, + SALLA_BOT_APP, + scopeOfCheck, + summaryMarkdown, + type CheckRunSummary, + type PullRequestSnapshot, +} from './prGates.ts'; + +function run(name: string, overrides: Partial = {}): CheckRunSummary { + const app = name === 'Greptile Review' ? GREPTILE_APP : SALLA_BOT_APP; + return { + id: 1, + name, + status: 'completed', + conclusion: 'success', + appId: app.appId, + appSlug: app.appSlug, + ...overrides, + }; +} + +function snapshot(overrides: Partial = {}): PullRequestSnapshot { + return { + number: 7, + headSha: 'a'.repeat(40), + isDraft: false, + checkRuns: [run('Greptile Review'), run('Validate PR title'), run('Validate branch name')], + unresolvedThreads: 0, + ...overrides, + }; +} + +describe('evaluatePullRequest', () => { + it('passes when every gate holds', () => { + expect(evaluatePullRequest(snapshot())).toEqual({ + number: 7, + passed: true, + reasons: [], + waiting: false, + }); + }); + + it('fails a draft', () => { + expect(evaluatePullRequest(snapshot({ isDraft: true })).reasons).toEqual([ + 'the pull request is a draft', + ]); + }); + + it('fails open review threads, singular and plural', () => { + expect(evaluatePullRequest(snapshot({ unresolvedThreads: 1 })).reasons).toEqual([ + '1 review thread is not resolved', + ]); + expect(evaluatePullRequest(snapshot({ unresolvedThreads: 3 })).reasons).toEqual([ + '3 review threads are not resolved', + ]); + }); + + it('waits for a check that has not been reported or has not finished', () => { + const missing = evaluatePullRequest( + snapshot({ checkRuns: [run('Validate PR title'), run('Validate branch name')] }) + ); + expect(missing).toMatchObject({ passed: false, waiting: true }); + expect(missing.reasons[0]).toContain('has not been reported by greptile-apps'); + const running = evaluatePullRequest( + snapshot({ + checkRuns: [ + run('Greptile Review', { status: 'in_progress', conclusion: null }), + run('Validate PR title'), + run('Validate branch name'), + ], + }) + ); + expect(running).toMatchObject({ + passed: false, + waiting: true, + reasons: ['"Greptile Review" is still in_progress'], + }); + }); + + it('fails a check that concluded anything but success, without waiting', () => { + const verdict = evaluatePullRequest( + snapshot({ + checkRuns: [ + run('Greptile Review', { conclusion: 'failure' }), + run('Validate PR title'), + run('Validate branch name', { conclusion: null }), + ], + }) + ); + expect(verdict.waiting).toBe(false); + expect(verdict.reasons).toEqual([ + '"Greptile Review" concluded failure', + '"Validate branch name" concluded without a result', + ]); + }); + + it('does not accept a check of the right name from the wrong app (a workflow borrowing the name)', () => { + const spoofed = run('Greptile Review', { id: 99, appId: 15_368, appSlug: 'github-actions' }); + const verdict = evaluatePullRequest( + snapshot({ checkRuns: [spoofed, run('Validate PR title'), run('Validate branch name')] }) + ); + expect(verdict).toMatchObject({ passed: false, waiting: true }); + }); + + it('does not let a spoofed success hide the real failure, nor the real one be replaced by an older run', () => { + const spoofed = run('Greptile Review', { id: 99, appId: 15_368, appSlug: 'github-actions' }); + const real = run('Greptile Review', { id: 5, conclusion: 'failure' }); + const verdict = evaluatePullRequest( + snapshot({ + checkRuns: [real, spoofed, run('Validate PR title'), run('Validate branch name')], + }) + ); + expect(verdict.reasons).toEqual(['"Greptile Review" concluded failure']); + const rerun = evaluatePullRequest( + snapshot({ + checkRuns: [ + run('Greptile Review', { id: 3, conclusion: 'failure' }), + run('Greptile Review', { id: 8 }), + run('Validate PR title'), + run('Validate branch name'), + ], + }) + ); + expect(rerun.passed).toBe(true); + }); + + it('requires the app slug as well as the id', () => { + const verdict = evaluatePullRequest( + snapshot({ + checkRuns: [ + run('Greptile Review', { appSlug: 'someone-else' }), + run('Validate PR title'), + run('Validate branch name'), + ], + }) + ); + expect(verdict.passed).toBe(false); + }); + + it('collects every reason', () => { + const verdict = evaluatePullRequest( + snapshot({ isDraft: true, checkRuns: [], unresolvedThreads: 2 }) + ); + expect(verdict.reasons).toHaveLength(5); + }); +}); + +describe('scopeOfCheck', () => { + it('scopes a merge group job to its check; only Greptile also needs resolved threads, none needs a non-draft', () => { + expect(scopeOfCheck('Greptile Review')).toMatchObject({ + rejectDraft: false, + requireResolvedThreads: true, + }); + expect(scopeOfCheck('Validate PR title')).toMatchObject({ requireResolvedThreads: false }); + expect(scopeOfCheck('Testing Ready')).toBeUndefined(); + }); + + it('verifies only its own check', () => { + const scope = scopeOfCheck('Validate PR title'); + const onlyTitle = snapshot({ + isDraft: true, + unresolvedThreads: 4, + checkRuns: [run('Validate PR title')], + }); + expect(scope && evaluatePullRequest(onlyTitle, scope).passed).toBe(true); + expect(evaluatePullRequest(onlyTitle, ALL_GATES).passed).toBe(false); + }); +}); + +describe('mergeGroupPullRequests', () => { + const REF = 'refs/heads/gh-readonly-queue/master/pr-12-0123456789abcdef0123456789abcdef01234567'; + + it('reads the last pull request from the branch name, with or without refs/heads', () => { + expect(mergeGroupPullRequests(REF, ['feat: x (#12)'])).toEqual({ + numbers: [12], + unidentifiedCommits: [], + }); + expect( + mergeGroupPullRequests(REF.replace('refs/heads/', ''), ['feat: x (#12)']).numbers + ).toEqual([12]); + }); + + it('adds every pull request named by the squash commits of the group, sorted, and ignores earlier suffixes in a title', () => { + expect( + mergeGroupPullRequests(REF, ['fix: a (#10)', 'fix: b (#3) (#11)', 'feat: x (#12) ']) + ).toEqual({ + numbers: [10, 11, 12], + unidentifiedCommits: [], + }); + }); + + it('reports a commit that names no pull request, so the check fails closed', () => { + expect( + mergeGroupPullRequests(REF, ['Merge pull request #5 from x/y', 'feat: x (#12)']) + ).toEqual({ + numbers: [12], + unidentifiedCommits: ['Merge pull request #5 from x/y'], + }); + }); + + it('reports a branch that is not a queue branch when nothing else identifies a pull request', () => { + expect(mergeGroupPullRequests('refs/heads/main', [])).toEqual({ + numbers: [], + unidentifiedCommits: ['refs/heads/main'], + }); + expect( + mergeGroupPullRequests('gh-readonly-queue/master/pr-0-abcdef1', ['x (#0)']) + .unidentifiedCommits + ).toEqual(['x (#0)']); + }); +}); + +describe('summaryMarkdown', () => { + it('lists the verdicts, the reasons and the unidentified commits, never a title or a branch name', () => { + const failed = evaluatePullRequest(snapshot({ isDraft: true })); + const text = summaryMarkdown([evaluatePullRequest(snapshot({ number: 1 })), failed], ['x']); + expect(text).toBe( + '## PR gates\n\n- #1: passed\n- #7: failed\n - the pull request is a draft\n- 1 commit(s) of the merge group name no pull request (squash merge expected)\n' + ); + }); + + it('takes a heading and a note for the merge group jobs', () => { + expect(summaryMarkdown([], [], 'Greptile Review', 'note')).toBe( + '## Greptile Review\n\nnote\n\n' + ); + }); +}); diff --git a/scripts/ci/prGates.ts b/scripts/ci/prGates.ts new file mode 100644 index 0000000..b8d3e43 --- /dev/null +++ b/scripts/ci/prGates.ts @@ -0,0 +1,216 @@ +/** + * The decision behind the `PR gates` check (.github/workflows/pr-gates.yml, docs/merge-queue.md). The org rulesets "Pull Requests" + * and "Tested & Coverage" require `Greptile Review`, `Validate PR title` and `Validate branch name`, which never run on a merge queue + * commit, so the queue would wait for them forever. This check reports on the pull request and on the merge group instead, and + * reproduces what those rules enforce: the same checks green on the head of every pull request, no open review thread, not a draft. + * Everything here is a pure function of data the CLI fetched; none of it comes from a command line. + */ + +/** A check that must have succeeded, and the GitHub App that must have reported it (a workflow cannot impersonate it by name). */ +interface RequiredCheck { + readonly name: string; + readonly appId: number; + readonly appSlug: string; +} + +const GREPTILE_CHECK_NAME = 'Greptile Review'; + +export const GREPTILE_APP = { appId: 867_647, appSlug: 'greptile-apps' } as const; + +/** `Validate PR title` and `Validate branch name` come from the organisation's internal bot, not from a workflow of this repository. */ +export const SALLA_BOT_APP = { appId: 1_131_467, appSlug: 'sallainternalbot' } as const; + +const REQUIRED_CHECKS: readonly RequiredCheck[] = [ + { name: GREPTILE_CHECK_NAME, ...GREPTILE_APP }, + { name: 'Validate PR title', ...SALLA_BOT_APP }, + { name: 'Validate branch name', ...SALLA_BOT_APP }, +]; + +/** The fields of a check run the decision needs. */ +export interface CheckRunSummary { + readonly id: number; + readonly name: string; + readonly status: string; + readonly conclusion: string | null; + readonly appId: number | undefined; + readonly appSlug: string | undefined; +} + +export interface PullRequestSnapshot { + readonly number: number; + readonly headSha: string; + readonly isDraft: boolean; + readonly checkRuns: readonly CheckRunSummary[]; + /** Review threads that are not resolved (outdated ones included: the ruleset's conversation resolution counts them too). */ + readonly unresolvedThreads: number; +} + +export interface PullRequestVerdict { + readonly number: number; + readonly passed: boolean; + /** Why it did not pass, one line each; empty when it passed. */ + readonly reasons: readonly string[]; + /** True when a failing reason is only that a check has not finished or not started: re-evaluating later can change it. */ + readonly waiting: boolean; +} + +interface CheckVerdict { + readonly reason: string | undefined; + readonly waiting: boolean; +} + +/** The newest run of a check reported by the required app; runs of the same name from any other app do not count. */ +function latestRunOf( + required: RequiredCheck, + runs: readonly CheckRunSummary[] +): CheckRunSummary | undefined { + let latest: CheckRunSummary | undefined; + for (const run of runs) { + const fromRequiredApp = + run.name === required.name && + run.appId === required.appId && + run.appSlug === required.appSlug; + if (fromRequiredApp && (latest === undefined || run.id > latest.id)) { + latest = run; + } + } + return latest; +} + +function checkVerdict(required: RequiredCheck, runs: readonly CheckRunSummary[]): CheckVerdict { + const run = latestRunOf(required, runs); + if (run === undefined) { + return { + reason: `"${required.name}" has not been reported by ${required.appSlug} on the head commit yet`, + waiting: true, + }; + } + if (run.status !== 'completed') { + return { reason: `"${required.name}" is still ${run.status}`, waiting: true }; + } + return { + reason: + run.conclusion === 'success' + ? undefined + : `"${required.name}" concluded ${run.conclusion ?? 'without a result'}`, + waiting: false, + }; +} + +/** What one job verifies: the default is every gate of `PR gates`; the merge group jobs named after a check verify one each. */ +export interface GateScope { + readonly required: readonly RequiredCheck[]; + readonly rejectDraft: boolean; + readonly requireResolvedThreads: boolean; +} + +export const ALL_GATES: GateScope = { + required: REQUIRED_CHECKS, + rejectDraft: true, + requireResolvedThreads: true, +}; + +/** Passes only when every required check succeeded on the head commit, no review thread is open and the pull request is not a draft. */ +export function evaluatePullRequest( + snapshot: PullRequestSnapshot, + scope: GateScope = ALL_GATES +): PullRequestVerdict { + const reasons: string[] = []; + let waiting = false; + if (scope.rejectDraft && snapshot.isDraft) { + reasons.push('the pull request is a draft'); + } + for (const check of scope.required) { + const verdict = checkVerdict(check, snapshot.checkRuns); + if (verdict.reason === undefined) { + continue; + } + reasons.push(verdict.reason); + waiting ||= verdict.waiting; + } + if (scope.requireResolvedThreads && snapshot.unresolvedThreads > 0) { + reasons.push( + `${String(snapshot.unresolvedThreads)} review thread${snapshot.unresolvedThreads === 1 ? ' is' : 's are'} not resolved` + ); + } + return { number: snapshot.number, passed: reasons.length === 0, reasons, waiting }; +} + +/** The scope of a job from its `CHECK_NAME` ("Greptile Review" also needs resolved threads); undefined for a name that is not a required check. */ +export function scopeOfCheck(name: string): GateScope | undefined { + const required = REQUIRED_CHECKS.filter((check) => check.name === name); + return required.length === 0 + ? undefined + : { required, rejectDraft: false, requireResolvedThreads: name === GREPTILE_CHECK_NAME }; +} + +/** `gh-readonly-queue//pr--` (with or without `refs/heads/`): the pull request the queue added last. */ +const QUEUE_REF = /^(?:refs\/heads\/)?gh-readonly-queue\/.+\/pr-(\d+)-[\da-f]{7,40}$/; + +/** The `(#N)` GitHub appends to the title of a squash commit; only the last one counts, a title can contain others. */ +const SQUASH_SUFFIX = /\(#(\d+)\)$/; + +export interface MergeGroupPullRequests { + /** Every pull request in the group, ascending. */ + readonly numbers: readonly number[]; + /** Commits of the group whose subject names no pull request; the group cannot be fully identified and must fail. */ + readonly unidentifiedCommits: readonly string[]; +} + +function parseNumber(digits: string | undefined): number | undefined { + const value = Number(digits); + return Number.isSafeInteger(value) && value > 0 ? value : undefined; +} + +/** + * The pull requests of a merge group. The branch name only names the last one added, so the commits of the group + * (`base_sha...head_sha`) are read as well: with the squash merge method of the queue each carries `(#N)` at the end of its subject. + * A commit that does not (another merge method) is reported instead of ignored, so the check fails closed. + */ +export function mergeGroupPullRequests( + headRef: string, + commitSubjects: readonly string[] +): MergeGroupPullRequests { + const numbers = new Set(); + const unidentifiedCommits: string[] = []; + const fromRef = parseNumber(QUEUE_REF.exec(headRef)?.[1]); + if (fromRef !== undefined) { + numbers.add(fromRef); + } + for (const subject of commitSubjects) { + const fromSubject = parseNumber(SQUASH_SUFFIX.exec(subject.trim())?.[1]); + if (fromSubject === undefined) { + unidentifiedCommits.push(subject); + } else { + numbers.add(fromSubject); + } + } + if (numbers.size === 0 && unidentifiedCommits.length === 0) { + unidentifiedCommits.push(headRef); + } + return { numbers: [...numbers].toSorted((a, b) => a - b), unidentifiedCommits }; +} + +/** The job summary (Markdown): one line per pull request. Titles and branch names never appear in it, only numbers and fixed texts. */ +export function summaryMarkdown( + verdicts: readonly PullRequestVerdict[], + unidentifiedCommits: readonly string[] = [], + heading = 'PR gates', + note = '' +): string { + const lines = [`## ${heading}`, '', ...(note === '' ? [] : [note, ''])]; + for (const verdict of verdicts) { + lines.push( + verdict.passed + ? `- #${String(verdict.number)}: passed` + : `- #${String(verdict.number)}: failed`, + ...verdict.reasons.map((reason) => ` - ${reason}`) + ); + } + if (unidentifiedCommits.length > 0) { + lines.push( + `- ${String(unidentifiedCommits.length)} commit(s) of the merge group name no pull request (squash merge expected)` + ); + } + return `${lines.join('\n')}\n`; +} diff --git a/scripts/ci/prGatesApi.test.ts b/scripts/ci/prGatesApi.test.ts new file mode 100644 index 0000000..92832b8 --- /dev/null +++ b/scripts/ci/prGatesApi.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from 'vitest'; +import { + commitSubjects, + pullRequestSnapshot, + type Fetch, + type GitHubClient, +} from './prGatesApi.ts'; + +const SHA = 'b'.repeat(40); + +function clientAnswering( + answer: (url: string, body: string | undefined) => { status?: number; body: unknown } +): { + client: GitHubClient; + urls: string[]; +} { + const urls: string[] = []; + const fetchStub: Fetch = (url, init) => { + urls.push(`${init.method} ${url}`); + const { status = 200, body } = answer(url, init.body); + return Promise.resolve({ ok: status < 400, status, json: () => Promise.resolve(body) }); + }; + return { + client: { fetch: fetchStub, token: 't', apiUrl: 'https://api.test', repository: 'o/r' }, + urls, + }; +} + +describe('commitSubjects', () => { + it('returns the first line of each commit of the range', async () => { + const { client, urls } = clientAnswering(() => ({ + body: { commits: [{ commit: { message: 'a (#1)\n\nbody' } }, { commit: {} }] }, + })); + expect(await commitSubjects(client, SHA, SHA)).toEqual(['a (#1)', '']); + expect(urls[0]).toContain(`/repos/o/r/compare/${SHA}...${SHA}?per_page=100&page=1`); + }); + + it('refuses a base or head that is not a full SHA', async () => { + const { client } = clientAnswering(() => ({ body: {} })); + await expect(commitSubjects(client, '--help', SHA)).rejects.toThrow('full commit SHAs'); + }); + + it('follows pages and gives up on a huge list', async () => { + const page = { + commits: Array.from({ length: 100 }, () => ({ commit: { message: 'x (#1)' } })), + }; + const { client, urls } = clientAnswering((url) => ({ + body: url.endsWith('page=2') ? { commits: [] } : page, + })); + expect(await commitSubjects(client, SHA, SHA)).toHaveLength(100); + expect(urls).toHaveLength(2); + const endless = clientAnswering(() => ({ body: page })); + await expect(commitSubjects(endless.client, SHA, SHA)).rejects.toThrow('More than'); + }); +}); + +const DRAFT_PULL = { head: { sha: SHA }, draft: true }; + +type Answer = (url: string) => { status?: number; body: unknown }; + +function pullAnswers(threadPages: readonly unknown[], pull: unknown = DRAFT_PULL): Answer { + let thread = 0; + return (url: string): { status?: number; body: unknown } => { + if (url.endsWith('/graphql')) { + return { body: threadPages[thread++] }; + } + if (url.includes('/check-runs')) { + return { + body: { + check_runs: [ + { + id: 4, + name: 'Greptile Review', + status: 'completed', + conclusion: 'success', + app: { id: 867_647, slug: 'greptile-apps' }, + }, + 'junk', + ], + }, + }; + } + return { body: pull }; + }; +} + +function threads(nodes: readonly unknown[], next?: string): unknown { + return { + data: { + repository: { + pullRequest: { + reviewThreads: { pageInfo: { hasNextPage: next !== undefined, endCursor: next }, nodes }, + }, + }, + }, + }; +} + +async function failureOf(answer: Answer): Promise { + try { + await pullRequestSnapshot(clientAnswering(answer).client, 9); + } catch (error: unknown) { + return error instanceof Error ? error.message : ''; + } + return ''; +} + +describe('pullRequestSnapshot', () => { + it('reads the live head, draft state, check runs of that head and unresolved threads across pages', async () => { + const { client, urls } = clientAnswering( + pullAnswers([ + threads([{ isResolved: false }, { isResolved: true }], 'c1'), + threads([{ isResolved: false }]), + ]) + ); + const result = await pullRequestSnapshot(client, 9); + expect(result).toMatchObject({ number: 9, headSha: SHA, isDraft: true, unresolvedThreads: 2 }); + expect(result.checkRuns[0]).toEqual({ + id: 4, + name: 'Greptile Review', + status: 'completed', + conclusion: 'success', + appId: 867_647, + appSlug: 'greptile-apps', + }); + expect(result.checkRuns[1]).toEqual({ + id: 0, + name: '', + status: '', + conclusion: null, + appId: undefined, + appSlug: undefined, + }); + expect( + urls.some((url) => url.includes(`/commits/${SHA}/check-runs?filter=latest&per_page=100`)) + ).toBe(true); + }); + + it('skips the threads when the job does not need them, and treats a missing draft flag as ready', async () => { + const { client, urls } = clientAnswering(pullAnswers([], { head: { sha: SHA } })); + expect(await pullRequestSnapshot(client, 9, false)).toMatchObject({ + isDraft: false, + unresolvedThreads: 0, + }); + expect(urls.some((url) => url.endsWith('/graphql'))).toBe(false); + }); + + it('fails on a head that is not a commit, an API error, GraphQL errors and endless thread pages', async () => { + expect(await failureOf(() => ({ body: {} }))).toContain('no readable head'); + expect(await failureOf(() => ({ status: 403, body: {} }))).toContain( + '403 to GET /repos/o/r/pulls/9' + ); + expect(await failureOf(pullAnswers([{ errors: [{}] }]))).toContain('GraphQL returned errors'); + const endless = Array.from({ length: 25 }, () => threads([], 'next')); + expect(await failureOf(pullAnswers(endless))).toContain('Too many review threads'); + }); + + it('stops when a page says there is more but gives no cursor', async () => { + const noCursor = { + data: { + repository: { + pullRequest: { reviewThreads: { pageInfo: { hasNextPage: true }, nodes: [] } }, + }, + }, + }; + const { client } = clientAnswering(pullAnswers([noCursor, threads([])])); + const snapshotWithoutCursor = await pullRequestSnapshot(client, 9); + expect(snapshotWithoutCursor.unresolvedThreads).toBe(0); + }); +}); diff --git a/scripts/ci/prGatesApi.ts b/scripts/ci/prGatesApi.ts new file mode 100644 index 0000000..761e9ae --- /dev/null +++ b/scripts/ci/prGatesApi.ts @@ -0,0 +1,204 @@ +/** + * The GitHub reads of the `PR gates` check, over `fetch` so a test can answer them. Read-only: the token needs `contents`, `pull-requests` + * and `checks` read access. Every value that reaches a URL is a validated number or SHA, never free text. + */ +import type { CheckRunSummary, PullRequestSnapshot } from './prGates.ts'; + +export type Fetch = ( + url: string, + init: { + readonly method: string; + readonly headers: Record; + readonly body?: string | undefined; + } +) => Promise<{ + readonly ok: boolean; + readonly status: number; + json: () => Promise; +}>; + +export interface GitHubClient { + readonly fetch: Fetch; + readonly token: string; + /** `https://api.github.com` unless GitHub Enterprise says otherwise. */ + readonly apiUrl: string; + /** `owner/name`. */ + readonly repository: string; +} + +const PAGE_LIMIT = 20; +const COMMIT_SHA = /^[\da-f]{40}$/; + +async function request( + client: GitHubClient, + method: string, + url: string, + body?: unknown +): Promise { + const response = await client.fetch(url, { + method, + headers: { + authorization: `Bearer ${client.token}`, + accept: 'application/vnd.github+json', + 'x-github-api-version': '2022-11-28', + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + if (!response.ok) { + throw new Error( + `GitHub answered ${String(response.status)} to ${method} ${new URL(url).pathname}` + ); + } + return response.json(); +} + +function recordOf(value: unknown): Record { + return typeof value === 'object' && value !== null ? (value as Record) : {}; +} + +/** The value at a path of keys, or undefined when any step is not an object. */ +function reach(value: unknown, keys: readonly string[]): unknown { + let current = value; + for (const key of keys) { + current = recordOf(current)[key]; + } + return current; +} + +function firstLine(text = ''): string { + return text.split('\n', 1)[0] ?? ''; +} + +function listOf(value: unknown): readonly unknown[] { + return Array.isArray(value) ? value : []; +} + +function textOf(value: unknown): string | undefined { + return typeof value === 'string' ? value : undefined; +} + +function numberOf(value: unknown): number | undefined { + return typeof value === 'number' ? value : undefined; +} + +/** Every item of a paged REST list, read from `field` of each page (or the page itself when it is the array). */ +async function pagedItems( + client: GitHubClient, + path: string, + field: string | undefined +): Promise { + const items: unknown[] = []; + for (let page = 1; page <= PAGE_LIMIT; page += 1) { + const separator = path.includes('?') ? '&' : '?'; + const body = await request( + client, + 'GET', + `${client.apiUrl}${path}${separator}per_page=100&page=${String(page)}` + ); + const pageItems = listOf(field === undefined ? body : recordOf(body)[field]); + items.push(...pageItems); + if (pageItems.length < 100) { + return items; + } + } + throw new Error(`More than ${String(PAGE_LIMIT * 100)} items at ${path}`); +} + +function checkRunOf(raw: unknown): CheckRunSummary { + const run = recordOf(raw); + const app = recordOf(run['app']); + return { + id: numberOf(run['id']) ?? 0, + name: textOf(run['name']) ?? '', + status: textOf(run['status']) ?? '', + conclusion: textOf(run['conclusion']) ?? null, + appId: numberOf(app['id']), + appSlug: textOf(app['slug']), + }; +} + +/** The subjects (first lines) of the commits `base...head` adds: for a merge group, one per pull request with the squash method. */ +export async function commitSubjects( + client: GitHubClient, + base: string, + head: string +): Promise { + if (!COMMIT_SHA.test(base) || !COMMIT_SHA.test(head)) { + throw new Error('The merge group base and head must be full commit SHAs'); + } + const commits = await pagedItems( + client, + `/repos/${client.repository}/compare/${base}...${head}`, + 'commits' + ); + return commits.map((commit) => firstLine(textOf(reach(commit, ['commit', 'message'])))); +} + +const THREADS_QUERY = `query($owner: String!, $name: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $after) { + pageInfo { hasNextPage endCursor } + nodes { isResolved } + } + } + } +}`; + +async function unresolvedThreadCount(client: GitHubClient, number: number): Promise { + const [owner = '', name = ''] = client.repository.split('/', 2); + let unresolved = 0; + let after: string | null = null; + for (let page = 0; page < PAGE_LIMIT; page += 1) { + const body = recordOf( + await request(client, 'POST', `${client.apiUrl}/graphql`, { + query: THREADS_QUERY, + variables: { owner, name, number, after }, + }) + ); + if (listOf(body['errors']).length > 0) { + throw new Error('GitHub GraphQL returned errors reading the review threads'); + } + const threads = recordOf(reach(body, ['data', 'repository', 'pullRequest', 'reviewThreads'])); + unresolved += listOf(threads['nodes']).filter( + (node) => recordOf(node)['isResolved'] !== true + ).length; + const pageInfo = recordOf(threads['pageInfo']); + if (pageInfo['hasNextPage'] !== true) { + return unresolved; + } + after = textOf(pageInfo['endCursor']) ?? null; + } + throw new Error('Too many review threads to read'); +} + +/** The live state of a pull request: its current head, whether it is a draft, its check runs on that head and its open threads. */ +export async function pullRequestSnapshot( + client: GitHubClient, + number: number, + withThreads = true +): Promise { + const pull = recordOf( + await request( + client, + 'GET', + `${client.apiUrl}/repos/${client.repository}/pulls/${String(number)}` + ) + ); + const headSha = textOf(recordOf(pull['head'])['sha']) ?? ''; + if (!COMMIT_SHA.test(headSha)) { + throw new Error(`Pull request #${String(number)} has no readable head commit`); + } + const runs = await pagedItems( + client, + `/repos/${client.repository}/commits/${headSha}/check-runs?filter=latest`, + 'check_runs' + ); + return { + number, + headSha, + isDraft: pull['draft'] === true, + checkRuns: runs.map((run) => checkRunOf(run)), + unresolvedThreads: withThreads ? await unresolvedThreadCount(client, number) : 0, + }; +} diff --git a/scripts/ci/prGatesCli.ts b/scripts/ci/prGatesCli.ts new file mode 100644 index 0000000..cf2c7b3 --- /dev/null +++ b/scripts/ci/prGatesCli.ts @@ -0,0 +1,108 @@ +/** + * `node scripts/ci/prGatesCli.ts`: the `PR gates` check. Reads `EVENT`, `REPOSITORY`, `GITHUB_TOKEN` and, for a pull request, + * `PR_NUMBER`, or, for a merge group, `MERGE_GROUP_HEAD_REF`, `MERGE_GROUP_BASE_SHA` and `MERGE_GROUP_HEAD_SHA` from the environment. + * `CHECK_NAME` (merge group jobs named after a required check) narrows it to that one check: the job then verifies the stage 1 result + * of the real check on each pull request instead of reporting a check of its own. Prints one verdict per pull request, appends it to the job summary and exits 1 when any pull request does not pass. + */ +import { appendFileSync } from 'node:fs'; +import { commitSubjects, pullRequestSnapshot, type GitHubClient } from './prGatesApi.ts'; +import { + ALL_GATES, + evaluatePullRequest, + mergeGroupPullRequests, + scopeOfCheck, + summaryMarkdown, + type GateScope, + type PullRequestVerdict, +} from './prGates.ts'; + +const environment = process.env; +const token = environment['GITHUB_TOKEN'] ?? ''; +const repository = environment['REPOSITORY'] ?? ''; + +const client: GitHubClient = { + fetch: (url, { body, ...init }) => fetch(url, body === undefined ? init : { ...init, body }), + token, + apiUrl: environment['GITHUB_API_URL'] ?? 'https://api.github.com', + repository, +}; + +async function pullRequestNumbers(): Promise<{ + numbers: readonly number[]; + unidentifiedCommits: readonly string[]; +}> { + if (environment['EVENT'] === 'merge_group') { + const subjects = await commitSubjects( + client, + environment['MERGE_GROUP_BASE_SHA'] ?? '', + environment['MERGE_GROUP_HEAD_SHA'] ?? '' + ); + return mergeGroupPullRequests(environment['MERGE_GROUP_HEAD_REF'] ?? '', subjects); + } + const number = Number(environment['PR_NUMBER']); + if (!Number.isSafeInteger(number) || number <= 0) { + throw new Error('PR_NUMBER is not a pull request number'); + } + return { numbers: [number], unidentifiedCommits: [] }; +} + +/** The scope of this job: every gate, or the one check named by `CHECK_NAME` (the merge group jobs). */ +function jobScope(checkName: string): GateScope { + const scope = checkName === '' ? ALL_GATES : scopeOfCheck(checkName); + if (scope === undefined) { + throw new Error('CHECK_NAME is not a required check'); + } + return scope; +} + +const STAGE_ONE_NOTE = + 'Verifies the stage 1 result: the real check on the head of each pull request in the merge group (docs/merge-queue.md).'; + +function report(markdown: string): void { + process.stdout.write(markdown); + const summary = environment['GITHUB_STEP_SUMMARY']; + if (summary !== undefined) { + appendFileSync(summary, markdown); + } +} + +async function main(): Promise { + if (token === '' || !/^[\w.-]+\/[\w.-]+$/.test(repository)) { + throw new Error('GITHUB_TOKEN and REPOSITORY (owner/name) are required'); + } + const checkName = environment['CHECK_NAME'] ?? ''; + const scope = jobScope(checkName); + const { numbers, unidentifiedCommits } = await pullRequestNumbers(); + const verdicts: PullRequestVerdict[] = []; + for (const number of numbers) { + verdicts.push( + evaluatePullRequest( + await pullRequestSnapshot(client, number, scope.requireResolvedThreads), + scope + ) + ); + } + report( + checkName === '' + ? summaryMarkdown(verdicts, unidentifiedCommits) + : summaryMarkdown(verdicts, unidentifiedCommits, checkName, STAGE_ONE_NOTE) + ); + const failed = verdicts.filter((verdict) => !verdict.passed); + if (failed.length === 0 && unidentifiedCommits.length === 0) { + return; + } + const pending = failed.some((verdict) => verdict.waiting) + ? ': a check is still pending, this job re-runs by itself when it finishes (see docs/merge-queue.md)' + : ''; + process.stderr.write(`::error::PR gates failed${pending}\n`); + process.exitCode = 1; +} + +try { + await main(); +} catch (error: unknown) { + process.stderr.write( + `::error::PR gates could not be evaluated: ${error instanceof Error ? error.message : 'unknown error'}\n` + ); + process.exitCode = 1; +} diff --git a/tsconfig.json b/tsconfig.json index 88cf678..38483fc 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,6 +4,8 @@ "module": "ESNext", "moduleResolution": "bundler", "jsx": "react-jsx", + "lib": ["ES2023", "DOM", "DOM.Iterable"], + "allowImportingTsExtensions": true, "strict": true, "esModuleInterop": true, "skipLibCheck": true, @@ -15,6 +17,6 @@ "node_modules/@salla.sa/twilight-theme-engine/dist/ambient/salla-components.d.ts", "node_modules/@salla.sa/twilight-theme-engine/dist/ambient/virtual-modules.d.ts" ], - "include": ["app/**/*", "vite.config.ts", "*.d.ts"], + "include": ["app/**/*", "scripts/**/*", "vite.config.ts", "*.d.ts"], "exclude": ["node_modules", "dist"] } diff --git a/vitest.config.ts b/vitest.config.ts index 27bd3f7..3c5270b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -19,7 +19,7 @@ export default defineConfig({ globals: true, environment: 'jsdom', setupFiles: ['./tests/setup.tsx'], - include: ['tests/**/*.test.{ts,tsx}'], + include: ['tests/**/*.test.{ts,tsx}', 'scripts/**/*.test.ts'], // The style tests compile the theme's Sass; under a full parallel run they pass alone but exceed the 5s default. testTimeout: 20000, server: { From 04f711bcc7f56465c1551b9b22c5c7e5031098be Mon Sep 17 00:00:00 2001 From: Salah Al-Nawah Date: Tue, 27 Oct 2026 09:05:25 +0000 Subject: [PATCH 2/4] ci(themes): typecheck after build and show the PR gates API error (SAL-3553) Typecheck needs the generated route tree, so it runs after the build. PR gates now reports GitHub's error message and sends a User-Agent. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 9 +++++---- scripts/ci/prGatesApi.ts | 7 ++++++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a8c7f6..8fee2e1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,7 +41,7 @@ jobs: needs: ${{ toJSON(needs) }} static: - name: Lint, Types & Catalogue + name: Lint & Static Checks if: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} timeout-minutes: 15 @@ -52,8 +52,6 @@ jobs: uses: actions/checkout@v4 - name: Set up pnpm and Node uses: ./.github/actions/setup - - name: Typecheck - run: pnpm typecheck - name: Lint run: pnpm lint - name: Format check @@ -100,7 +98,7 @@ jobs: endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} build: - name: Build & Bundle Size + name: Build, Types & Bundle Size if: github.event_name != 'pull_request' || github.event.pull_request.draft == false runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} timeout-minutes: 15 @@ -111,6 +109,9 @@ jobs: uses: ./.github/actions/setup - name: Build run: pnpm build + # Typecheck needs app/routeTree.gen.ts, which the build generates on a clean checkout. + - name: Typecheck + run: pnpm typecheck - name: Report the bundle size run: | mkdir -p bundle-report diff --git a/scripts/ci/prGatesApi.ts b/scripts/ci/prGatesApi.ts index 761e9ae..99265d3 100644 --- a/scripts/ci/prGatesApi.ts +++ b/scripts/ci/prGatesApi.ts @@ -41,12 +41,17 @@ async function request( authorization: `Bearer ${client.token}`, accept: 'application/vnd.github+json', 'x-github-api-version': '2022-11-28', + 'user-agent': 'theme-raed-react-pr-gates', }, body: body === undefined ? undefined : JSON.stringify(body), }); if (!response.ok) { + const detail = await response.json().then( + (reason) => String(recordOf(reason)['message'] ?? ''), + () => '' + ); throw new Error( - `GitHub answered ${String(response.status)} to ${method} ${new URL(url).pathname}` + `GitHub answered ${String(response.status)} to ${method} ${new URL(url).pathname}${detail ? `: ${detail}` : ''}` ); } return response.json(); From e457f412852ce05fe9e0913ae6289f52ac3d121a Mon Sep 17 00:00:00 2001 From: Salah Al-Nawah Date: Tue, 27 Oct 2026 10:18:06 +0000 Subject: [PATCH 3/4] ci(themes): drop internal dependencies and report in a PR comment (SAL-3553) This repository is public: remove the use of private Salla actions, the reports worker and the org-bot PR gates. The CI now uses public actions only and posts one summary comment on the pull request. Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 176 +++++++++------- .github/workflows/pr-gates-merge-group.yml | 86 -------- .github/workflows/pr-gates-rerun.yml | 61 ------ .github/workflows/pr-gates.yml | 48 ----- docs/ci.md | 32 ++- eslint.config.js | 7 - scripts/ci/prGates.test.ts | 232 --------------------- scripts/ci/prGates.ts | 216 ------------------- scripts/ci/prGatesApi.test.ts | 170 --------------- scripts/ci/prGatesApi.ts | 209 ------------------- scripts/ci/prGatesCli.ts | 108 ---------- tsconfig.json | 4 +- vitest.config.ts | 4 +- 13 files changed, 112 insertions(+), 1241 deletions(-) delete mode 100644 .github/workflows/pr-gates-merge-group.yml delete mode 100644 .github/workflows/pr-gates-rerun.yml delete mode 100644 .github/workflows/pr-gates.yml delete mode 100644 scripts/ci/prGates.test.ts delete mode 100644 scripts/ci/prGates.ts delete mode 100644 scripts/ci/prGatesApi.test.ts delete mode 100644 scripts/ci/prGatesApi.ts delete mode 100644 scripts/ci/prGatesCli.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fee2e1..cd0d6ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,21 +1,19 @@ -name: Theme Raed React — Quality Gates +name: CI -# A draft runs nothing. A pull request that is ready for review runs every gate on each push; the merge queue (when -# enabled) runs them on the exact result it would merge. Reports are published to the shared tests-reports worker. +# Quality gates. A draft runs nothing; a pull request that is ready for review runs every gate on each push. +# Results are summarised in one comment on the pull request (no external services involved). on: pull_request: branches: - master - feature/quality-gates-twig-parity-audit types: [opened, synchronize, reopened, ready_for_review, converted_to_draft] - merge_group: - types: [checks_requested] push: branches: - feature/quality-gates-twig-parity-audit -# A draft run skips every job, so it gets its own group: sharing one with the real run lets a late-starting draft run -# cancel the ready_for_review run and leave every gate failed at 0s. +# A draft run skips every job, so it gets its own group: sharing one with the real run could cancel the +# ready_for_review run. concurrency: group: quality-gates-${{ github.event.pull_request.number || github.ref }}${{ github.event.pull_request.draft && '-draft' || '' }} cancel-in-progress: true @@ -28,22 +26,10 @@ defaults: shell: bash jobs: - # Required check for the "Tested & Coverage" ruleset: one verdict over every gate below. - testing-result: - name: Testing Ready - needs: [static, unit, build] - # Not on a draft: every gate is skipped there, and skipped counts as passed. - if: always() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - steps: - - uses: SallaApp/.CI/testing-ready@master - with: - needs: ${{ toJSON(needs) }} - static: name: Lint & Static Checks if: github.event_name != 'pull_request' || github.event.pull_request.draft == false - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + runs-on: ubuntu-latest timeout-minutes: 15 env: NODE_OPTIONS: --max-old-space-size=6144 @@ -64,7 +50,7 @@ jobs: unit: name: Unit Tests & Coverage if: github.event_name != 'pull_request' || github.event.pull_request.draft == false - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + runs-on: ubuntu-latest timeout-minutes: 20 steps: - name: Checkout @@ -73,6 +59,22 @@ jobs: uses: ./.github/actions/setup - name: Run unit tests and enforce the coverage floor run: pnpm test:coverage + - name: Write the coverage report + if: always() + run: | + mkdir -p report + { + echo "### Unit tests and coverage" + if [ -f coverage/coverage-summary.json ]; then + echo + echo "| Metric | Covered | Total | % |" + echo "| --- | --- | --- | --- |" + jq -r '.total | to_entries[] | select(.key | IN("lines","statements","functions","branches")) | "| \(.key) | \(.value.covered) | \(.value.total) | \(.value.pct) |"' coverage/coverage-summary.json + else + echo + echo "No coverage summary was produced." + fi + } > report/coverage.md - name: Upload coverage report if: always() uses: actions/upload-artifact@v4 @@ -81,26 +83,18 @@ jobs: path: coverage/ if-no-files-found: warn retention-days: 7 - - name: Publish the coverage report + - name: Upload the report section if: always() - continue-on-error: true - uses: SallaApp/checkout-tests-reports/publish@master + uses: actions/upload-artifact@v4 with: - name: coverage - title: Unit tests and coverage - folders: | - coverage:coverage - entry: coverage/index.html - vitest-json: coverage/test-results.json - status: ${{ job.status }} - access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} - secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} - endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} + name: report-coverage + path: report/coverage.md + retention-days: 1 build: name: Build, Types & Bundle Size if: github.event_name != 'pull_request' || github.event.pull_request.draft == false - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} + runs-on: ubuntu-latest timeout-minutes: 15 steps: - name: Checkout @@ -112,67 +106,91 @@ jobs: # Typecheck needs app/routeTree.gen.ts, which the build generates on a clean checkout. - name: Typecheck run: pnpm typecheck - - name: Report the bundle size + - name: Write the bundle size report + if: always() run: | - mkdir -p bundle-report + mkdir -p report { - echo "Build output size (dist/)" - du -sh dist - echo - find dist -type f \( -name '*.js' -o -name '*.css' \) -printf '%s %p\n' | sort -rn | head -40 | awk '{printf "%8.1f KB %s\n", $1/1024, $2}' - } | tee bundle-report/bundle-size.txt - - name: Publish the bundle size + echo "### Bundle size" + if [ -d dist ]; then + echo + echo "Total build output: $(du -sh dist | cut -f1)" + echo + echo "Largest JS and CSS files:" + echo + echo '```' + find dist -type f \( -name '*.js' -o -name '*.css' \) -printf '%s %p\n' | sort -rn | head -15 | awk '{printf "%8.1f KB %s\n", $1/1024, $2}' + echo '```' + else + echo + echo "The build produced no output." + fi + } > report/size.md + - name: Upload the report section if: always() - continue-on-error: true - uses: SallaApp/checkout-tests-reports/publish@master + uses: actions/upload-artifact@v4 with: - name: size - title: Bundle size - folders: | - bundle-report - entry: bundle-size.txt - status: ${{ job.status }} - access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} - secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} - endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} + name: report-size + path: report/size.md + retention-days: 1 - # Not in Testing Ready's needs: a reports problem never blocks a merge. Builds the run index and posts the PR comment. + # One verdict over every gate above: the single check to require in a branch ruleset. + testing-result: + name: Testing Ready + needs: [static, unit, build] + if: always() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) + runs-on: ubuntu-latest + steps: + - name: Every gate passed + env: + NEEDS: ${{ toJSON(needs) }} + run: | + echo "$NEEDS" | jq -r 'to_entries[] | "\(.key): \(.value.result)"' + echo "$NEEDS" | jq -e 'all(.[]; .result == "success")' > /dev/null + + # Not part of Testing Ready: a comment problem never blocks a merge. reports: name: Test Reports needs: [static, unit, build] - if: always() && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 15 + if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft == false + runs-on: ubuntu-latest + timeout-minutes: 10 continue-on-error: true - # One run at a time per pull request, so two runs never post the result comment at the same moment. concurrency: - group: test-reports-${{ github.event.pull_request.number || github.ref }} + group: test-reports-${{ github.event.pull_request.number }} cancel-in-progress: false permissions: contents: read pull-requests: write steps: - # `unit` publishes the coverage row, so its own `needs` row would be a second, empty one: fold it in. - - name: Fold the unit verdict into the coverage row - id: verdicts - if: always() + - name: Download the report sections + uses: actions/download-artifact@v4 + with: + path: report + pattern: report-* + merge-multiple: true continue-on-error: true + - name: Comment on the pull request env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} NEEDS: ${{ toJSON(needs) }} run: | + marker='' { - echo 'needs<> "$GITHUB_OUTPUT" - - name: Build the run index and comment on the pull request - if: always() - continue-on-error: true - uses: SallaApp/checkout-tests-reports/finalize@master - with: - base-url: ${{ secrets.REPORTS_BASE_URL }} - needs: ${{ steps.verdicts.outputs.needs || toJSON(needs) }} - github-token: ${{ github.token }} - access-key-id: ${{ secrets.R2_REPORTS_ACCESS_KEY_ID }} - secret-access-key: ${{ secrets.R2_REPORTS_SECRET_ACCESS_KEY }} - endpoint: ${{ secrets.R2_REPORTS_ENDPOINT }} + echo "$marker" + echo "## Test report" + echo + echo "| Gate | Result |" + echo "| --- | --- |" + echo "$NEEDS" | jq -r 'to_entries[] | "| \(.key) | \(.value.result) |"' + for f in report/coverage.md report/size.md; do + if [ -f "$f" ]; then echo; cat "$f"; fi + done + } > comment.md + existing="$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" --paginate --jq ".[] | select(.body | contains(\"$marker\")) | .id" | head -n 1)" + if [ -n "$existing" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing" -F body=@comment.md + else + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" -F body=@comment.md + fi diff --git a/.github/workflows/pr-gates-merge-group.yml b/.github/workflows/pr-gates-merge-group.yml deleted file mode 100644 index 798ef6c..0000000 --- a/.github/workflows/pr-gates-merge-group.yml +++ /dev/null @@ -1,86 +0,0 @@ -name: Theme Raed React — Merge group verification - -# The jobs below carry the names of the checks the repository ruleset requires and exist for the merge queue only: the group commit never -# gets Greptile or the organisation validators, so each re-verifies the stage 1 result on the head of every pull request of the group -# (docs/merge-queue.md). They are in their own workflow, triggered by `merge_group` alone, because a job skipped by an `if` still -# reports a check run, and a skipped check counts as passed: on a pull request those names must belong to the real apps only. -on: - merge_group: - types: [checks_requested] - -concurrency: - group: pr-gates-merge-group-${{ github.ref }} - cancel-in-progress: false - -permissions: - contents: read - pull-requests: read - checks: read - -defaults: - run: - shell: bash - -jobs: - greptile-review: - name: Greptile Review - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 5 - env: - CHECK_NAME: Greptile Review - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - node-version-file: .nvmrc - - name: Verify the stage 1 result - env: - EVENT: ${{ github.event_name }} - REPOSITORY: ${{ github.repository }} - GITHUB_TOKEN: ${{ github.token }} - MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} - MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} - MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} - run: node scripts/ci/prGatesCli.ts - - validate-pr-title: - name: Validate PR title - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 5 - env: - CHECK_NAME: Validate PR title - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - node-version-file: .nvmrc - - name: Verify the stage 1 result - env: - EVENT: ${{ github.event_name }} - REPOSITORY: ${{ github.repository }} - GITHUB_TOKEN: ${{ github.token }} - MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} - MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} - MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} - run: node scripts/ci/prGatesCli.ts - - validate-branch-name: - name: Validate branch name - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 5 - env: - CHECK_NAME: Validate branch name - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - node-version-file: .nvmrc - - name: Verify the stage 1 result - env: - EVENT: ${{ github.event_name }} - REPOSITORY: ${{ github.repository }} - GITHUB_TOKEN: ${{ github.token }} - MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} - MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} - MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} - run: node scripts/ci/prGatesCli.ts diff --git a/.github/workflows/pr-gates-rerun.yml b/.github/workflows/pr-gates-rerun.yml deleted file mode 100644 index e0f104b..0000000 --- a/.github/workflows/pr-gates-rerun.yml +++ /dev/null @@ -1,61 +0,0 @@ -name: Theme Raed React — PR gates re-run - -# `PR gates` reads live state, but a pull request run only starts on a pull request event. Greptile, the validators and review threads -# change without one, so this workflow re-runs the `PR gates` run of that head commit when they do. GitHub limits what can be -# used here: `check_run` workflows run from the default branch and attach to its commit, so they cannot report the gate themselves -# (they re-run the pull request's run instead); a check run made with GITHUB_TOKEN never starts a workflow, but Greptile and the -# validators are apps, so theirs do. Resolving a review thread starts no workflow (there is no such event), so the next pull request -# event or a manual re-run picks it up; GitHub's own conversation resolution rule and the merge group jobs hold that gate meanwhile. -# Nothing here is a required check. -on: - check_run: - types: [completed] - pull_request_review: - types: [submitted, dismissed] - -concurrency: - group: pr-gates-rerun-${{ github.event.check_run.head_sha || github.event.pull_request.head.sha }} - cancel-in-progress: false - -permissions: - contents: read - actions: write - -jobs: - rerun: - name: Re-run PR gates - # Only the three checks `PR gates` reads, and only from the apps it trusts (the script re-checks the names). - if: >- - github.event_name == 'pull_request_review' || - contains(fromJSON('["Greptile Review", "Validate PR title", "Validate branch name"]'), github.event.check_run.name) - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 10 - steps: - - name: Re-run PR gates of the head commit - env: - GH_TOKEN: ${{ github.token }} - REPOSITORY: ${{ github.repository }} - HEAD_SHA: ${{ github.event.check_run.head_sha || github.event.pull_request.head.sha }} - run: | - if [[ ! "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]]; then - echo "::error::No head commit to re-run for" - exit 1 - fi - for attempt in 1 2 3 4 5 6 7 8 9 10; do - run=$(gh api "repos/$REPOSITORY/actions/workflows/pr-gates.yml/runs?head_sha=$HEAD_SHA&event=pull_request&per_page=1" \ - --jq '.workflow_runs[0] | "\(.id) \(.status) \(.conclusion)"') - read -r run_id status conclusion <<<"$run" - if [[ -z "${run_id:-}" || "$run_id" == "null" ]]; then - echo "No PR gates run on $HEAD_SHA yet: the next pull request event starts one." - exit 0 - fi - if [[ "$status" == "completed" ]]; then - gh api -X POST "repos/$REPOSITORY/actions/runs/$run_id/rerun" - echo "Re-ran PR gates run $run_id (it was $conclusion): the state it checks changed." - exit 0 - fi - echo "PR gates run $run_id is $status; waiting (attempt $attempt)." - sleep 30 - done - echo "::error::PR gates did not finish in time to be re-run" - exit 1 diff --git a/.github/workflows/pr-gates.yml b/.github/workflows/pr-gates.yml deleted file mode 100644 index 4d14816..0000000 --- a/.github/workflows/pr-gates.yml +++ /dev/null @@ -1,48 +0,0 @@ -name: Theme Raed React — PR gates - -# Two stages (docs/merge-queue.md). Stage 1 is the pull request: the real Greptile app, the organisation's validators and the fast -# tier report on it. Stage 2 is the merge queue (pr-gates-merge-group.yml). -# `PR gates` runs on both and is the anti-spoof gate: the required checks must come from their real apps, not from a workflow that -# borrows a name. It is re-run by pr-gates-rerun.yml when Greptile or a validator finishes. -on: - pull_request: - types: [opened, synchronize, reopened, ready_for_review, edited] - merge_group: - types: [checks_requested] - -concurrency: - group: pr-gates-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - -permissions: - contents: read - pull-requests: read - checks: read - -defaults: - run: - shell: bash - -jobs: - # Passes only when `Greptile Review` on the head of the pull request (every pull request of a merge group) was reported by the Greptile - # app (id 867647), the title and branch validators by their real app, no review thread is open and the pull request is not a draft. - pr-gates: - name: PR gates - runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} - timeout-minutes: 5 - steps: - - name: Checkout - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: - node-version-file: .nvmrc - - name: Evaluate the gates - env: - EVENT: ${{ github.event_name }} - REPOSITORY: ${{ github.repository }} - GITHUB_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} - MERGE_GROUP_HEAD_REF: ${{ github.event.merge_group.head_ref }} - MERGE_GROUP_BASE_SHA: ${{ github.event.merge_group.base_sha }} - MERGE_GROUP_HEAD_SHA: ${{ github.event.merge_group.head_sha }} - run: node scripts/ci/prGatesCli.ts diff --git a/docs/ci.md b/docs/ci.md index ab697fc..daa124b 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -1,22 +1,14 @@ # CI quality gates -Ported from `SallaApp/checkout-react/.github`, limited to what this repository has. - -| Workflow | What it does | -| -------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `ci.yml` | `static` (typecheck, lint, format, manifest, knip), `unit` (coverage floor), `build` (build + bundle size), `Testing Ready` verdict, `Test Reports` index | -| `pr-gates.yml` | `PR gates`: the Greptile app, the title and branch validators passed on the head commit, no open review thread, not a draft | -| `pr-gates-merge-group.yml` | Merge-queue-only jobs that re-verify the stage 1 result (`Greptile Review`, `Validate PR title`, `Validate branch name`) | -| `pr-gates-rerun.yml` | Re-runs `PR gates` when Greptile, a validator or a review changes | - -Reports (coverage, bundle size) are pushed to the shared tests-reports worker through -`SallaApp/checkout-tests-reports/publish` and `/finalize`. They need the repository secrets -`R2_REPORTS_ACCESS_KEY_ID`, `R2_REPORTS_SECRET_ACCESS_KEY`, `R2_REPORTS_ENDPOINT` and `REPORTS_BASE_URL`; -publishing is `continue-on-error`, so missing secrets never block a merge. `RUNS_ON` (repository variable) selects the -runner and falls back to `ubuntu-latest`. - -Not ported because this repository has no Storybook, Playwright E2E, API contract tests, Lighthouse setup or Cloudflare -checkout Worker: `storybook-baselines`, `figma-visual*`, `e2e-nightly`, `api-drift`, `deploy`. - -Admin setup for the required checks (`Testing Ready`, `Greptile Review`, `Validate PR title`, `Validate branch name`, -`PR gates`) and the merge queue follows checkout-react's `docs/merge-queue.md`. +`.github/workflows/ci.yml` runs on every pull request that is ready for review (and on pushes to the base branch). +It uses only public actions and has no dependency on private repositories, organization variables or external services. + +| Job | What it does | +| ---------------------------- | ------------------------------------------------------------------------- | +| `Lint & Static Checks` | lint, format check, manifest check, dead code (knip) | +| `Unit Tests & Coverage` | `pnpm test:coverage`, which enforces the coverage floor | +| `Build, Types & Bundle Size` | build, typecheck (needs the generated route tree), bundle size | +| `Testing Ready` | one verdict over the three jobs above: the check to require | +| `Test Reports` | posts one summary comment on the pull request and updates it on each push | + +Reports are plain text in that comment, with no links. The coverage HTML is kept as the `coverage` workflow artifact for 7 days. diff --git a/eslint.config.js b/eslint.config.js index b6de273..b0f135f 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -80,13 +80,6 @@ export default tseslint.config( languageOptions: { globals: { ...globals.node } }, }, - // CI scripts run on Node. - { - files: ['scripts/**/*.ts'], - languageOptions: { globals: { ...globals.node } }, - rules: { 'no-restricted-properties': 'off', 'no-restricted-globals': 'off' }, - }, - // Must stay last: turns off formatting rules that conflict with Prettier. prettier ); diff --git a/scripts/ci/prGates.test.ts b/scripts/ci/prGates.test.ts deleted file mode 100644 index 81bbeaa..0000000 --- a/scripts/ci/prGates.test.ts +++ /dev/null @@ -1,232 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { - ALL_GATES, - evaluatePullRequest, - GREPTILE_APP, - mergeGroupPullRequests, - SALLA_BOT_APP, - scopeOfCheck, - summaryMarkdown, - type CheckRunSummary, - type PullRequestSnapshot, -} from './prGates.ts'; - -function run(name: string, overrides: Partial = {}): CheckRunSummary { - const app = name === 'Greptile Review' ? GREPTILE_APP : SALLA_BOT_APP; - return { - id: 1, - name, - status: 'completed', - conclusion: 'success', - appId: app.appId, - appSlug: app.appSlug, - ...overrides, - }; -} - -function snapshot(overrides: Partial = {}): PullRequestSnapshot { - return { - number: 7, - headSha: 'a'.repeat(40), - isDraft: false, - checkRuns: [run('Greptile Review'), run('Validate PR title'), run('Validate branch name')], - unresolvedThreads: 0, - ...overrides, - }; -} - -describe('evaluatePullRequest', () => { - it('passes when every gate holds', () => { - expect(evaluatePullRequest(snapshot())).toEqual({ - number: 7, - passed: true, - reasons: [], - waiting: false, - }); - }); - - it('fails a draft', () => { - expect(evaluatePullRequest(snapshot({ isDraft: true })).reasons).toEqual([ - 'the pull request is a draft', - ]); - }); - - it('fails open review threads, singular and plural', () => { - expect(evaluatePullRequest(snapshot({ unresolvedThreads: 1 })).reasons).toEqual([ - '1 review thread is not resolved', - ]); - expect(evaluatePullRequest(snapshot({ unresolvedThreads: 3 })).reasons).toEqual([ - '3 review threads are not resolved', - ]); - }); - - it('waits for a check that has not been reported or has not finished', () => { - const missing = evaluatePullRequest( - snapshot({ checkRuns: [run('Validate PR title'), run('Validate branch name')] }) - ); - expect(missing).toMatchObject({ passed: false, waiting: true }); - expect(missing.reasons[0]).toContain('has not been reported by greptile-apps'); - const running = evaluatePullRequest( - snapshot({ - checkRuns: [ - run('Greptile Review', { status: 'in_progress', conclusion: null }), - run('Validate PR title'), - run('Validate branch name'), - ], - }) - ); - expect(running).toMatchObject({ - passed: false, - waiting: true, - reasons: ['"Greptile Review" is still in_progress'], - }); - }); - - it('fails a check that concluded anything but success, without waiting', () => { - const verdict = evaluatePullRequest( - snapshot({ - checkRuns: [ - run('Greptile Review', { conclusion: 'failure' }), - run('Validate PR title'), - run('Validate branch name', { conclusion: null }), - ], - }) - ); - expect(verdict.waiting).toBe(false); - expect(verdict.reasons).toEqual([ - '"Greptile Review" concluded failure', - '"Validate branch name" concluded without a result', - ]); - }); - - it('does not accept a check of the right name from the wrong app (a workflow borrowing the name)', () => { - const spoofed = run('Greptile Review', { id: 99, appId: 15_368, appSlug: 'github-actions' }); - const verdict = evaluatePullRequest( - snapshot({ checkRuns: [spoofed, run('Validate PR title'), run('Validate branch name')] }) - ); - expect(verdict).toMatchObject({ passed: false, waiting: true }); - }); - - it('does not let a spoofed success hide the real failure, nor the real one be replaced by an older run', () => { - const spoofed = run('Greptile Review', { id: 99, appId: 15_368, appSlug: 'github-actions' }); - const real = run('Greptile Review', { id: 5, conclusion: 'failure' }); - const verdict = evaluatePullRequest( - snapshot({ - checkRuns: [real, spoofed, run('Validate PR title'), run('Validate branch name')], - }) - ); - expect(verdict.reasons).toEqual(['"Greptile Review" concluded failure']); - const rerun = evaluatePullRequest( - snapshot({ - checkRuns: [ - run('Greptile Review', { id: 3, conclusion: 'failure' }), - run('Greptile Review', { id: 8 }), - run('Validate PR title'), - run('Validate branch name'), - ], - }) - ); - expect(rerun.passed).toBe(true); - }); - - it('requires the app slug as well as the id', () => { - const verdict = evaluatePullRequest( - snapshot({ - checkRuns: [ - run('Greptile Review', { appSlug: 'someone-else' }), - run('Validate PR title'), - run('Validate branch name'), - ], - }) - ); - expect(verdict.passed).toBe(false); - }); - - it('collects every reason', () => { - const verdict = evaluatePullRequest( - snapshot({ isDraft: true, checkRuns: [], unresolvedThreads: 2 }) - ); - expect(verdict.reasons).toHaveLength(5); - }); -}); - -describe('scopeOfCheck', () => { - it('scopes a merge group job to its check; only Greptile also needs resolved threads, none needs a non-draft', () => { - expect(scopeOfCheck('Greptile Review')).toMatchObject({ - rejectDraft: false, - requireResolvedThreads: true, - }); - expect(scopeOfCheck('Validate PR title')).toMatchObject({ requireResolvedThreads: false }); - expect(scopeOfCheck('Testing Ready')).toBeUndefined(); - }); - - it('verifies only its own check', () => { - const scope = scopeOfCheck('Validate PR title'); - const onlyTitle = snapshot({ - isDraft: true, - unresolvedThreads: 4, - checkRuns: [run('Validate PR title')], - }); - expect(scope && evaluatePullRequest(onlyTitle, scope).passed).toBe(true); - expect(evaluatePullRequest(onlyTitle, ALL_GATES).passed).toBe(false); - }); -}); - -describe('mergeGroupPullRequests', () => { - const REF = 'refs/heads/gh-readonly-queue/master/pr-12-0123456789abcdef0123456789abcdef01234567'; - - it('reads the last pull request from the branch name, with or without refs/heads', () => { - expect(mergeGroupPullRequests(REF, ['feat: x (#12)'])).toEqual({ - numbers: [12], - unidentifiedCommits: [], - }); - expect( - mergeGroupPullRequests(REF.replace('refs/heads/', ''), ['feat: x (#12)']).numbers - ).toEqual([12]); - }); - - it('adds every pull request named by the squash commits of the group, sorted, and ignores earlier suffixes in a title', () => { - expect( - mergeGroupPullRequests(REF, ['fix: a (#10)', 'fix: b (#3) (#11)', 'feat: x (#12) ']) - ).toEqual({ - numbers: [10, 11, 12], - unidentifiedCommits: [], - }); - }); - - it('reports a commit that names no pull request, so the check fails closed', () => { - expect( - mergeGroupPullRequests(REF, ['Merge pull request #5 from x/y', 'feat: x (#12)']) - ).toEqual({ - numbers: [12], - unidentifiedCommits: ['Merge pull request #5 from x/y'], - }); - }); - - it('reports a branch that is not a queue branch when nothing else identifies a pull request', () => { - expect(mergeGroupPullRequests('refs/heads/main', [])).toEqual({ - numbers: [], - unidentifiedCommits: ['refs/heads/main'], - }); - expect( - mergeGroupPullRequests('gh-readonly-queue/master/pr-0-abcdef1', ['x (#0)']) - .unidentifiedCommits - ).toEqual(['x (#0)']); - }); -}); - -describe('summaryMarkdown', () => { - it('lists the verdicts, the reasons and the unidentified commits, never a title or a branch name', () => { - const failed = evaluatePullRequest(snapshot({ isDraft: true })); - const text = summaryMarkdown([evaluatePullRequest(snapshot({ number: 1 })), failed], ['x']); - expect(text).toBe( - '## PR gates\n\n- #1: passed\n- #7: failed\n - the pull request is a draft\n- 1 commit(s) of the merge group name no pull request (squash merge expected)\n' - ); - }); - - it('takes a heading and a note for the merge group jobs', () => { - expect(summaryMarkdown([], [], 'Greptile Review', 'note')).toBe( - '## Greptile Review\n\nnote\n\n' - ); - }); -}); diff --git a/scripts/ci/prGates.ts b/scripts/ci/prGates.ts deleted file mode 100644 index b8d3e43..0000000 --- a/scripts/ci/prGates.ts +++ /dev/null @@ -1,216 +0,0 @@ -/** - * The decision behind the `PR gates` check (.github/workflows/pr-gates.yml, docs/merge-queue.md). The org rulesets "Pull Requests" - * and "Tested & Coverage" require `Greptile Review`, `Validate PR title` and `Validate branch name`, which never run on a merge queue - * commit, so the queue would wait for them forever. This check reports on the pull request and on the merge group instead, and - * reproduces what those rules enforce: the same checks green on the head of every pull request, no open review thread, not a draft. - * Everything here is a pure function of data the CLI fetched; none of it comes from a command line. - */ - -/** A check that must have succeeded, and the GitHub App that must have reported it (a workflow cannot impersonate it by name). */ -interface RequiredCheck { - readonly name: string; - readonly appId: number; - readonly appSlug: string; -} - -const GREPTILE_CHECK_NAME = 'Greptile Review'; - -export const GREPTILE_APP = { appId: 867_647, appSlug: 'greptile-apps' } as const; - -/** `Validate PR title` and `Validate branch name` come from the organisation's internal bot, not from a workflow of this repository. */ -export const SALLA_BOT_APP = { appId: 1_131_467, appSlug: 'sallainternalbot' } as const; - -const REQUIRED_CHECKS: readonly RequiredCheck[] = [ - { name: GREPTILE_CHECK_NAME, ...GREPTILE_APP }, - { name: 'Validate PR title', ...SALLA_BOT_APP }, - { name: 'Validate branch name', ...SALLA_BOT_APP }, -]; - -/** The fields of a check run the decision needs. */ -export interface CheckRunSummary { - readonly id: number; - readonly name: string; - readonly status: string; - readonly conclusion: string | null; - readonly appId: number | undefined; - readonly appSlug: string | undefined; -} - -export interface PullRequestSnapshot { - readonly number: number; - readonly headSha: string; - readonly isDraft: boolean; - readonly checkRuns: readonly CheckRunSummary[]; - /** Review threads that are not resolved (outdated ones included: the ruleset's conversation resolution counts them too). */ - readonly unresolvedThreads: number; -} - -export interface PullRequestVerdict { - readonly number: number; - readonly passed: boolean; - /** Why it did not pass, one line each; empty when it passed. */ - readonly reasons: readonly string[]; - /** True when a failing reason is only that a check has not finished or not started: re-evaluating later can change it. */ - readonly waiting: boolean; -} - -interface CheckVerdict { - readonly reason: string | undefined; - readonly waiting: boolean; -} - -/** The newest run of a check reported by the required app; runs of the same name from any other app do not count. */ -function latestRunOf( - required: RequiredCheck, - runs: readonly CheckRunSummary[] -): CheckRunSummary | undefined { - let latest: CheckRunSummary | undefined; - for (const run of runs) { - const fromRequiredApp = - run.name === required.name && - run.appId === required.appId && - run.appSlug === required.appSlug; - if (fromRequiredApp && (latest === undefined || run.id > latest.id)) { - latest = run; - } - } - return latest; -} - -function checkVerdict(required: RequiredCheck, runs: readonly CheckRunSummary[]): CheckVerdict { - const run = latestRunOf(required, runs); - if (run === undefined) { - return { - reason: `"${required.name}" has not been reported by ${required.appSlug} on the head commit yet`, - waiting: true, - }; - } - if (run.status !== 'completed') { - return { reason: `"${required.name}" is still ${run.status}`, waiting: true }; - } - return { - reason: - run.conclusion === 'success' - ? undefined - : `"${required.name}" concluded ${run.conclusion ?? 'without a result'}`, - waiting: false, - }; -} - -/** What one job verifies: the default is every gate of `PR gates`; the merge group jobs named after a check verify one each. */ -export interface GateScope { - readonly required: readonly RequiredCheck[]; - readonly rejectDraft: boolean; - readonly requireResolvedThreads: boolean; -} - -export const ALL_GATES: GateScope = { - required: REQUIRED_CHECKS, - rejectDraft: true, - requireResolvedThreads: true, -}; - -/** Passes only when every required check succeeded on the head commit, no review thread is open and the pull request is not a draft. */ -export function evaluatePullRequest( - snapshot: PullRequestSnapshot, - scope: GateScope = ALL_GATES -): PullRequestVerdict { - const reasons: string[] = []; - let waiting = false; - if (scope.rejectDraft && snapshot.isDraft) { - reasons.push('the pull request is a draft'); - } - for (const check of scope.required) { - const verdict = checkVerdict(check, snapshot.checkRuns); - if (verdict.reason === undefined) { - continue; - } - reasons.push(verdict.reason); - waiting ||= verdict.waiting; - } - if (scope.requireResolvedThreads && snapshot.unresolvedThreads > 0) { - reasons.push( - `${String(snapshot.unresolvedThreads)} review thread${snapshot.unresolvedThreads === 1 ? ' is' : 's are'} not resolved` - ); - } - return { number: snapshot.number, passed: reasons.length === 0, reasons, waiting }; -} - -/** The scope of a job from its `CHECK_NAME` ("Greptile Review" also needs resolved threads); undefined for a name that is not a required check. */ -export function scopeOfCheck(name: string): GateScope | undefined { - const required = REQUIRED_CHECKS.filter((check) => check.name === name); - return required.length === 0 - ? undefined - : { required, rejectDraft: false, requireResolvedThreads: name === GREPTILE_CHECK_NAME }; -} - -/** `gh-readonly-queue//pr--` (with or without `refs/heads/`): the pull request the queue added last. */ -const QUEUE_REF = /^(?:refs\/heads\/)?gh-readonly-queue\/.+\/pr-(\d+)-[\da-f]{7,40}$/; - -/** The `(#N)` GitHub appends to the title of a squash commit; only the last one counts, a title can contain others. */ -const SQUASH_SUFFIX = /\(#(\d+)\)$/; - -export interface MergeGroupPullRequests { - /** Every pull request in the group, ascending. */ - readonly numbers: readonly number[]; - /** Commits of the group whose subject names no pull request; the group cannot be fully identified and must fail. */ - readonly unidentifiedCommits: readonly string[]; -} - -function parseNumber(digits: string | undefined): number | undefined { - const value = Number(digits); - return Number.isSafeInteger(value) && value > 0 ? value : undefined; -} - -/** - * The pull requests of a merge group. The branch name only names the last one added, so the commits of the group - * (`base_sha...head_sha`) are read as well: with the squash merge method of the queue each carries `(#N)` at the end of its subject. - * A commit that does not (another merge method) is reported instead of ignored, so the check fails closed. - */ -export function mergeGroupPullRequests( - headRef: string, - commitSubjects: readonly string[] -): MergeGroupPullRequests { - const numbers = new Set(); - const unidentifiedCommits: string[] = []; - const fromRef = parseNumber(QUEUE_REF.exec(headRef)?.[1]); - if (fromRef !== undefined) { - numbers.add(fromRef); - } - for (const subject of commitSubjects) { - const fromSubject = parseNumber(SQUASH_SUFFIX.exec(subject.trim())?.[1]); - if (fromSubject === undefined) { - unidentifiedCommits.push(subject); - } else { - numbers.add(fromSubject); - } - } - if (numbers.size === 0 && unidentifiedCommits.length === 0) { - unidentifiedCommits.push(headRef); - } - return { numbers: [...numbers].toSorted((a, b) => a - b), unidentifiedCommits }; -} - -/** The job summary (Markdown): one line per pull request. Titles and branch names never appear in it, only numbers and fixed texts. */ -export function summaryMarkdown( - verdicts: readonly PullRequestVerdict[], - unidentifiedCommits: readonly string[] = [], - heading = 'PR gates', - note = '' -): string { - const lines = [`## ${heading}`, '', ...(note === '' ? [] : [note, ''])]; - for (const verdict of verdicts) { - lines.push( - verdict.passed - ? `- #${String(verdict.number)}: passed` - : `- #${String(verdict.number)}: failed`, - ...verdict.reasons.map((reason) => ` - ${reason}`) - ); - } - if (unidentifiedCommits.length > 0) { - lines.push( - `- ${String(unidentifiedCommits.length)} commit(s) of the merge group name no pull request (squash merge expected)` - ); - } - return `${lines.join('\n')}\n`; -} diff --git a/scripts/ci/prGatesApi.test.ts b/scripts/ci/prGatesApi.test.ts deleted file mode 100644 index 92832b8..0000000 --- a/scripts/ci/prGatesApi.test.ts +++ /dev/null @@ -1,170 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { - commitSubjects, - pullRequestSnapshot, - type Fetch, - type GitHubClient, -} from './prGatesApi.ts'; - -const SHA = 'b'.repeat(40); - -function clientAnswering( - answer: (url: string, body: string | undefined) => { status?: number; body: unknown } -): { - client: GitHubClient; - urls: string[]; -} { - const urls: string[] = []; - const fetchStub: Fetch = (url, init) => { - urls.push(`${init.method} ${url}`); - const { status = 200, body } = answer(url, init.body); - return Promise.resolve({ ok: status < 400, status, json: () => Promise.resolve(body) }); - }; - return { - client: { fetch: fetchStub, token: 't', apiUrl: 'https://api.test', repository: 'o/r' }, - urls, - }; -} - -describe('commitSubjects', () => { - it('returns the first line of each commit of the range', async () => { - const { client, urls } = clientAnswering(() => ({ - body: { commits: [{ commit: { message: 'a (#1)\n\nbody' } }, { commit: {} }] }, - })); - expect(await commitSubjects(client, SHA, SHA)).toEqual(['a (#1)', '']); - expect(urls[0]).toContain(`/repos/o/r/compare/${SHA}...${SHA}?per_page=100&page=1`); - }); - - it('refuses a base or head that is not a full SHA', async () => { - const { client } = clientAnswering(() => ({ body: {} })); - await expect(commitSubjects(client, '--help', SHA)).rejects.toThrow('full commit SHAs'); - }); - - it('follows pages and gives up on a huge list', async () => { - const page = { - commits: Array.from({ length: 100 }, () => ({ commit: { message: 'x (#1)' } })), - }; - const { client, urls } = clientAnswering((url) => ({ - body: url.endsWith('page=2') ? { commits: [] } : page, - })); - expect(await commitSubjects(client, SHA, SHA)).toHaveLength(100); - expect(urls).toHaveLength(2); - const endless = clientAnswering(() => ({ body: page })); - await expect(commitSubjects(endless.client, SHA, SHA)).rejects.toThrow('More than'); - }); -}); - -const DRAFT_PULL = { head: { sha: SHA }, draft: true }; - -type Answer = (url: string) => { status?: number; body: unknown }; - -function pullAnswers(threadPages: readonly unknown[], pull: unknown = DRAFT_PULL): Answer { - let thread = 0; - return (url: string): { status?: number; body: unknown } => { - if (url.endsWith('/graphql')) { - return { body: threadPages[thread++] }; - } - if (url.includes('/check-runs')) { - return { - body: { - check_runs: [ - { - id: 4, - name: 'Greptile Review', - status: 'completed', - conclusion: 'success', - app: { id: 867_647, slug: 'greptile-apps' }, - }, - 'junk', - ], - }, - }; - } - return { body: pull }; - }; -} - -function threads(nodes: readonly unknown[], next?: string): unknown { - return { - data: { - repository: { - pullRequest: { - reviewThreads: { pageInfo: { hasNextPage: next !== undefined, endCursor: next }, nodes }, - }, - }, - }, - }; -} - -async function failureOf(answer: Answer): Promise { - try { - await pullRequestSnapshot(clientAnswering(answer).client, 9); - } catch (error: unknown) { - return error instanceof Error ? error.message : ''; - } - return ''; -} - -describe('pullRequestSnapshot', () => { - it('reads the live head, draft state, check runs of that head and unresolved threads across pages', async () => { - const { client, urls } = clientAnswering( - pullAnswers([ - threads([{ isResolved: false }, { isResolved: true }], 'c1'), - threads([{ isResolved: false }]), - ]) - ); - const result = await pullRequestSnapshot(client, 9); - expect(result).toMatchObject({ number: 9, headSha: SHA, isDraft: true, unresolvedThreads: 2 }); - expect(result.checkRuns[0]).toEqual({ - id: 4, - name: 'Greptile Review', - status: 'completed', - conclusion: 'success', - appId: 867_647, - appSlug: 'greptile-apps', - }); - expect(result.checkRuns[1]).toEqual({ - id: 0, - name: '', - status: '', - conclusion: null, - appId: undefined, - appSlug: undefined, - }); - expect( - urls.some((url) => url.includes(`/commits/${SHA}/check-runs?filter=latest&per_page=100`)) - ).toBe(true); - }); - - it('skips the threads when the job does not need them, and treats a missing draft flag as ready', async () => { - const { client, urls } = clientAnswering(pullAnswers([], { head: { sha: SHA } })); - expect(await pullRequestSnapshot(client, 9, false)).toMatchObject({ - isDraft: false, - unresolvedThreads: 0, - }); - expect(urls.some((url) => url.endsWith('/graphql'))).toBe(false); - }); - - it('fails on a head that is not a commit, an API error, GraphQL errors and endless thread pages', async () => { - expect(await failureOf(() => ({ body: {} }))).toContain('no readable head'); - expect(await failureOf(() => ({ status: 403, body: {} }))).toContain( - '403 to GET /repos/o/r/pulls/9' - ); - expect(await failureOf(pullAnswers([{ errors: [{}] }]))).toContain('GraphQL returned errors'); - const endless = Array.from({ length: 25 }, () => threads([], 'next')); - expect(await failureOf(pullAnswers(endless))).toContain('Too many review threads'); - }); - - it('stops when a page says there is more but gives no cursor', async () => { - const noCursor = { - data: { - repository: { - pullRequest: { reviewThreads: { pageInfo: { hasNextPage: true }, nodes: [] } }, - }, - }, - }; - const { client } = clientAnswering(pullAnswers([noCursor, threads([])])); - const snapshotWithoutCursor = await pullRequestSnapshot(client, 9); - expect(snapshotWithoutCursor.unresolvedThreads).toBe(0); - }); -}); diff --git a/scripts/ci/prGatesApi.ts b/scripts/ci/prGatesApi.ts deleted file mode 100644 index 99265d3..0000000 --- a/scripts/ci/prGatesApi.ts +++ /dev/null @@ -1,209 +0,0 @@ -/** - * The GitHub reads of the `PR gates` check, over `fetch` so a test can answer them. Read-only: the token needs `contents`, `pull-requests` - * and `checks` read access. Every value that reaches a URL is a validated number or SHA, never free text. - */ -import type { CheckRunSummary, PullRequestSnapshot } from './prGates.ts'; - -export type Fetch = ( - url: string, - init: { - readonly method: string; - readonly headers: Record; - readonly body?: string | undefined; - } -) => Promise<{ - readonly ok: boolean; - readonly status: number; - json: () => Promise; -}>; - -export interface GitHubClient { - readonly fetch: Fetch; - readonly token: string; - /** `https://api.github.com` unless GitHub Enterprise says otherwise. */ - readonly apiUrl: string; - /** `owner/name`. */ - readonly repository: string; -} - -const PAGE_LIMIT = 20; -const COMMIT_SHA = /^[\da-f]{40}$/; - -async function request( - client: GitHubClient, - method: string, - url: string, - body?: unknown -): Promise { - const response = await client.fetch(url, { - method, - headers: { - authorization: `Bearer ${client.token}`, - accept: 'application/vnd.github+json', - 'x-github-api-version': '2022-11-28', - 'user-agent': 'theme-raed-react-pr-gates', - }, - body: body === undefined ? undefined : JSON.stringify(body), - }); - if (!response.ok) { - const detail = await response.json().then( - (reason) => String(recordOf(reason)['message'] ?? ''), - () => '' - ); - throw new Error( - `GitHub answered ${String(response.status)} to ${method} ${new URL(url).pathname}${detail ? `: ${detail}` : ''}` - ); - } - return response.json(); -} - -function recordOf(value: unknown): Record { - return typeof value === 'object' && value !== null ? (value as Record) : {}; -} - -/** The value at a path of keys, or undefined when any step is not an object. */ -function reach(value: unknown, keys: readonly string[]): unknown { - let current = value; - for (const key of keys) { - current = recordOf(current)[key]; - } - return current; -} - -function firstLine(text = ''): string { - return text.split('\n', 1)[0] ?? ''; -} - -function listOf(value: unknown): readonly unknown[] { - return Array.isArray(value) ? value : []; -} - -function textOf(value: unknown): string | undefined { - return typeof value === 'string' ? value : undefined; -} - -function numberOf(value: unknown): number | undefined { - return typeof value === 'number' ? value : undefined; -} - -/** Every item of a paged REST list, read from `field` of each page (or the page itself when it is the array). */ -async function pagedItems( - client: GitHubClient, - path: string, - field: string | undefined -): Promise { - const items: unknown[] = []; - for (let page = 1; page <= PAGE_LIMIT; page += 1) { - const separator = path.includes('?') ? '&' : '?'; - const body = await request( - client, - 'GET', - `${client.apiUrl}${path}${separator}per_page=100&page=${String(page)}` - ); - const pageItems = listOf(field === undefined ? body : recordOf(body)[field]); - items.push(...pageItems); - if (pageItems.length < 100) { - return items; - } - } - throw new Error(`More than ${String(PAGE_LIMIT * 100)} items at ${path}`); -} - -function checkRunOf(raw: unknown): CheckRunSummary { - const run = recordOf(raw); - const app = recordOf(run['app']); - return { - id: numberOf(run['id']) ?? 0, - name: textOf(run['name']) ?? '', - status: textOf(run['status']) ?? '', - conclusion: textOf(run['conclusion']) ?? null, - appId: numberOf(app['id']), - appSlug: textOf(app['slug']), - }; -} - -/** The subjects (first lines) of the commits `base...head` adds: for a merge group, one per pull request with the squash method. */ -export async function commitSubjects( - client: GitHubClient, - base: string, - head: string -): Promise { - if (!COMMIT_SHA.test(base) || !COMMIT_SHA.test(head)) { - throw new Error('The merge group base and head must be full commit SHAs'); - } - const commits = await pagedItems( - client, - `/repos/${client.repository}/compare/${base}...${head}`, - 'commits' - ); - return commits.map((commit) => firstLine(textOf(reach(commit, ['commit', 'message'])))); -} - -const THREADS_QUERY = `query($owner: String!, $name: String!, $number: Int!, $after: String) { - repository(owner: $owner, name: $name) { - pullRequest(number: $number) { - reviewThreads(first: 100, after: $after) { - pageInfo { hasNextPage endCursor } - nodes { isResolved } - } - } - } -}`; - -async function unresolvedThreadCount(client: GitHubClient, number: number): Promise { - const [owner = '', name = ''] = client.repository.split('/', 2); - let unresolved = 0; - let after: string | null = null; - for (let page = 0; page < PAGE_LIMIT; page += 1) { - const body = recordOf( - await request(client, 'POST', `${client.apiUrl}/graphql`, { - query: THREADS_QUERY, - variables: { owner, name, number, after }, - }) - ); - if (listOf(body['errors']).length > 0) { - throw new Error('GitHub GraphQL returned errors reading the review threads'); - } - const threads = recordOf(reach(body, ['data', 'repository', 'pullRequest', 'reviewThreads'])); - unresolved += listOf(threads['nodes']).filter( - (node) => recordOf(node)['isResolved'] !== true - ).length; - const pageInfo = recordOf(threads['pageInfo']); - if (pageInfo['hasNextPage'] !== true) { - return unresolved; - } - after = textOf(pageInfo['endCursor']) ?? null; - } - throw new Error('Too many review threads to read'); -} - -/** The live state of a pull request: its current head, whether it is a draft, its check runs on that head and its open threads. */ -export async function pullRequestSnapshot( - client: GitHubClient, - number: number, - withThreads = true -): Promise { - const pull = recordOf( - await request( - client, - 'GET', - `${client.apiUrl}/repos/${client.repository}/pulls/${String(number)}` - ) - ); - const headSha = textOf(recordOf(pull['head'])['sha']) ?? ''; - if (!COMMIT_SHA.test(headSha)) { - throw new Error(`Pull request #${String(number)} has no readable head commit`); - } - const runs = await pagedItems( - client, - `/repos/${client.repository}/commits/${headSha}/check-runs?filter=latest`, - 'check_runs' - ); - return { - number, - headSha, - isDraft: pull['draft'] === true, - checkRuns: runs.map((run) => checkRunOf(run)), - unresolvedThreads: withThreads ? await unresolvedThreadCount(client, number) : 0, - }; -} diff --git a/scripts/ci/prGatesCli.ts b/scripts/ci/prGatesCli.ts deleted file mode 100644 index cf2c7b3..0000000 --- a/scripts/ci/prGatesCli.ts +++ /dev/null @@ -1,108 +0,0 @@ -/** - * `node scripts/ci/prGatesCli.ts`: the `PR gates` check. Reads `EVENT`, `REPOSITORY`, `GITHUB_TOKEN` and, for a pull request, - * `PR_NUMBER`, or, for a merge group, `MERGE_GROUP_HEAD_REF`, `MERGE_GROUP_BASE_SHA` and `MERGE_GROUP_HEAD_SHA` from the environment. - * `CHECK_NAME` (merge group jobs named after a required check) narrows it to that one check: the job then verifies the stage 1 result - * of the real check on each pull request instead of reporting a check of its own. Prints one verdict per pull request, appends it to the job summary and exits 1 when any pull request does not pass. - */ -import { appendFileSync } from 'node:fs'; -import { commitSubjects, pullRequestSnapshot, type GitHubClient } from './prGatesApi.ts'; -import { - ALL_GATES, - evaluatePullRequest, - mergeGroupPullRequests, - scopeOfCheck, - summaryMarkdown, - type GateScope, - type PullRequestVerdict, -} from './prGates.ts'; - -const environment = process.env; -const token = environment['GITHUB_TOKEN'] ?? ''; -const repository = environment['REPOSITORY'] ?? ''; - -const client: GitHubClient = { - fetch: (url, { body, ...init }) => fetch(url, body === undefined ? init : { ...init, body }), - token, - apiUrl: environment['GITHUB_API_URL'] ?? 'https://api.github.com', - repository, -}; - -async function pullRequestNumbers(): Promise<{ - numbers: readonly number[]; - unidentifiedCommits: readonly string[]; -}> { - if (environment['EVENT'] === 'merge_group') { - const subjects = await commitSubjects( - client, - environment['MERGE_GROUP_BASE_SHA'] ?? '', - environment['MERGE_GROUP_HEAD_SHA'] ?? '' - ); - return mergeGroupPullRequests(environment['MERGE_GROUP_HEAD_REF'] ?? '', subjects); - } - const number = Number(environment['PR_NUMBER']); - if (!Number.isSafeInteger(number) || number <= 0) { - throw new Error('PR_NUMBER is not a pull request number'); - } - return { numbers: [number], unidentifiedCommits: [] }; -} - -/** The scope of this job: every gate, or the one check named by `CHECK_NAME` (the merge group jobs). */ -function jobScope(checkName: string): GateScope { - const scope = checkName === '' ? ALL_GATES : scopeOfCheck(checkName); - if (scope === undefined) { - throw new Error('CHECK_NAME is not a required check'); - } - return scope; -} - -const STAGE_ONE_NOTE = - 'Verifies the stage 1 result: the real check on the head of each pull request in the merge group (docs/merge-queue.md).'; - -function report(markdown: string): void { - process.stdout.write(markdown); - const summary = environment['GITHUB_STEP_SUMMARY']; - if (summary !== undefined) { - appendFileSync(summary, markdown); - } -} - -async function main(): Promise { - if (token === '' || !/^[\w.-]+\/[\w.-]+$/.test(repository)) { - throw new Error('GITHUB_TOKEN and REPOSITORY (owner/name) are required'); - } - const checkName = environment['CHECK_NAME'] ?? ''; - const scope = jobScope(checkName); - const { numbers, unidentifiedCommits } = await pullRequestNumbers(); - const verdicts: PullRequestVerdict[] = []; - for (const number of numbers) { - verdicts.push( - evaluatePullRequest( - await pullRequestSnapshot(client, number, scope.requireResolvedThreads), - scope - ) - ); - } - report( - checkName === '' - ? summaryMarkdown(verdicts, unidentifiedCommits) - : summaryMarkdown(verdicts, unidentifiedCommits, checkName, STAGE_ONE_NOTE) - ); - const failed = verdicts.filter((verdict) => !verdict.passed); - if (failed.length === 0 && unidentifiedCommits.length === 0) { - return; - } - const pending = failed.some((verdict) => verdict.waiting) - ? ': a check is still pending, this job re-runs by itself when it finishes (see docs/merge-queue.md)' - : ''; - process.stderr.write(`::error::PR gates failed${pending}\n`); - process.exitCode = 1; -} - -try { - await main(); -} catch (error: unknown) { - process.stderr.write( - `::error::PR gates could not be evaluated: ${error instanceof Error ? error.message : 'unknown error'}\n` - ); - process.exitCode = 1; -} diff --git a/tsconfig.json b/tsconfig.json index 38483fc..88cf678 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,8 +4,6 @@ "module": "ESNext", "moduleResolution": "bundler", "jsx": "react-jsx", - "lib": ["ES2023", "DOM", "DOM.Iterable"], - "allowImportingTsExtensions": true, "strict": true, "esModuleInterop": true, "skipLibCheck": true, @@ -17,6 +15,6 @@ "node_modules/@salla.sa/twilight-theme-engine/dist/ambient/salla-components.d.ts", "node_modules/@salla.sa/twilight-theme-engine/dist/ambient/virtual-modules.d.ts" ], - "include": ["app/**/*", "scripts/**/*", "vite.config.ts", "*.d.ts"], + "include": ["app/**/*", "vite.config.ts", "*.d.ts"], "exclude": ["node_modules", "dist"] } diff --git a/vitest.config.ts b/vitest.config.ts index 3c5270b..da7753a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -19,7 +19,7 @@ export default defineConfig({ globals: true, environment: 'jsdom', setupFiles: ['./tests/setup.tsx'], - include: ['tests/**/*.test.{ts,tsx}', 'scripts/**/*.test.ts'], + include: ['tests/**/*.test.{ts,tsx}'], // The style tests compile the theme's Sass; under a full parallel run they pass alone but exceed the 5s default. testTimeout: 20000, server: { @@ -30,7 +30,7 @@ export default defineConfig({ }, coverage: { provider: 'v8', - reporter: ['text', 'json', 'html', 'lcov'], + reporter: ['text', 'json', 'json-summary', 'html', 'lcov'], include: ['app/**/*.{ts,tsx}'], exclude: ['app/**/*.d.ts'], // Coverage floor (ratchet): measured on app/** and rounded down. It only goes up — From ff3ded72371ad40cd81ba2e1e1c6ec46d50e7c33 Mon Sep 17 00:00:00 2001 From: Salah Al-Nawah Date: Tue, 27 Oct 2026 10:29:50 +0000 Subject: [PATCH 4/4] ci(themes): post the PR report from the org runner and keep a job summary (SAL-3553) The organization IP allow list rejects REST calls from GitHub-hosted runners (HTTP 403). Made-with: agent Co-authored-by: mc-agents[bot] <298817976+mc-agents[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 7 ++++++- docs/ci.md | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd0d6ea..38f68f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -153,7 +153,9 @@ jobs: name: Test Reports needs: [static, unit, build] if: always() && github.event_name == 'pull_request' && github.event.pull_request.draft == false - runs-on: ubuntu-latest + # The organization's API IP allow list rejects calls from GitHub-hosted runners, so the comment is posted from the + # organization runner when the RUNS_ON variable is set (a JSON runner label or group), else from ubuntu-latest. + runs-on: ${{ vars.RUNS_ON && fromJson(vars.RUNS_ON) || 'ubuntu-latest' }} timeout-minutes: 10 continue-on-error: true concurrency: @@ -171,6 +173,8 @@ jobs: merge-multiple: true continue-on-error: true - name: Comment on the pull request + # The report is always in the job summary too, so a comment the API refuses does not fail the run. + continue-on-error: true env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} @@ -188,6 +192,7 @@ jobs: if [ -f "$f" ]; then echo; cat "$f"; fi done } > comment.md + cat comment.md >> "$GITHUB_STEP_SUMMARY" existing="$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" --paginate --jq ".[] | select(.body | contains(\"$marker\")) | .id" | head -n 1)" if [ -n "$existing" ]; then gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing" -F body=@comment.md diff --git a/docs/ci.md b/docs/ci.md index daa124b..7a01a84 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -11,4 +11,4 @@ It uses only public actions and has no dependency on private repositories, organ | `Testing Ready` | one verdict over the three jobs above: the check to require | | `Test Reports` | posts one summary comment on the pull request and updates it on each push | -Reports are plain text in that comment, with no links. The coverage HTML is kept as the `coverage` workflow artifact for 7 days. +The `Test Reports` job posts with the workflow token (`pull-requests: write`, no extra credentials) and also writes the report to the job summary. The organization restricts API calls by IP, so it runs on the `RUNS_ON` runner when that variable is set. Reports are plain text in that comment, with no links. The coverage HTML is kept as the `coverage` workflow artifact for 7 days.