Harden stacked CI scope and merge validation

.github/STACKED_CI.md

@@ -0,0 +1,9 @@
+# CI for stacked pull requests
+
+CI follows GitHub's [lowest unmerged pull request condition](https://docs.github.com/en/pull-requests/how-tos/merge-and-close-pull-requests/optimizing-ci-for-stacked-pull-requests): a native stacked PR gets full validation when its base branch matches the stack's base branch. After a bottom PR merges, GitHub rebases the next PR onto the stack base and its new CI run performs full validation. The original stack position can remain greater than 1.
+
+Ordinary PRs, fork PRs, outside contributors, pushes to `main`, and events with missing or incomplete scope metadata always receive full CI. Only upper stack layers authored by an `OWNER`, `MEMBER`, or `COLLABORATOR`, with both head and base branches in this repository, defer expensive jobs. Lint, formatting, and CI policy tests still run on every PR. Scope selection reads GitHub's event without checking out PR code and needs no token permissions.
+
+An upper layer's `test` check intentionally fails until that layer becomes the lowest unmerged PR and all CI lanes pass. Merge these stacks individually from the bottom up. This prevents an untested upper layer or contiguous group from passing the merge gate. Skipping the required check would not enforce this policy because GitHub treats skipped jobs as successful. The aggregate check also runs after failed or cancelled dependencies and requires every full CI lane to succeed.
+
+The stack base's branch protection or ruleset must require the GitHub Actions `test` check for this merge gate to be enforced. Workflow changes also need maintainer review: like other `pull_request` workflows, a PR can modify the workflow definition itself. Configure code owner review or equivalent repository rules for `.github/workflows/` as appropriate. The workflow does not configure repository protection or override permitted bypasses.

.github/workflows/ci.yml

@@ -10,20 +10,39 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  # Each layer gets full validation when it becomes the lowest unmerged PR.
-  # Upper layers retain code-quality checks while GitHub rebases the stack.
+  # GitHub recommends stack.base.ref == base.ref for the lowest unmerged PR:
+  # https://docs.github.com/en/pull-requests/how-tos/merge-and-close-pull-requests/optimizing-ci-for-stacked-pull-requests
+  # Only trusted, same-repository upper layers defer full CI. Forks, ordinary
+  # PRs, pushes and incomplete metadata always retain full validation.
   validation_scope:
     name: Select CI scope
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
+    timeout-minutes: 5
+    permissions: {}
     outputs:
       full: ${{ steps.scope.outputs.full }}
     steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v4
-        with:
-          node-version-file: .node-version
       - id: scope
-        run: node scripts/ci-stack-scope.mjs
+        # Read GitHub's event directly; do not run scripts from a PR checkout.
+        run: |
+          node <<'NODE'
+          const { appendFileSync, readFileSync } = require('node:fs');
+          const event = JSON.parse(readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8'));
+          const pr = event.pull_request;
+          const repository = event.repository?.full_name;
+          const sameRepository = typeof repository === 'string' && repository.length > 0 &&
+            pr?.head?.repo?.full_name === repository && pr?.base?.repo?.full_name === repository;
+          const trustedAuthor = ['OWNER', 'MEMBER', 'COLLABORATOR'].includes(pr?.author_association);
+          const stackBase = pr?.stack?.base?.ref;
+          const prBase = pr?.base?.ref;
+          // Position 1 can already be merged; compare the current base branches.
+          const upperLayer = process.env.GITHUB_EVENT_NAME === 'pull_request' &&
+            sameRepository && trustedAuthor && typeof stackBase === 'string' && stackBase.length > 0 &&
+            typeof prBase === 'string' && prBase.length > 0 && stackBase !== prBase;
+          const full = !upperLayer;
+          appendFileSync(process.env.GITHUB_OUTPUT, `full=${full}\n`);
+          console.log(full ? 'Run full CI' : 'Defer full CI until this PR becomes the lowest unmerged layer');
+          NODE
 
   code_quality:
     name: lint and format
@@ -714,14 +733,15 @@ jobs:
         with:
           runtime: ${{ matrix.runtime }}
 
-  # Retain the pre-matrix "test" check for branch protection and badges. It
-  # Full runs require every lane to pass. Upper stack layers pass only when
-  # code quality succeeds and every expensive job was intentionally skipped.
+  # Retain the pre-matrix "test" check for branch protection and badges.
+  # Require this check on the stack base. Upper layers must not pass before
+  # full validation, so untested groups cannot merge. Merge bottom-up.
   test:
     needs: [validation_scope, code_quality, native_macos, tests, native_linux, effects, bootstrap_seed, bootstrap, linux_host_clang, ts7_parity, llvm_artifacts_linux_x64, llvm_artifacts_macos_arm64, llvm_artifacts_macos_x64, windows_cli, llvm_artifacts_windows_x64, mobile_runtime_packs]
-    # A superseded run must release its concurrency slot immediately.
-    if: ${{ !cancelled() }}
+    # Run even after failed or cancelled lanes: skipped required jobs allow merging.
+    if: ${{ always() }}
     runs-on: ubuntu-latest
+    timeout-minutes: 5
     steps:
       - name: All lanes green
         run: |
@@ -745,20 +765,8 @@ jobs:
               test "${{ needs.mobile_runtime_packs.result }}" = success
               ;;
             false)
