perf(ci): build artifact-mutating tests into per-file repo sandboxes

.github/workflows/validate.yml

@@ -624,31 +624,18 @@ jobs:
       - name: Prepare test reports dir
         run: mkdir -p reports/junit
 
-      # The suite runs in two non-overlapping passes (see vitest.config.ts's
-      # fileParallelism note). `test:ci` runs the parallelizable bulk — every
-      # file EXCEPT five that race each other (or the shared R2 staging tree)
-      # under parallel execution — concurrently WITH coverage;
-      # `test:ci:artifacts` (the last step below) then runs those five
-      # serially, after coverage is already captured + uploaded. Two
-      # (artifacts.test.ts, discovery-artifacts.test.ts) are
-      # filesystem-mutating artifact writers; two more (public-safety.test.ts,
-      # validate-error-messages.test.ts) each transiently write/mutate a real
-      # file validate-schemas.ts's own full-registry scan reads, which raced
-      # a concurrent registry scan in another test file (see
-      # public-safety.test.ts's header comment for the original incident
-      # writeup); refresh-build-summary.test.ts rewrites the real
-      # build-summary.json at the R2 staging root in place, racing the same
-      # staging tree the artifact writers above touch. All five drive their
-      # work via execFileSync child processes and add zero in-process
-      # coverage, so coverage comes only from `test:ci`.
+      # `test:ci` (scripts/run-ci-tests.ts) runs the whole suite in two vitest
+      # passes: the ~17 files that use vi.mock/vi.doMock/vi.unmock/
+      # vi.resetModules keep per-file isolation because they rewrite the module
+      # registry itself, and every other file runs with `--isolate=false`,
+      # sharing one registry per worker (#8922). Each pass writes its own lcov +
+      # junit -- neither is a valid coverage denominator alone, so BOTH are
+      # uploaded and Codecov merges them.
       #
-      # `test:ci` is itself two vitest passes (scripts/run-ci-tests.ts, #8922):
-      # the ~17 files that use vi.mock/vi.doMock/vi.unmock/vi.resetModules keep
-      # per-file isolation, and everything else shares one module registry per
-      # worker (`--isolate=false`), which cuts the run from ~499s to ~141s of
-      # user CPU. Each pass writes its own lcov + junit -- neither is a valid
-      # coverage denominator alone, so BOTH are uploaded and Codecov merges
-      # them.
+      # #8937 removed the former serial third pass: the filesystem-mutating
+      # tests now build into their own copy of the repo's data directories
+      # rather than the one shared artifact tree, so they parallelize with
+      # everything else.
       #
       # NOTE: the `test` job intentionally does NOT participate in the docs
       # fast lane above. Coverage is a repo-wide delta gate (codecov.yml), not
@@ -748,14 +735,6 @@ jobs:
           override_pr: ${{ github.event.pull_request.number }}
           fail_ci_if_error: false
 
-      # The two filesystem-mutating artifact writers, run serially LAST so they
-      # never overlap the parallel readers above and coverage is already
-      # uploaded. They mutate the same on-disk artifact tree, so they cannot run
-      # concurrently with each other either — the serial default applies. This is
-      # a pure correctness gate (no coverage); a failure here still fails the job.
-      - name: Test (artifact writers, serial)
-        run: npm run test:ci:artifacts
-
   # The Python SDK (python/) ships its own hermetic unittest suite (37 cases) but
   # was never run in CI — a regression in retry/backoff, Retry-After capping,
   # pagination, JSON-RPC unwrap, or the SSRF-safe redirect handler could land on a

package.json

@@ -135,7 +135,6 @@
     "test": "vitest run",
     "test:coverage": "vitest run --coverage",
     "test:ci": "node scripts/run-ci-tests.ts",
-    "test:ci:artifacts": "vitest run tests/artifacts.test tests/discovery-artifacts.test tests/public-safety.test tests/validate-error-messages.test tests/refresh-build-summary.test",
     "curation:brief": "node scripts/curation-brief.ts",
     "curation:stale-notes": "node scripts/stale-gap-notes.ts",
     "curation:identity-divergence": "node scripts/identity-field-divergence.ts",

scripts/lib.ts

@@ -27,7 +27,25 @@ type Row = Record<string, unknown>;
 // root is a valid native path on every OS. On Windows the bare `.pathname` form
 // yields a leading-slash, drive-prefixed string (e.g. `/E:/work/...`) that
 // `path.join` mangles into `E:\E:\work\...`, breaking every artifact read.
