refactor(api): unify test coverage under one Vitest projects config

.github/workflows/ci.yml

@@ -186,37 +186,17 @@ jobs:
         # Three legs that partition the suite into disjoint sets (check names
         # `Test (base)`, `Test (storage-azure)`, `Test (storage-minio)` are
         # required under branch protection):
-        #   - base:          the full suite EXCEPT the storage manifest
-        #                     (vitest.base.config.ts) — the bulk, run once.
+        #   - base:          the full suite EXCEPT the storage manifest — the
+        #                     bulk, run once.
         #   - storage-azure: the storage manifest against Azurite.
         #   - storage-minio: the storage manifest against MinIO.
-        # base ∪ storage-* == full suite; the storage manifest is the single
-        # source of truth for both the base `exclude` and the storage `include`.
-        # Each `test:storage-*` script sets STORAGE_PROVIDER itself, which picks
-        # the testcontainer in globalSetup.ts.
-        include:
-          - leg: base
-            test_script: test:base
-            coverage_artifact: coverage-report-base
-          - leg: storage-azure
-            test_script: test:storage-azure
-            coverage_artifact: coverage-report-storage-azure
-          - leg: storage-minio
-            test_script: test:storage-minio
-            coverage_artifact: coverage-report-storage-minio
-    env:
-      # Provider-specific vars below satisfy the conditional validation in
-      # apps/api/src/config/environment.ts. STORAGE_PROVIDER itself is set by
-      # each test script (azure_blob_storage for test:base and
-      # test:storage-azure, minio for test:storage-minio); base sets it so its
-      # app.ready() boot never depends on the storage container starting.
-      # Real connection details come from the testcontainer.
-      AZURE_STORAGE_ACCOUNT_NAME: devstoreaccount1
-      AZURE_STORAGE_CONTAINER_NAME: test-files
-      MINIO_ENDPOINT: http://localhost:9000
-      MINIO_ACCESS_KEY: minioadmin
-      MINIO_SECRET_KEY: minioadmin
-      MINIO_BUCKET: test-files
+        # `leg` is the Vitest project name in apps/api/vitest.config.ts
+        # (test.projects); base ∪ storage-* == full suite. The storage manifest
+        # is the single source of truth for both the base `exclude` and the
+        # storage `include`. Each project selects its provider (and the
+        # matching STORAGE_PROVIDER in its test.env) from its name in
+        # globalSetup.ts — no env is needed here.
+        leg: [base, storage-azure, storage-minio]
     # The storage legs are quick (18 files each); the base leg is the long pole.
     # Gating every step is where docs-only PRs save the most. All three legs
     # still report their required "Test (...)" checks.
@@ -242,25 +222,35 @@ jobs:
         if: needs.changes.outputs.code == 'true'
       - name: Run tests
         if: needs.changes.outputs.code == 'true'
-        # Indirection via env (never interpolate matrix values into `run:`
-        # directly — that is a template-injection finding for zizmor).
+        # Runs one Vitest project and emits a blob report (coverage embedded).
+        # LEG is passed via env (never interpolate matrix values into `run:`
+        # directly — that is a template-injection finding for zizmor); the blob
+        # filename embeds LEG so the three legs never collide in the coverage job.
         env:
-          TEST_SCRIPT: ${{ matrix.test_script }}
-        run: pnpm "$TEST_SCRIPT"
+          LEG: ${{ matrix.leg }}
+        run: pnpm test:ci
       - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         if: always() && needs.changes.outputs.code == 'true'
         with:
-          name: ${{ matrix.coverage_artifact }}
-          path: apps/api/coverage/
+          name: blob-report-${{ matrix.leg }}
+          path: apps/api/.vitest-reports/
+          # `.vitest-reports` is a dotfile dir; without this, upload-artifact
+          # (default include-hidden-files: false) skips its contents and the
+          # coverage job's download finds nothing.
+          include-hidden-files: true
           retention-days: 7
 
   # Enforces the apps/api coverage gate. Unlike the other heavy jobs, this one
