diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 777e6a2b74..53cfd90926 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,10 @@ name: CI on: pull_request: + # Explicit list because the default (opened/synchronize/reopened) omits ready_for_review -- once the + # heavy jobs below start skipping draft PRs, marking a PR ready must itself trigger a real CI run, not + # wait for the next push. + types: [opened, synchronize, reopened, ready_for_review] push: branches: - main @@ -165,7 +169,9 @@ jobs: validate-code: name: validate-code needs: changes - if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true' }} + # draft != true also gates push runs correctly: github.event.pull_request is unset there, so the + # property access evaluates to null, and null != true is true. + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true')) }} runs-on: ubuntu-latest timeout-minutes: 45 env: @@ -446,7 +452,7 @@ jobs: validate-tests: name: validate-tests needs: changes - if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && needs.changes.outputs.backend == 'true') }} runs-on: ubuntu-latest timeout-minutes: 20 strategy: @@ -627,7 +633,7 @@ jobs: validate-tests-merge: name: validate-tests-merge needs: [changes, validate-tests] - if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && needs.changes.outputs.backend == 'true') }} runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -678,7 +684,7 @@ jobs: # audit workflow, so one upstream CVE never blocks unrelated PRs. security: name: security - if: ${{ github.event_name == 'pull_request' }} + if: ${{ github.event_name == 'pull_request' && github.event.pull_request.draft != true }} # actions/dependency-review-action needs no self-hosted toolchain/cache (checkout + a lockfile diff), so # this always runs on ubuntu-latest -- keeping it self-hosted for same-repo PRs only competed for the # scarce self-hosted pool with validate-code, the one job that actually benefits from it (#2501). diff --git a/test/unit/ci-skip-draft-prs.test.ts b/test/unit/ci-skip-draft-prs.test.ts new file mode 100644 index 0000000000..7bd7c228d6 --- /dev/null +++ b/test/unit/ci-skip-draft-prs.test.ts @@ -0,0 +1,53 @@ +import { readFileSync } from "node:fs"; +import { parse } from "yaml"; +import { describe, expect, it } from "vitest"; + +function readYaml(path: string): Record { + return record(parse(readFileSync(path, "utf8")), path); +} + +function record(value: unknown, label: string): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error(`${label} must be an object`); + } + return value as Record; +} + +// Regression guard: draft PRs were consuming the exact same 11-job GitHub-hosted-runner fan-out as ready +// PRs (validate-code, the 6-way validate-tests shard matrix, validate-tests-merge, security), which both +// (a) let contributors farm bot labels/AI review/screenshots for free while sitting in draft, and (b) +// materially worsened the account's shared-runner queue contention for every other PR in flight. The heavy +// jobs now require the PR not be a draft; the cheap `changes` and `validate` (result-aggregation) jobs still +// run unconditionally since `validate` already treats a skipped dependency as success. +describe("ci.yml skips the heavy jobs for draft pull requests", () => { + const workflow = readYaml(".github/workflows/ci.yml"); + const jobs = record(workflow.jobs, "workflow.jobs"); + + it("pull_request trigger explicitly includes ready_for_review (the default type list omits it)", () => { + const on = record(workflow.on, "workflow.on"); + const pullRequest = record(on.pull_request, "workflow.on.pull_request"); + expect(pullRequest.types).toEqual(["opened", "synchronize", "reopened", "ready_for_review"]); + }); + + it.each(["validate-code", "validate-tests", "validate-tests-merge"])( + "%s's if-condition requires github.event.pull_request.draft != true alongside the existing push/path-filter checks", + (jobName) => { + const job = record(jobs[jobName], `jobs.${jobName}`); + const condition = String(job.if); + expect(condition).toContain("github.event.pull_request.draft != true"); + // Push runs (no PR context at all) must still be unaffected -- github.event_name == 'push' short-circuits first. + expect(condition).toContain("github.event_name == 'push'"); + }, + ); + + it("security's if-condition requires github.event.pull_request.draft != true", () => { + const job = record(jobs.security, "jobs.security"); + expect(String(job.if)).toBe("${{ github.event_name == 'pull_request' && github.event.pull_request.draft != true }}"); + }); + + it("validate still aggregates all four gated jobs and treats a skipped dependency as success", () => { + const job = record(jobs.validate, "jobs.validate"); + expect(job.needs).toEqual(["changes", "validate-code", "validate-tests", "validate-tests-merge", "security"]); + expect(String(job.if)).toBe("${{ always() }}"); + }); +});