Speed up PR CI critical path (#5147)

## Thinking Path > - Paperclip orchestrates AI agents for autonomous companies, so developer throughput on the control plane repo directly affects how fast the product can evolve. > - The PR workflow is part of that throughput surface because every change waits on it before review and merge. > - This branch started from measured evidence that the PR critical path was dominated by work that was either serialized unnecessarily or placed on the wrong part of the graph. > - The biggest concrete problems were: the canary dry run living inside `verify`, the server isolated suites running one-by-one in a single lane, and duplicate CI work that the PR path was paying for without increasing coverage proportionally. > - This pull request restructures the PR workflow so those costs are reduced without removing the important coverage that was already protecting release and test quality. > - Follow-up fixes on the branch hardened the new entrypoints so they work on clean GitHub runners and so the reduced PR typecheck path stays self-maintaining as workspace packages evolve. > - The benefit is materially faster PR wall-clock time while keeping canary packaging checks, serialized-suite isolation, plugin SDK consumers, and explicit TypeScript coverage where builds do not already provide it. ## What Changed - Moved the PR canary dry run into its own `Canary Dry Run` job so it still runs on PRs but no longer extends the `verify` critical path. - Split the custom Vitest runner into `general`, `serialized`, and `all` modes, and added shard support for the isolated server suites. - Added `test:run:general` and `test:run:serialized` scripts, then rewired PR CI to fan the serialized server suites out across a 4-way matrix. - Added the required `@paperclipai/plugin-sdk` build preflight before the new reduced-scope typecheck and test entrypoints so they succeed on clean CI runners. - Replaced the hardcoded PR build-gap list with `scripts/run-typecheck-build-gaps.mjs`, which discovers workspace packages whose `build` scripts skip TypeScript and runs only their explicit `typecheck` scripts. - Removed the redundant `pnpm build` from the PR `e2e` job because the Playwright onboarding path boots Paperclip from source. ## Verification - `ruby -e "require 'yaml'; YAML.load_file('.github/workflows/pr.yml'); puts 'workflow ok'"` - `node scripts/run-vitest-stable.mjs --mode general --dry-run` - `node scripts/run-vitest-stable.mjs --mode serialized --shard-index 0 --shard-count 4 --dry-run` - `pnpm run typecheck:build-gaps` - `pnpm test:run:general` - `pnpm test:run:serialized -- --shard-index 0 --shard-count 4` - `pnpm build` - `pnpm paperclipai onboard --yes --run` - `curl http://127.0.0.1:3299/api/health` ## Risks - Branch protection or required-check configuration may need to be updated for the new standalone `Canary Dry Run` job and the serialized-suite matrix job names. - `scripts/run-typecheck-build-gaps.mjs` assumes packages that need explicit PR-time typechecking are the ones whose `build` scripts omit `tsc`; if build conventions change, that heuristic needs to stay aligned. - Serialized test sharding preserves per-suite isolation, but the first few CI runs should still be watched for shard-balance or naming assumptions in downstream tooling. ## Model Used - OpenAI GPT-5.4 via the Codex local adapter, using high reasoning effort with shell, git, and file-edit tool use in a local worktree. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-05-03 20:20:14 -07:00
parent e01ffc18d3
commit 47920f9c47
4 changed files with 356 additions and 28 deletions
@@ -85,11 +85,11 @@ jobs:
      - name: Install dependencies
        run: pnpm install --frozen-lockfile

-      - name: Typecheck
-        run: pnpm -r typecheck
+      - name: Typecheck workspaces whose build scripts skip TypeScript
+        run: pnpm run typecheck:build-gaps

-      - name: Run tests
-        run: pnpm test:run
+      - name: Run general test suites
+        run: pnpm test:run:general

      - name: Verify release registry test coverage
        run: pnpm run test:release-registry
@@ -97,7 +97,76 @@ jobs:
      - name: Build
        run: pnpm build

-      - name: Release canary dry run
+  verify_serialized_server:
+    name: Verify serialized server suites (${{ matrix.shard_label }})
+    needs: [policy]
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - shard_index: 0
+            shard_count: 4
+            shard_label: 1/4
+          - shard_index: 1
+            shard_count: 4
+            shard_label: 2/4
+          - shard_index: 2
+            shard_count: 4
+            shard_label: 3/4
+          - shard_index: 3
+            shard_count: 4
+            shard_label: 4/4
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Setup pnpm
+        uses: pnpm/action-setup@v4
+        with:
+          version: 9.15.4
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 24
+          cache: pnpm
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      - name: Run serialized server test shard
+        run: pnpm test:run:serialized -- --shard-index ${{ matrix.shard_index }} --shard-count ${{ matrix.shard_count }}
+
+  canary_dry_run:
+    name: Canary Dry Run
+    needs: [policy]
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Setup pnpm
+        uses: pnpm/action-setup@v4
+        with:
+          version: 9.15.4
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 24
+          cache: pnpm
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      # `release.sh` always executes its Step 2/7 workspace build, even when
+      # `--skip-verify` bypasses the initial verification gate.
+      - name: Release canary dry run via release.sh internal build
        run: |
          git checkout -B master HEAD
          git checkout -- pnpm-lock.yaml
@@ -126,9 +195,6 @@ jobs:
      - name: Install dependencies
        run: pnpm install --frozen-lockfile

-      - name: Build
-        run: pnpm build
-
      - name: Install Playwright
        run: npx playwright install --with-deps chromium

@@ -15,9 +15,12 @@
    "build-storybook": "pnpm --filter @paperclipai/ui build-storybook",
    "build": "pnpm run preflight:workspace-links && pnpm -r build",
    "typecheck": "pnpm run preflight:workspace-links && pnpm -r typecheck",
+    "typecheck:build-gaps": "pnpm run preflight:workspace-links && node scripts/run-typecheck-build-gaps.mjs",
    "test": "pnpm run test:run",
    "test:watch": "pnpm run preflight:workspace-links && vitest",
    "test:run": "pnpm run preflight:workspace-links && node scripts/run-vitest-stable.mjs",
+    "test:run:general": "pnpm run preflight:workspace-links && pnpm --filter @paperclipai/plugin-sdk build && node scripts/run-vitest-stable.mjs --mode general",
+    "test:run:serialized": "pnpm run preflight:workspace-links && pnpm --filter @paperclipai/plugin-sdk build && node scripts/run-vitest-stable.mjs --mode serialized",
    "db:generate": "pnpm --filter @paperclipai/db generate",
    "db:migrate": "pnpm --filter @paperclipai/db migrate",
    "issue-references:backfill": "pnpm run preflight:workspace-links && tsx scripts/backfill-issue-reference-mentions.ts",
@@ -0,0 +1,93 @@
+#!/usr/bin/env node
+import { readFileSync } from "node:fs";
+import path from "node:path";
+import { spawnSync } from "node:child_process";
+
+const repoRoot = process.cwd();
+
+function fail(message) {
+  console.error(`[typecheck:build-gaps] ${message}`);
+  process.exit(1);
+}
+
+function run(command, args) {
+  const result = spawnSync(command, args, {
+    cwd: repoRoot,
+    stdio: "inherit",
+  });
+
+  if (result.error) {
+    console.error(`[typecheck:build-gaps] Failed to spawn ${command}: ${result.error.message}`);
+    process.exit(1);
+  }
+
+  if (result.status !== 0) {
+    process.exit(result.status ?? 1);
+  }
+}
+
+function readJson(filePath) {
+  return JSON.parse(readFileSync(filePath, "utf8"));
+}
+
+function listWorkspacePackages() {
+  const result = spawnSync("pnpm", ["ls", "-r", "--depth", "-1", "--json"], {
+    cwd: repoRoot,
+    encoding: "utf8",
+  });
+
+  if (result.error) {
+    fail(`Unable to spawn pnpm to list workspace packages: ${result.error.message}`);
+  }
+
+  if (result.status !== 0) {
+    fail("Unable to list pnpm workspace packages.");
+  }
+
+  return JSON.parse(result.stdout);
+}
+
+function buildSkipsTypeScript(pkg) {
+  const buildScript = pkg.scripts?.build;
+  if (typeof buildScript !== "string") {
+    return false;
+  }
+
+  return !/\btsc\b/.test(buildScript);
+}
+
+const workspacePackages = listWorkspacePackages();
+const buildGapCandidates = workspacePackages
+  .filter((workspacePkg) => workspacePkg.path !== repoRoot)
+  .map((workspacePkg) => ({
+    name: workspacePkg.name,
+    path: workspacePkg.path,
+    pkg: readJson(path.join(workspacePkg.path, "package.json")),
+  }))
+  .filter(({ pkg }) => buildSkipsTypeScript(pkg));
+const packagesMissingTypecheck = buildGapCandidates.filter(
+  ({ pkg }) => typeof pkg.scripts?.typecheck !== "string",
+);
+if (packagesMissingTypecheck.length > 0) {
+  const missingNames = packagesMissingTypecheck.map((workspacePkg) => workspacePkg.name).join(", ");
+  fail(
+    `Workspace packages with build scripts that skip tsc must define a typecheck script. Missing: ${missingNames}`,
+  );
+}
+const buildGapPackages = buildGapCandidates.filter(
+  ({ pkg }) => typeof pkg.scripts?.typecheck === "string",
+);
+
+console.log(
+  `[typecheck:build-gaps] typechecking ${buildGapPackages.length} workspace(s): ${buildGapPackages.map(({ name }) => name).join(", ") || "(none)"}`,
+);
+
+if (buildGapPackages.length === 0) {
+  process.exit(0);
+}
+
+run("pnpm", ["--filter", "@paperclipai/plugin-sdk", "build"]);
+
+for (const workspacePkg of buildGapPackages) {
+  run("pnpm", ["--filter", workspacePkg.name, "typecheck"]);
+}
@@ -45,6 +45,9 @@ const additionalSerializedServerTests = new Set([
  "server/src/__tests__/routines-e2e.test.ts",
 ]);
 let invocationIndex = 0;
+const serializedModeName = "serialized";
+const generalModeName = "general";
+const allModeName = "all";

 function walk(dir) {
  const entries = readdirSync(dir);
@@ -77,6 +80,130 @@ function isRouteOrAuthzTest(file) {
  return additionalSerializedServerTests.has(file);
 }

+function fail(message) {
+  console.error(`[test:run] ${message}`);
+  process.exit(1);
+}
+
+function readOptionValue(argv, index, argName) {
+  const value = argv[index + 1];
+  if (value === undefined) {
+    fail(`Missing value for ${argName}`);
+  }
+
+  return value;
+}
+
+function parseNonNegativeInteger(value, argName) {
+  const parsed = Number(value);
+  if (value.trim() === "" || !Number.isInteger(parsed) || parsed < 0) {
+    fail(`${argName} must be a non-negative integer. Received "${value}".`);
+  }
+
+  return parsed;
+}
+
+function parsePositiveInteger(value, argName) {
+  const parsed = Number(value);
+  if (value.trim() === "" || !Number.isInteger(parsed) || parsed < 1) {
+    fail(`${argName} must be a positive integer. Received "${value}".`);
+  }
+
+  return parsed;
+}
+
+function parseCliOptions(argv) {
+  let mode = allModeName;
+  let shardIndex = null;
+  let shardCount = null;
+  let dryRun = false;
+
+  for (let index = 0; index < argv.length; index += 1) {
+    const arg = argv[index];
+    if (arg === "--") {
+      continue;
+    }
+
+    if (arg === "--mode") {
+      mode = readOptionValue(argv, index, arg);
+      index += 1;
+      continue;
+    }
+
+    if (arg.startsWith("--mode=")) {
+      mode = arg.slice("--mode=".length);
+      continue;
+    }
+
+    if (arg === "--shard-index") {
+      shardIndex = parseNonNegativeInteger(readOptionValue(argv, index, arg), arg);
+      index += 1;
+      continue;
+    }
+
+    if (arg.startsWith("--shard-index=")) {
+      shardIndex = parseNonNegativeInteger(arg.slice("--shard-index=".length), "--shard-index");
+      continue;
+    }
+
+    if (arg === "--shard-count") {
+      shardCount = parsePositiveInteger(readOptionValue(argv, index, arg), arg);
+      index += 1;
+      continue;
+    }
+
+    if (arg.startsWith("--shard-count=")) {
+      shardCount = parsePositiveInteger(arg.slice("--shard-count=".length), "--shard-count");
+      continue;
+    }
+
+    if (arg === "--dry-run") {
+      dryRun = true;
+      continue;
+    }
+
+    fail(`Unknown argument "${arg}".`);
+  }
+
+  if (!new Set([allModeName, generalModeName, serializedModeName]).has(mode)) {
+    fail(`Unknown mode "${mode}". Expected one of: ${allModeName}, ${generalModeName}, ${serializedModeName}.`);
+  }
+
+  if ((shardIndex === null) !== (shardCount === null)) {
+    fail("--shard-index and --shard-count must be provided together.");
+  }
+
+  if (mode !== serializedModeName && shardIndex !== null) {
+    fail("--shard-index/--shard-count are only valid with --mode serialized.");
+  }
+
+  if (mode === serializedModeName) {
+    const resolvedShardCount = shardCount ?? 1;
+    const resolvedShardIndex = shardIndex ?? 0;
+    if (resolvedShardIndex >= resolvedShardCount) {
+      fail(`--shard-index must be less than --shard-count. Received ${resolvedShardIndex} of ${resolvedShardCount}.`);
+    }
+
+    return {
+      mode,
+      shardIndex: resolvedShardIndex,
+      shardCount: resolvedShardCount,
+      dryRun,
+    };
+  }
+
+  return {
+    mode,
+    shardIndex: null,
+    shardCount: null,
+    dryRun,
+  };
+}
+
+function selectSerializedSuites(routeTests, shardIndex, shardCount) {
+  return routeTests.filter((_, index) => index % shardCount === shardIndex);
+}
+
 function runVitest(args, label) {
  console.log(`\n[test:run] ${label}`);
  invocationIndex += 1;
@@ -103,25 +230,25 @@ function runVitest(args, label) {
  }
 }

-const routeTests = walk(serverTestsDir)
-  .filter((file) => isRouteOrAuthzTest(toRepoPath(file)))
-  .map((file) => ({
-    repoPath: toRepoPath(file),
-    serverPath: toServerPath(file),
-  }))
-  .sort((a, b) => a.repoPath.localeCompare(b.repoPath));
-
-const excludeRouteArgs = routeTests.flatMap((file) => ["--exclude", file.serverPath]);
-for (const project of nonServerProjects) {
+function runGeneralSuites(routeTests) {
+  const excludeRouteArgs = routeTests.flatMap((file) => ["--exclude", file.serverPath]);
+  for (const project of nonServerProjects) {
    runVitest(["--project", project], `non-server project ${project}`);
-}
+  }

-runVitest(
+  runVitest(
    ["--project", "@paperclipai/server", ...excludeRouteArgs],
    `server suites excluding ${routeTests.length} serialized suites`,
-);
+  );
+}

-for (const routeTest of routeTests) {
+function runSerializedSuites(routeTests, shardIndex, shardCount) {
+  const shardTests = selectSerializedSuites(routeTests, shardIndex, shardCount);
+  console.log(
+    `\n[test:run] serialized shard ${shardIndex + 1}/${shardCount} running ${shardTests.length} of ${routeTests.length} suites`,
+  );
+
+  for (const routeTest of shardTests) {
    runVitest(
      [
        "--project",
@@ -132,4 +259,43 @@ for (const routeTest of routeTests) {
      ],
      routeTest.repoPath,
    );
+  }
+}
+
+const routeTests = walk(serverTestsDir)
+  .filter((file) => isRouteOrAuthzTest(toRepoPath(file)))
+  .map((file) => ({
+    repoPath: toRepoPath(file),
+    serverPath: toServerPath(file),
+  }))
+  .sort((a, b) => a.repoPath.localeCompare(b.repoPath));
+
+const options = parseCliOptions(process.argv.slice(2));
+if (options.dryRun) {
+  const serializedSuites =
+    options.mode === serializedModeName
+      ? selectSerializedSuites(routeTests, options.shardIndex, options.shardCount)
+      : routeTests;
+  console.log(
+    JSON.stringify(
+      {
+        mode: options.mode,
+        shardIndex: options.shardIndex,
+        shardCount: options.shardCount,
+        serializedSuiteCount: routeTests.length,
+        selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath),
+      },
+      null,
+      2,
+    ),
+  );
+  process.exit(0);
+}
+
+if (options.mode === generalModeName || options.mode === allModeName) {
+  runGeneralSuites(routeTests);
+}
+
+if (options.mode === serializedModeName || options.mode === allModeName) {
+  runSerializedSuites(routeTests, options.shardIndex ?? 0, options.shardCount ?? 1);
 }