Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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).
Expand Down
53 changes: 53 additions & 0 deletions test/unit/ci-skip-draft-prs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { readFileSync } from "node:fs";
import { parse } from "yaml";
import { describe, expect, it } from "vitest";

function readYaml(path: string): Record<string, unknown> {
return record(parse(readFileSync(path, "utf8")), path);
}

function record(value: unknown, label: string): Record<string, unknown> {
if (!value || typeof value !== "object" || Array.isArray(value)) {
throw new Error(`${label} must be an object`);
}
return value as Record<string, unknown>;
}

// 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() }}");
});
});