-  # `needs: test` because it consumes the three legs' coverage artifacts — the
-  # legs partition the suite, so coverage must be merged across them before any
-  # threshold means anything (a single leg never sees the whole codebase). When
-  # a test leg fails there is nothing to gate and the PR is already blocked, so
-  # this job is simply skipped. On docs-only PRs every step is gated off (like
-  # the other jobs) and the required "Coverage" check still reports success.
+  # `needs: test` because it consumes the three legs' blob reports — the legs
+  # partition the suite, so coverage must be merged across them before any
+  # threshold means anything (a single leg never sees the whole codebase). Vitest
+  # merges the blobs natively (`--merge-reports`) and applies the gate
+  # (90% for lines, statements, functions, and branches, passed by flag in the
+  # test:coverage:merge script); there is no external merge script. When a test
+  # leg fails there is nothing to gate
+  # and the PR is already blocked, so this job is simply skipped. On docs-only PRs
+  # every step is gated off (like the other jobs) and the required "Coverage"
+  # check still reports success.
   coverage:
     needs: [check-draft, changes, test]
     name: Coverage
@@ -279,16 +269,29 @@ jobs:
           cache: pnpm
       - run: pnpm install --frozen-lockfile
         if: needs.changes.outputs.code == 'true'
-      # Pull every leg's coverage artifact (coverage-report-base,
-      # coverage-report-storage-azure, coverage-report-storage-minio) into a
-      # dir each, so check-coverage.mjs can merge them.
+      # Pull every leg's blob report into ONE flat dir. `merge-multiple: true` is
+      # required: `vitest --merge-reports` reads the dir non-recursively and
+      # rejects subfolders. Each leg's blob is uniquely named (blob-<leg>.json)
+      # so they never collide.
       - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
         if: needs.changes.outputs.code == 'true'
         with:
-          pattern: coverage-report-*
-          path: coverage-artifacts
-      - run: node scripts/check-coverage.mjs coverage-artifacts
-        if: needs.changes.outputs.code == 'true'
+          pattern: blob-report-*
+          path: apps/api/.vitest-reports
+          merge-multiple: true
+      # Native Vitest merge + coverage + gate (90% all metrics, replaces check-coverage.mjs).
+      - run: pnpm test:coverage:merge
+        if: needs.changes.outputs.code == 'true'
+      # Upload the human-readable merged report (html + lcov + json) this step
+      # already produces. `always()` so it is available precisely when the gate
+      # FAILS — that is when a contributor most needs to see which lines are
+      # missing, without having to fetch and merge the three opaque blobs by hand.
+      - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+        if: always() && needs.changes.outputs.code == 'true'
+        with:
+          name: coverage-report-merged
+          path: apps/api/coverage/
+          retention-days: 7
 
   build:
     needs: [check-draft, changes]

.gitignore

@@ -77,7 +77,7 @@ Thumbs.db
 
 # Test coverage
 coverage/
-coverage-artifacts/
+.vitest-reports/
 *.lcov
 .nyc_output/
 vitest-report/

apps/api/package.json

@@ -14,10 +14,9 @@
     "build": "tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json",
     "start": "node dist/server.js",
     "test": "vitest run",
-    "test:base": "STORAGE_PROVIDER=azure_blob_storage vitest run --config vitest.base.config.ts",
-    "test:storage-azure": "STORAGE_PROVIDER=azure_blob_storage vitest run --config vitest.storage.config.ts",
-    "test:storage-minio": "STORAGE_PROVIDER=minio vitest run --config vitest.storage.config.ts",
-    "test:coverage": "COVERAGE_DIR=coverage-artifacts/base pnpm run test:base && COVERAGE_DIR=coverage-artifacts/storage-azure pnpm run test:storage-azure && COVERAGE_DIR=coverage-artifacts/storage-minio pnpm run test:storage-minio && node ../../scripts/check-coverage.mjs coverage-artifacts",
+    "test:coverage": "vitest run --coverage",
+    "test:ci": ": \"${LEG:?LEG must be set (base|storage-azure|storage-minio)}\" && vitest run --project=\"$LEG\" --coverage --reporter=blob --outputFile.blob=.vitest-reports/blob-$LEG.json --coverage.thresholds.lines=0 --coverage.thresholds.statements=0 --coverage.thresholds.functions=0 --coverage.thresholds.branches=0",
+    "test:coverage:merge": "vitest run --merge-reports=.vitest-reports --coverage",
     "test:verify-storage-manifest": "tsx test/setup/assertStorageTestManifest.ts",
     "test:ui": "vitest --ui",
     "lint": "eslint .",

