Skip to content
Closed
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
51 changes: 47 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
timeout-minutes: 20
outputs:
docs_required: ${{ steps.paths.outputs.docs_required }}
perf_required: ${{ steps.paths.outputs.perf_required }}
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
Expand Down Expand Up @@ -113,8 +114,11 @@ jobs:
run: bun install --frozen-lockfile
- name: Install coverage toolchain
run: cargo install cargo-llvm-cov --version "$CARGO_LLVM_COV_VERSION" --locked
- name: Install benchmark browser
run: bun run browser:install
# `bench:verify` runs `render-driver.ts --smoke`, which launches Chromium and
# nothing else. Installing Firefox and WebKit here downloads two engines plus
# their system dependencies on every run for no consumer.
- name: Install Chromium for the render smoke
run: bun run browser:install:chromium
- name: Download canonical tarballs
uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5
with:
Expand Down Expand Up @@ -407,7 +411,27 @@ jobs:

controlled-performance:
name: Matched Zero-Regression Performance
needs: artifact-build
# `preflight` is in `needs` because the `if:` below reads its output. The
# `needs` context exposes DIRECT dependencies only, so omitting it would make
# the expression resolve against an undefined context and skip this job on
# every run. Execution order is unchanged: `artifact-build` already needs
# `preflight`.
needs: [preflight, artifact-build]
# Two independent conditions, both required.
#
# 1. Cost: this job occupies a single self-hosted runner for up to 90 minutes
# and every other pull request queues behind it, so it runs only when the
# diff could move a measurement. `requiresPerformanceRun` defaults to true.
# 2. Security: this is the only NON-ephemeral runner in the workflow. It must
# never check out and execute code from a fork — a persistent host exposes
# its filesystem, caches, and network position regardless of the read-only
# token. Fork pull requests skip it; the measurement is taken instead by the
# `push` run on `develop` after merge, which is same-repository by
# construction.
if: >-
needs.preflight.outputs.perf_required == 'true' &&
(github.event_name == 'push' ||
github.event.pull_request.head.repo.full_name == github.repository)
runs-on: sheetwrite-perf-i9-12900h-cachyos
timeout-minutes: 90
steps:
Expand Down Expand Up @@ -482,14 +506,15 @@ jobs:
BROWSER: ${{ needs.browser-smoke.result }}
PERFORMANCE: ${{ needs.controlled-performance.result }}
DOCS_REQUIRED: ${{ needs.preflight.outputs.docs_required }}
PERF_REQUIRED: ${{ needs.preflight.outputs.perf_required }}
PERF_FROM_FORK: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}
run: |
test "$PREFLIGHT" = success
test "$UNIT_COVERAGE" = success
test "$ARTIFACT_BUILD" = success
test "$PACKED" = success
test "$BUNDLERS" = success
test "$SIZE" = success
test "$PERFORMANCE" = success
if [ "$DOCS_REQUIRED" = "true" ]; then
test "$DOCS" = success
test "$BROWSER" = success
Expand All @@ -498,3 +523,21 @@ jobs:
test "$DOCS" = skipped
test "$BROWSER" = skipped
fi
# The self-hosted benchmark never runs fork code (see
# `controlled-performance`). That protects the host but must not become a
# way to merge perf-sensitive changes without evidence: a fork could edit
# `bench/results/render-baseline.json` and, if we deferred to a post-merge
# run, be validated against its own altered baseline. So a fork pull
# request is allowed through only when the diff needs no measurement.
if [ "$PERF_REQUIRED" != "true" ]; then
test "$PERF_REQUIRED" = "false"
test "$PERFORMANCE" = skipped
elif [ "$PERF_FROM_FORK" = "true" ]; then
echo "This pull request changes performance-sensitive paths, but the" >&2
echo "matched zero-regression benchmark does not run fork code on the" >&2
echo "self-hosted runner. A maintainer must replay these commits on a" >&2
echo "same-repository branch so the gate runs before merge." >&2
exit 1
else
test "$PERFORMANCE" = success
fi
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"test:browser": "playwright test --config test/browser/playwright.config.ts",
"browser:install:chromium": "playwright install --with-deps chromium",
"test:browser:portability": "playwright test --config test/browser/playwright.config.ts --grep @portability",
"test:tooling": "bun test scripts/toolchain-contract.test.ts scripts/workspace-tooling.test.ts scripts/dependency-audit.test.ts scripts/verify-clean-build.test.ts scripts/size-report.test.ts scripts/release-artifacts.test.ts scripts/consumer-lock.test.ts scripts/release-verify.test.ts scripts/release-workflow.test.ts scripts/workflow-contract.test.ts scripts/public-api.test.ts scripts/coverage-check.test.ts scripts/sheetwrite-code-hovers.test.ts scripts/normalize-sitemap.test.ts",
"test:tooling": "bun test scripts/toolchain-contract.test.ts scripts/workspace-tooling.test.ts scripts/dependency-audit.test.ts scripts/verify-clean-build.test.ts scripts/size-report.test.ts scripts/release-artifacts.test.ts scripts/consumer-lock.test.ts scripts/release-verify.test.ts scripts/release-workflow.test.ts scripts/workflow-contract.test.ts scripts/ci-paths.test.ts scripts/public-api.test.ts scripts/coverage-check.test.ts scripts/sheetwrite-code-hovers.test.ts scripts/normalize-sitemap.test.ts",
"test:coverage:ts": "bun scripts/coverage.ts ts",
"test:coverage:rust": "bun scripts/coverage.ts rust",
"test:coverage": "bun run test:coverage:ts && bun run test:coverage:rust",
Expand Down
48 changes: 47 additions & 1 deletion scripts/ci-paths.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "bun:test";
import { requiresDocumentationBuild } from "./ci-paths.js";
import { requiresDocumentationBuild, requiresPerformanceRun } from "./ci-paths.js";