-              test "${{ needs.native_macos.result }}" = skipped
-              test "${{ needs.tests.result }}" = skipped
-              test "${{ needs.native_linux.result }}" = skipped
-              test "${{ needs.effects.result }}" = skipped
-              test "${{ needs.bootstrap_seed.result }}" = skipped
-              test "${{ needs.bootstrap.result }}" = skipped
-              test "${{ needs.linux_host_clang.result }}" = skipped
-              test "${{ needs.ts7_parity.result }}" = skipped
-              test "${{ needs.llvm_artifacts_linux_x64.result }}" = skipped
-              test "${{ needs.llvm_artifacts_macos_arm64.result }}" = skipped
-              test "${{ needs.llvm_artifacts_macos_x64.result }}" = skipped
-              test "${{ needs.windows_cli.result }}" = skipped
-              test "${{ needs.llvm_artifacts_windows_x64.result }}" = skipped
-              test "${{ needs.mobile_runtime_packs.result }}" = skipped
+              echo "::notice::Full CI is deferred until this PR becomes the lowest unmerged layer. Merge the stack from the bottom up."
+              exit 1
               ;;
             *)
               echo "Invalid CI scope"

scripts/ci-stack-scope.mjs

@@ -1,25 +0,0 @@
-import { appendFileSync, readFileSync } from "node:fs";
-import { fileURLToPath } from "node:url";
-
-export function requiresFullCi(eventName, event) {
-  if (eventName !== "pull_request") return true;
-  const pr = event?.pull_request;
-  const stackBase = pr?.stack?.base?.ref;
-  const prBase = pr?.base?.ref;
-  // Missing or incomplete stack metadata must retain full validation. The
-  // lowest unmerged layer targets the stack base; its position need not be 1.
-  return (
-    typeof stackBase !== "string" ||
-    stackBase.length === 0 ||
-    typeof prBase !== "string" ||
-    prBase.length === 0 ||
-    stackBase === prBase
-  );
-}
-
-if (process.argv[1] === fileURLToPath(import.meta.url)) {
-  const event = JSON.parse(readFileSync(process.env.GITHUB_EVENT_PATH, "utf8"));
-  const full = requiresFullCi(process.env.GITHUB_EVENT_NAME, event);
-  appendFileSync(process.env.GITHUB_OUTPUT, `full=${full}\n`);
-  console.log(full ? "Run full CI" : "Run code quality for an upper stack layer");
-}

tests/harness/ci-stack-scope.test.ts

@@ -1,40 +1,161 @@
 import { spawnSync } from "node:child_process";
-import { readFileSync } from "node:fs";
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { load } from "js-yaml";
 import { expect, test } from "vitest";
-import { requiresFullCi } from "../../scripts/ci-stack-scope.mjs";
-
-const stacked = (base: string, position = 1) => ({
-  pull_request: { base: { ref: base }, stack: { base: { ref: "main" }, position } },
-});
-
-test.each([
-  ["ordinary PR", "pull_request", { pull_request: { base: { ref: "main" } } }, true],
-  ["bottom layer", "pull_request", stacked("main"), true],
-  ["new bottom after a merge", "pull_request", stacked("main", 2), true],
-  ["upper layer", "pull_request", stacked("feature/lower", 3), false],
-  ["main push", "push", {}, true],
-  ["push with stack metadata", "push", stacked("feature/lower"), true],
-  ["missing event", "pull_request", undefined, true],
-  ["missing stack base", "pull_request", { pull_request: { stack: {} } }, true],
-  ["missing PR base", "pull_request", { pull_request: { stack: { base: { ref: "main" } } } }, true],
-  ["empty PR base", "pull_request", stacked(""), true],
-])("CI selects the correct scope for %s", (_name, eventName, event, full) => {
-  expect(requiresFullCi(eventName, event)).toBe(full);
-});
 
 type Job = {
   needs?: string | string[];
   if?: string;
-  steps?: { run?: string }[];
+  permissions?: Record<string, string>;
+  steps?: { run?: string; uses?: string }[];
 };
 const workflow = load(
   readFileSync(new URL("../../.github/workflows/ci.yml", import.meta.url), "utf8"),
 ) as { jobs: Record<string, Job> };