apps/api/test/factories/appFactory.ts

@@ -52,10 +52,41 @@ async function buildTestAdapter(
   }
 }
 
+/**
+ * Points the app's storage config at the running testcontainer BEFORE it boots.
+ *
+ * The MinIO endpoint is a dynamic testcontainer port, only known at runtime, so
+ * it cannot live in the static per-project `test.env` (which carries a localhost
+ * placeholder just to satisfy validation). We set it here, before `app.ready()`,
+ * so `buildStorageConfig()` — read by both the storage plugin and the
+ * storage-relay plugin at registration — sees the real endpoint. Azure needs
+ * nothing: its adapter uses the injected `connectionString`, and
+ * `buildStorageConfig()` only requires `AZURE_STORAGE_ACCOUNT_NAME`, which
+ * `test.env` already provides.
+ */
+function applyStorageEnvFromDescriptor(
+  descriptor: TestStorageDescriptor
+): void {
+  if (descriptor.provider === StorageProvider.MINIO) {
+    process.env.MINIO_ENDPOINT = descriptor.endpoint;
+    process.env.MINIO_ACCESS_KEY = descriptor.accessKey;
+    process.env.MINIO_SECRET_KEY = descriptor.secretKey;
+    process.env.MINIO_BUCKET = descriptor.bucket;
+    process.env.MINIO_REGION = descriptor.region;
+  }
+}
+
 export async function createTestApp(
   databaseUrl: string,
   options?: CreateTestAppOptions
 ): Promise<FastifyInstance> {
+  const descriptor = options?.storageDescriptor;
+
+  // Set the real testcontainer storage endpoint before the app boots, so the
+  // storage + relay plugins read it at `app.ready()` (see the helper's note).
+  // `null`/`undefined` = storage-agnostic test; the dummy `test.env` suffices.
+  if (descriptor) applyStorageEnvFromDescriptor(descriptor);
+
   const app = await createApp(false, { skipUnderPressure: true });
   app.log.level = "debug";
 
@@ -71,18 +102,17 @@ export async function createTestApp(
   // storagePlugin runs during ready() and would overwrite any earlier assignment.
   await app.ready();
 
-  const descriptor = options?.storageDescriptor;
-
   if (descriptor === null) {
-    // The test explicitly requested storage (`storageDescriptor:
-    // inject("storageDescriptor")`) but the storage testcontainer failed to
-    // start, so globalSetup provided `null`. Fail early with a clear reason
-    // instead of a confusing adapter error deeper in the test.
+    // globalSetup provides `null` only for the container-less `base` project, so
+    // reaching here means a `base` test requested storage (`storageDescriptor:
+    // inject("storageDescriptor")`). Such a test belongs in the storage manifest
+    // so it runs under a storage-* project. Fail early with a clear reason
+    // instead of a confusing adapter error deeper in the test. (A storage
+    // container that fails to start now fails its project fast at globalSetup.)
     throw new Error(
-      "createTestApp received `storageDescriptor: null` — the storage " +
-        "testcontainer failed to start (see the globalSetup warning above). " +
-        "This test requires real storage. Ensure Docker is available and the " +
-        "storage testcontainer starts successfully."
+      "createTestApp received `storageDescriptor: null`. Only the container-less " +
+        "`base` project provides null — a test that needs real storage must be " +
+        "listed in the storage manifest so it runs under a storage-* project."
     );
   }
 

apps/api/test/setup/globalSetup.ts

@@ -12,66 +12,61 @@ import type { TestProject } from "vitest/node";
 import { StorageProvider } from "@repo/storage";
 
 /**
- * Sets process.env for the chosen storage provider before workers are spawned,
- * so the storage plugin's `buildStorageConfig()` validation passes at
- * `app.ready()` in each worker.
+ * Maps a Vitest project name to the storage provider its tests run against.
  *
- * Connection details are dummies — real values are injected per-app in
- * `createTestApp`. The storagePlugin will still construct an adapter at boot
- * (its background health check will warn), but `createTestApp` then overrides
- * `app.storage` with the testcontainer-backed adapter.
+ * The provider selection travels via the project NAME (set in vitest.config.ts),
+ * not `process.env` — globalSetup runs once per project in the main process, so
+ * a shared `process.env.STORAGE_PROVIDER` would be last-writer-wins across the
+ * three projects when they run in a single command. Each project also declares
+ * the matching `STORAGE_PROVIDER` in its `test.env`, which Vitest applies inside
+ * the worker so `buildStorageConfig()` validation passes at `app.ready()`.
+ *
+ * `base` runs the storage-independent suite and boots no storage container.
  */
-function applyStorageEnv(descriptor: TestStorageDescriptor): void {
-  process.env.STORAGE_PROVIDER = descriptor.provider;
-  if (descriptor.provider === StorageProvider.AZURE_BLOB_STORAGE) {
-    process.env.AZURE_STORAGE_ACCOUNT_NAME ??= "devstoreaccount1";
-    process.env.AZURE_STORAGE_CONTAINER_NAME = descriptor.containerName;
-  }
-  if (descriptor.provider === StorageProvider.MINIO) {
-    process.env.MINIO_ENDPOINT = descriptor.endpoint;
-    process.env.MINIO_ACCESS_KEY = descriptor.accessKey;
-    process.env.MINIO_SECRET_KEY = descriptor.secretKey;
-    process.env.MINIO_BUCKET = descriptor.bucket;
-    process.env.MINIO_REGION = descriptor.region;
+function storageProviderForProject(
+  name: string | undefined
+): StorageProvider | null {
+  switch (name) {
+    case "storage-azure":
+      return StorageProvider.AZURE_BLOB_STORAGE;
+    case "storage-minio":
+      return StorageProvider.MINIO;
+    case "base":
+      return null;
+    default:
+      throw new Error(
+        `Unknown Vitest project "${String(name)}" — expected "base", ` +
+          `"storage-azure", or "storage-minio". globalSetup selects the storage ` +
+          `provider from the project name (see vitest.config.ts).`
+      );
   }
 }
 
 export default async function setup(project: TestProject) {
-  // The chatbot is opt-in (CHATBOT_ENABLED defaults off). Enable it for the
-  // suite so its routes register and the chatbot integration tests run
-  // (LLM_PROVIDER defaults to "mock"). Set before workers spawn so each worker
-  // inherits it — same mechanism as the storage env below.
-  process.env.CHATBOT_ENABLED = "true";
-
   // Database is required for all tests — let it propagate and fail fast.
   const { databaseUrl, container: dbContainer } = await setupTestDatabase();
 
-  // Storage is best-effort: only the storage-manifest tests (the storage-*
-  // legs) need the testcontainer. If it fails to start (wrong Node.js version,
-  // missing Docker image, CI network issue, etc.) we still want the
-  // storage-independent tests to run.
-  //
-  // This invariant holds because each test script sets STORAGE_PROVIDER itself
-  // (base leg = azure_blob_storage), so `buildStorageConfig()` at `app.ready()`
-  // clears its "STORAGE_PROVIDER is required" check without the container. The
-  // remaining provider-required var (AZURE_STORAGE_ACCOUNT_NAME) comes from the
-  // CI job env, so boot succeeds in CI even when the container is down. Locally
-  // that var is only injected by `applyStorageEnv` on the happy path, so a
-  // failed container there still breaks boot unless it is exported in the shell.
+  // Storage is per-project: only the storage projects boot a testcontainer, each
+  // against its own provider (picked from the project name, so the three
+  // projects never contend on a shared process.env). The `base` project skips it
+  // entirely and provides `null`. If the container fails to start in a storage
+  // project we fail fast — every file there needs it, so there is nothing to fall
+  // back to (unlike the old shared run, base is its own container-less project).
+  const provider = storageProviderForProject(project.name);
   let storageDescriptor: TestStorageDescriptor | null = null;
   let storageContainer: TestStorageContainer | null = null;
 
-  try {
-    const storage = await setupTestStorage();
-    storageDescriptor = storage.descriptor;
-    storageContainer = storage.container;
-    applyStorageEnv(storageDescriptor);
-  } catch (error) {
-    // eslint-disable-next-line no-console
-    console.warn(
-      "\n⚠️  Storage testcontainer failed to start — storage-dependent tests will fail.\n",
-      error
-    );
+  if (provider) {
+    try {
+      const storage = await setupTestStorage(provider);
+      storageDescriptor = storage.descriptor;
+      storageContainer = storage.container;
+    } catch (error) {
+      // Stop the DB container already started above, but never let its shutdown
+      // failure mask the real storage error.
+      await dbContainer.stop().catch(() => {});
+      throw error;
+    }
   }
 
   // Provide values BEFORE migrations/seeds so that an error in the next block
@@ -97,7 +92,10 @@ export default async function setup(project: TestProject) {
 declare module "vitest" {
   export interface ProvidedContext {
     databaseUrl: string;
-    /** `null` when the storage testcontainer failed to start. */
+    /**
+     * `null` only for the `base` project, which boots no storage container. The
+     * storage projects either provide a descriptor or fail fast at setup.
+     */
     storageDescriptor: TestStorageDescriptor | null;
   }
 }

apps/api/test/setup/storageTestManifest.ts

@@ -5,10 +5,10 @@
  *
  * Why this list exists:
  * - Both storage CI legs (storage-azure and storage-minio) run ONLY these files
- *   against their provider (see `vitest.storage.config.ts`); the base leg
- *   EXCLUDES them (it runs the full suite except this manifest). Together they
- *   prove the storage layer works against both providers without paying to run
- *   every test file twice.
+ *   against their provider (the `storage-*` projects in `vitest.config.ts`); the
+ *   base project EXCLUDES them (it runs the full suite except this manifest).
+ *   Together they prove the storage layer works against both providers without
+ *   paying to run every test file twice.
  * - `test:verify-storage-manifest` (test/setup/assertStorageTestManifest.ts)
  *   keeps this list honest: CI fails if a test touches storage but is missing
  *   here, if an entry no longer exists on disk, or if an entry no longer shows

apps/api/test/setup/testStorage.ts

@@ -109,24 +109,22 @@ async function setupMinioTestStorage(): Promise<{
 }
 
 /**
- * Starts the storage testcontainer matching `STORAGE_PROVIDER`.
- * Defaults to Azure Blob (Azurite) when the env var is unset, to preserve the
- * existing developer workflow.
+ * Starts the storage testcontainer for the given provider. The provider is
+ * chosen by the caller (globalSetup, from the Vitest project name) rather than
+ * read from `process.env`, so the three projects can boot different providers in
+ * a single run without contending on a shared env var.
  */
-export async function setupTestStorage(): Promise<{
+export async function setupTestStorage(provider: StorageProvider): Promise<{
   descriptor: TestStorageDescriptor;
   container: TestStorageContainer;
 }> {
-  const provider = (process.env.STORAGE_PROVIDER ??
-    StorageProvider.AZURE_BLOB_STORAGE) as StorageProvider;
-
   if (provider === StorageProvider.MINIO) {
     return setupMinioTestStorage();
   }
   if (provider === StorageProvider.AZURE_BLOB_STORAGE) {
     return setupAzureTestStorage();
   }
   throw new Error(
-    `Invalid STORAGE_PROVIDER for tests: "${String(provider)}". Expected ${Object.values(StorageProvider).join(" or ")}.`
+    `Invalid storage provider for tests: "${String(provider)}". Expected ${Object.values(StorageProvider).join(" or ")}.`
   );
 }