describe("documentation CI path classification", () => {
it("runs for documentation, public API, package, and browser-contract changes", () => {
Expand Down Expand Up @@ -40,3 +40,49 @@ describe("documentation CI path classification", () => {
expect(requiresDocumentationBuild([])).toBeTrue();
});
});

describe("performance CI path classification", () => {
it("runs for engine, renderer, harness, manifest, and workflow changes", () => {
for (const path of [
"packages/core/src/canvas-paint.ts",
"packages/core/src/store/data-engine.ts",
"packages/wasm/src/query.rs",
"packages/wasm/Cargo.toml",
"packages/react/src/index.ts",
"bench/src/render-driver.ts",
// The gate compares fresh samples against this reference, so a change to it
// must re-run the benchmark rather than skip it.
"bench/results/render-baseline.json",
"package.json",
"bun.lock",
".github/workflows/ci.yml",
"scripts/ci-paths.ts",
]) {
expect(requiresPerformanceRun([path]), path).toBeTrue();
}
});

it("skips only paths proven unable to move a measurement", () => {
expect(
requiresPerformanceRun([
".changeset/release-note.md",
".github/ISSUE_TEMPLATE/bug-report.yml",
"README.md",
"SECURITY.md",
"docs/src/routes/index.tsx",
"examples/vanilla/index.html",
"packages/core/test/grid.test.ts",
"packages/core/README.md",
"bench/test/render-gate.test.ts",
"scripts/release-workflow.test.ts",
"test/browser/vanilla-workbench.spec.ts",
]),
).toBeFalse();
});

it("takes the expensive lane for mixed, unknown, and empty change sets", () => {
expect(requiresPerformanceRun([".changeset/note.md", "packages/wasm/src/sheet.rs"])).toBeTrue();
expect(requiresPerformanceRun(["new-subsystem/config.json"])).toBeTrue();
expect(requiresPerformanceRun([])).toBeTrue();
});
});
38 changes: 37 additions & 1 deletion scripts/ci-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,40 @@ export function requiresDocumentationBuild(paths: readonly string[]): boolean {
);
}

/**
* Paths proven unable to change measured runtime performance. Everything else —
* every `src` tree, manifest, lockfile, and workflow — selects the expensive lane.
*/
const PERFORMANCE_INDEPENDENT_PATHS = [
/^\.changeset\//,
/^\.github\/ISSUE_TEMPLATE\//,
/^\.github\/(?:CODEOWNERS|dependabot\.yml)$/,
/^(?:CONTRIBUTING|LICENSE|README|SECURITY|SUPPORT)\.md$/,
/^docs\//,
/^examples\//,
/^packages\/[^/]+\/(?:test|tests)\//,
/^packages\/[^/]+\/README\.md$/,
// `bench/test/` only. `bench/results/render-baseline.json` is the reference the
// zero-regression gate compares against (`ci.yml` check step), so a change there
// must run the benchmark rather than skip it.
/^bench\/test\//,
/^scripts\/[^/]+\.test\.ts$/,
/^test\//,
] as const;

/**
* Returns true unless every changed path is proven independent from measured
* performance. The matched zero-regression benchmark occupies a single
* self-hosted runner for up to 90 minutes, so it is skipped when the change
* cannot move it — but unknown paths, mixed sets, and empty sets deliberately
* take the expensive lane: a false negative ships a regression, a false positive
* costs one benchmark run.
*/
export function requiresPerformanceRun(paths: readonly string[]): boolean {
if (paths.length === 0) return true;
return paths.some((path) => !PERFORMANCE_INDEPENDENT_PATHS.some((pattern) => pattern.test(path)));
}

function changedPaths(baseSha: string): readonly string[] {
if (!/^[0-9a-f]{40}$/.test(baseSha)) {
throw new Error("BASE_SHA must be a full lowercase commit SHA");
Expand All @@ -44,5 +78,7 @@ function changedPaths(baseSha: string): readonly string[] {
if (import.meta.main) {
const baseSha = process.env.BASE_SHA;
if (!baseSha) throw new Error("BASE_SHA is required");
console.log(`docs_required=${requiresDocumentationBuild(changedPaths(baseSha))}`);
const paths = changedPaths(baseSha);
console.log(`docs_required=${requiresDocumentationBuild(paths)}`);
console.log(`perf_required=${requiresPerformanceRun(paths)}`);
}
35 changes: 34 additions & 1 deletion scripts/toolchain-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,16 @@ describe("contributor and CI toolchain contract", () => {
const jobs = parsedWorkflow.jobs ?? {};
const controlled = jobs["controlled-performance"];
expect(controlled?.["runs-on"]).toBe("sheetwrite-perf-i9-12900h-cachyos");
expect(controlled?.needs).toBe("artifact-build");
// `preflight` is required in `needs` because the job's `if:` reads
// `needs.preflight.outputs.perf_required`; the `needs` context exposes direct
// dependencies only, so omitting it would skip the gate on every run.
expect(controlled?.needs).toEqual(["preflight", "artifact-build"]);
const condition = String(controlled?.if ?? "").replace(/\s+/g, " ");
expect(condition).toContain("needs.preflight.outputs.perf_required == 'true'");
// The only non-ephemeral runner must never execute fork code.
expect(condition).toContain(
"github.event.pull_request.head.repo.full_name == github.repository",
);
const commands = controlled?.steps?.flatMap((step) => (step.run ? [step.run] : [])).join("\n");
expect(commands).toContain("--rounds 10");
expect(commands).toContain("src/check.ts");
Expand Down Expand Up @@ -206,6 +215,30 @@ describe("contributor and CI toolchain contract", () => {
expect(requiredCommand).toContain('test "$BROWSER" = skipped');
});

it("gates the performance benchmark without letting fork pull requests bypass it", () => {
const jobs = parsedWorkflow.jobs ?? {};
const preflight = jobs.preflight;
expect(preflight?.outputs?.perf_required).toBe("$" + "{{ steps.paths.outputs.perf_required }}");

const requiredJob = jobs.required;
const requiredCommand = requiredJob?.steps?.find((step) =>
step.run?.includes('test "$PREFLIGHT" = success'),
)?.run;

// A diff that needs no measurement may skip, and the skip must be exact.
expect(requiredCommand).toContain('if [ "$PERF_REQUIRED" != "true" ]');
expect(requiredCommand).toContain('test "$PERFORMANCE" = skipped');
expect(requiredCommand).toContain('test "$PERFORMANCE" = success');

// A fork pull request touching performance-sensitive paths must FAIL, not
// pass on a skip. Deferring to a post-merge run would let a fork alter
// `bench/results/render-baseline.json` and be validated against its own
// baseline after the merge already happened.
expect(JSON.stringify(requiredJob)).toContain("PERF_FROM_FORK");
expect(requiredCommand).toContain('elif [ "$PERF_FROM_FORK" = "true" ]');
expect(requiredCommand).toContain("exit 1");
});

it("shares a source-keyed Rust compilation cache across build and test jobs", () => {
const jobs = parsedWorkflow.jobs ?? {};
for (const jobName of ["artifact-build", "unit-coverage"]) {
Expand Down
59 changes: 59 additions & 0 deletions scripts/workflow-contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,65 @@ describe("CI and release workflow contracts", () => {
expect(changesetStatus?.if).toBeUndefined();
});

it("never grants a workflow write-scoped secrets against untrusted pull-request code", () => {
// `pull_request_target` runs with the base repository's token and secrets while
// checking out attacker-controllable head code. It is the trigger behind a
// well-known class of supply-chain compromises and has no use in this repo.
for (const [name, workflow] of Object.entries(workflows())) {
expect(workflow.on?.pull_request_target, `${name} pull_request_target`).toBeUndefined();
}
});

it("never executes fork pull-request code on a non-ephemeral runner", () => {
// GitHub-hosted runners are destroyed after each job; a self-hosted runner is
// a persistent machine whose filesystem, caches, and network position survive.
// Checking out fork code there is a host-compromise vector that a read-only
// token does not mitigate, so every self-hosted job must be gated to
// same-repository events.
for (const [workflowName, workflow] of Object.entries(workflows())) {
for (const [jobName, job] of Object.entries(workflow.jobs)) {
const runsOn = job["runs-on"];
const hosted = typeof runsOn === "string" && /^(?:ubuntu|windows|macos)-/.test(runsOn);
if (hosted) continue;
const condition = typeof job.if === "string" ? job.if : "";
expect(
condition.replace(/\s+/g, " "),
`${workflowName}.${jobName} runs on ${String(runsOn)} and must reject fork pull requests`,
).toContain("github.event.pull_request.head.repo.full_name == github.repository");
}
}
});

it("pins the CI trigger branches so release branches are an explicit decision", () => {
const triggers = workflows().ci.on;
expect((triggers?.push as { branches?: string[] } | undefined)?.branches).toEqual(["develop"]);
expect((triggers?.pull_request as { branches?: string[] } | undefined)?.branches).toEqual([
"develop",
]);
});

it("declares every job whose condition reads another job's outputs as a direct dependency", () => {
// `needs` exposes DIRECT dependencies only. A job that reads
// `needs.<other>.outputs` without listing `<other>` evaluates against an
// undefined context, so the condition is always false and the job silently
// never runs.
for (const [workflowName, workflow] of Object.entries(workflows())) {
for (const [jobName, job] of Object.entries(workflow.jobs)) {
const condition = typeof job.if === "string" ? job.if : "";
const referenced = [...condition.matchAll(/needs\.([A-Za-z0-9_-]+)\.outputs/g)].map(
(match) => match[1]!,
);
if (referenced.length === 0) continue;
const needs = job.needs === undefined ? [] : [job.needs].flat();
for (const dependency of referenced) {
expect(needs, `${workflowName}.${jobName} reads needs.${dependency}.outputs`).toContain(
dependency,
);
}
}
}
});

it("keeps each workflow's required toolchain versions in parity", () => {
const parsed = workflows();
for (const [name, workflow] of Object.entries(parsed)) {
Expand Down
Loading