+const scopeJob = workflow.jobs.validation_scope!;
+const scopeScript = scopeJob.steps!.find((step) => step.run)!.run!;
 const heavyJobs = Object.keys(workflow.jobs).filter(
   (name) => !["validation_scope", "code_quality", "test"].includes(name),
 );
 
+const repository = "vercel-labs/scriptc";
+const stacked = (base: string, position = 3) => ({
+  repository: { full_name: repository },
+  pull_request: {
+    author_association: "MEMBER",
+    head: { repo: { full_name: repository } },
+    base: { ref: base, repo: { full_name: repository } },
+    stack: { base: { ref: "main" }, position },
+  },
+});
+const upper = stacked("feature/lower");
+
+function runScope(eventName: string, event: unknown) {
+  const directory = mkdtempSync(join(tmpdir(), "ci-stack-scope-"));
+  try {
+    const eventPath = join(directory, "event.json");
+    const outputPath = join(directory, "output");
+    writeFileSync(eventPath, JSON.stringify(event));
+    writeFileSync(outputPath, "");
+    // Execute the actual workflow step, rather than a second copy of its policy.
+    const result = spawnSync("bash", ["-e", "-o", "pipefail", "-c", scopeScript], {
+      encoding: "utf8",
+      env: {
+        ...process.env,
+        GITHUB_EVENT_NAME: eventName,
+        GITHUB_EVENT_PATH: eventPath,
+        GITHUB_OUTPUT: outputPath,
+      },
+    });
+    expect(result.error).toBeUndefined();
+    expect(result.status, result.stderr).toBe(0);
+    return readFileSync(outputPath, "utf8");
+  } finally {
+    rmSync(directory, { recursive: true, force: true });
+  }
+}
+
+test.each([
+  ["ordinary PR", "pull_request", { pull_request: { base: { ref: "main" } } }, true],
+  ["bottom layer", "pull_request", stacked("main", 1), true],
+  ["new bottom after a merge", "pull_request", stacked("main", 2), true],
+  ["upper layer", "pull_request", upper, false],
+  ["main push", "push", {}, true],
+  ["push with stack metadata", "push", upper, true],
+  ["merge queue group", "merge_group", upper, true],
+  ["missing PR", "pull_request", {}, true],
+  ["missing repository", "pull_request", { pull_request: upper.pull_request }, true],
+  ["empty repository", "pull_request", { ...upper, repository: { full_name: "" } }, true],
+  [
+    "missing stack base",
+    "pull_request",
+    { ...upper, pull_request: { ...upper.pull_request, stack: {} } },
+    true,
+  ],
+  [
+    "empty stack base",
+    "pull_request",
+    { ...upper, pull_request: { ...upper.pull_request, stack: { base: { ref: "" } } } },
+    true,
+  ],
+  [
+    "invalid stack base",
+    "pull_request",
+    { ...upper, pull_request: { ...upper.pull_request, stack: { base: { ref: 42 } } } },
+    true,
+  ],
+  [
+    "missing PR base",
+    "pull_request",
+    { ...upper, pull_request: { ...upper.pull_request, base: undefined } },
+    true,
+  ],
+  ["empty PR base", "pull_request", stacked(""), true],
+  [
+    "missing head repository",
+    "pull_request",
+    { ...upper, pull_request: { ...upper.pull_request, head: { repo: null } } },
+    true,
+  ],
+  [
+    "different base repository",
+    "pull_request",
+    {
+      ...upper,
+      pull_request: {
+        ...upper.pull_request,
+        base: { ref: "feature/lower", repo: { full_name: "someone/scriptc" } },
+      },
+    },
+    true,
+  ],
+])("CI selects the correct scope for %s", (_name, eventName, event, full) => {
+  expect(runScope(eventName as string, event)).toBe(`full=${full}\n`);
+});
+
+test.each(["OWNER", "MEMBER", "COLLABORATOR"])(
+  "only trusted same-repository %s stacks can defer full CI",
+  (association) => {
+    const event = {
+      ...upper,
+      pull_request: { ...upper.pull_request, author_association: association },
+    };
+    expect(runScope("pull_request", event)).toBe("full=false\n");
+    expect(
+      runScope("pull_request", {
+        ...event,
+        pull_request: {
+          ...event.pull_request,
+          head: { repo: { full_name: "someone/scriptc" } },
+        },
+      }),
+    ).toBe("full=true\n");
+  },
+);
+
+test.each(["CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "NONE", "unknown", undefined])(
+  "outside contributors with association %s retain full CI",
+  (association) => {
+    expect(
+      runScope("pull_request", {
+        ...upper,
+        pull_request: { ...upper.pull_request, author_association: association },
+      }),
+    ).toBe("full=true\n");
+  },
+);
+
+test("scope selection treats branch metadata as data, without executing it", () => {
+  expect(runScope("pull_request", stacked("feature/$(exit 1)\n'\"; exit 1"))).toBe("full=false\n");
+  expect(scopeScript).not.toContain("${{");
+  expect(scopeJob.permissions).toEqual({});
+  expect(scopeJob.steps).toHaveLength(1);
+  expect(scopeJob.steps![0]!.uses).toBeUndefined();
+  expect(scopeScript).not.toContain("scripts/ci-stack-scope");
+});
+
 test("every expensive CI job is gated and included in the required test check", () => {
   for (const name of heavyJobs) {
     const job = workflow.jobs[name]!;
@@ -45,7 +166,8 @@ test("every expensive CI job is gated and included in the required test check",
   expect(workflow.jobs.code_quality!.if).toBeUndefined();
   expect(workflow.jobs.test!.needs).toContain("validation_scope");
   expect(workflow.jobs.test!.needs).toContain("code_quality");
-  expect(workflow.jobs.test!.if).toBe("${{ !cancelled() }}");
+  // A skipped required job reports success, so aggregate even on cancellation.
+  expect(workflow.jobs.test!.if).toBe("${{ always() }}");
 });
 
 function runGate(full: string, results: Record<string, string> = {}) {
@@ -67,30 +189,30 @@ function runGate(full: string, results: Record<string, string> = {}) {
         field === "result" ? needs[name]!.result : needs[name]!.outputs.full,
     );
   expect(script).not.toContain("${{");
-  return spawnSync("bash", ["-e", "-c", script], { encoding: "utf8" }).status;
+  return spawnSync("bash", ["-e", "-o", "pipefail", "-c", script], { encoding: "utf8" });
 }
 
-test("the required test check accepts a complete full run or intentionally skipped upper-layer jobs", () => {
-  expect(runGate("true")).toBe(0);
-  expect(runGate("false")).toBe(0);
+test("the required test check passes only after full validation", () => {
+  expect(runGate("true").status).toBe(0);
+  const deferred = runGate("false");
+  expect(deferred.status).toBe(1);
+  expect(deferred.stdout).toContain("Merge the stack from the bottom up");
+  // Even a green top layer cannot make untested middle layers mergeable.
+  expect(
+    runGate("false", Object.fromEntries(heavyJobs.map((name) => [name, "success"]))).status,
+  ).toBe(1);
 });
 
 test("full validation rejects any failed, cancelled or skipped expensive job", () => {
   for (const job of heavyJobs)
     for (const result of ["failure", "cancelled", "skipped"])
-      expect(runGate("true", { [job]: result }), `${job}: ${result}`).not.toBe(0);
-});
-
-test("upper-layer validation cannot hide a failed or cancelled expensive job", () => {
-  for (const job of heavyJobs)
-    for (const result of ["failure", "cancelled"])
-      expect(runGate("false", { [job]: result }), `${job}: ${result}`).not.toBe(0);
+      expect(runGate("true", { [job]: result }).status, `${job}: ${result}`).toBe(1);
 });
 
 test("both scopes require successful planning and code quality", () => {
   for (const full of ["true", "false"])
     for (const job of ["validation_scope", "code_quality"])
       for (const result of ["failure", "cancelled", "skipped"])
-        expect(runGate(full, { [job]: result }), `${full}: ${job}: ${result}`).not.toBe(0);
-  for (const full of ["", "unknown"]) expect(runGate(full)).not.toBe(0);
+        expect(runGate(full, { [job]: result }).status, `${full}: ${job}: ${result}`).toBe(1);
+  for (const full of ["", "unknown"]) expect(runGate(full).status).toBe(1);
 });