-export const repoRoot = fileURLToPath(new URL("..", import.meta.url));
+//
+// METAGRAPH_REPO_ROOT redirects every derived path — the 48 `public/**` write
+// sites across 23 scripts, both artifact roots below, and every consumer of
+// R2_STAGING_RELATIVE_ROOT — at the ONE place the root is computed. This is
+// what lets the filesystem-mutating tests each build into their own tree
+// instead of racing the single shared one (#8929): a test clones the repo's
+// data directories into a temp dir, points the real scripts at it, and asserts
+// there. Scripts still load from their real location, so only DATA is
+// redirected, never code.
+//
+// Deliberately not a general-purpose knob: unset (the normal case, including
+// all of CI's own build/publish steps) it resolves exactly as before.
+const defaultRepoRoot = fileURLToPath(new URL("..", import.meta.url));
+export const repoRoot = process.env.METAGRAPH_REPO_ROOT
+  ? // Trailing separator to match fileURLToPath's directory form, so the
+    // handful of `path.relative(repoRoot, …)` / string-prefix callers behave
+    // identically under both.
+    path.join(path.resolve(process.env.METAGRAPH_REPO_ROOT), path.sep)
+  : defaultRepoRoot;
 export const publicMetagraphRoot = path.join(repoRoot, "public/metagraph");
 export const r2StagingRoot = path.join(repoRoot, R2_STAGING_RELATIVE_ROOT);
 export const generatedSourceRoot = path.join(repoRoot, "dist/metagraph-source");

scripts/run-ci-tests.ts

@@ -28,30 +28,22 @@ import { repoRoot } from "./lib.ts";
 // NOT vitest `projects` (one invocation, auto-merged coverage), which looks
 // like the natural fit: the artifact-writer pass needs fileParallelism false
 // while these two need it true, which a single invocation cannot express, and
-// excluding the writers via project globs would make
-// `vitest run tests/artifacts.test` match nothing at all -- silently disabling
-// them. They stay a separate invocation (test:ci:artifacts), unchanged.
+// project globs would make `vitest run tests/artifacts.test` match nothing at
+// all -- silently disabling it.
+//
+// #8937 removed the third pass: the filesystem-mutating tests each build into
+// their own repo-data copy (tests/helpers/repo-sandbox.ts) instead of the one
+// shared tree, so they parallelize with everything else and test:ci:artifacts
+// is gone.
 
 /** Detects any API that manipulates the module registry for its own file. */
 const MODULE_MOCKING = /\bvi\.(mock|doMock|unmock|resetModules)\s*\(/;
 
-// The five filesystem-mutating files, run serially by test:ci:artifacts. Kept
-// here so both passes exclude them from one definition -- see vitest.config.ts
-// for why they cannot run alongside the readers.
-const ARTIFACT_WRITERS = [
-  "artifacts.test.ts",
-  "discovery-artifacts.test.ts",
-  "public-safety.test.ts",
-  "validate-error-messages.test.ts",
-  "refresh-build-summary.test.ts",
-];
-
 const testsDir = path.join(repoRoot, "tests");
 const testFiles = readdirSync(testsDir).filter((name) =>
   name.endsWith(".test.ts"),
 );
 const mockingFiles = testFiles
-  .filter((name) => !ARTIFACT_WRITERS.includes(name))
   .filter((name) =>
     MODULE_MOCKING.test(readFileSync(path.join(testsDir, name), "utf8")),
   )
@@ -120,13 +112,13 @@ function runPass(label: string, args: string[], suffix: string): void {
 
 // Pass 1 -- the bulk, one shared module registry per worker.
 runPass(
-  `shared registry (${testFiles.length - mockingFiles.length - ARTIFACT_WRITERS.length} files)`,
+  `shared registry (${testFiles.length - mockingFiles.length} files)`,
   [
     ...BASE,
     "--coverage",
     "--isolate=false",
     ...NO_PER_PASS_THRESHOLDS,
-    ...excludeArgs([...ARTIFACT_WRITERS, ...mockingFiles]),
+    ...excludeArgs(mockingFiles),
   ],
   "",
 );
@@ -148,5 +140,5 @@ runPass(
 
 console.log(
   `\nrun-ci-tests: both passes green (${mockingFiles.length} isolated, ` +
-    `${testFiles.length - mockingFiles.length - ARTIFACT_WRITERS.length} shared).`,
+    `${testFiles.length - mockingFiles.length} shared).`,
 );