ci: run coverage in one job again (#2079)

The Coverage lane was split into two matrix shards plus a Coverage Report
job that downloaded both blob reports and merged them. That claimed three
runner slots per PR and put a barrier in front of the merge: the report
job could not start until the slower shard finished, and the blobs it
waited on are tens of MB to upload and download.

One job asks for one slot and reports its own thresholds where it runs, so
the lane finishes when the suite finishes. Everything the split needed goes
with it: the shard/merge switches in vitest.config.ts, the blob reporter
swap, the zeroed per-shard thresholds, and the env blanking that
`test:fuzz-worker` carried only to keep the second leg from inheriting them.

Author: thymikee

.github/workflows/ci.yml

@@ -278,22 +278,11 @@ jobs:
         run: node --experimental-strip-types scripts/check-package.ts
 
   # Runs the full unit + provider-integration suites under coverage with
-  # thresholds, so a separate unit-tests job would rerun the same tests. The
-  # suite is sharded across runners; each shard writes a blob report and the
-  # Coverage Report job merges them, evaluates thresholds once over the full
-  # suite, and produces every coverage artifact (vitest.config.ts carries the
-  # shard/merge switches).
+  # thresholds, so a separate unit-tests job would rerun the same tests.
   coverage:
     name: Coverage
     runs-on: ubuntu-latest
-    timeout-minutes: 15
-    strategy:
-      fail-fast: false
-      matrix:
-        shard: [1, 2]
-    env:
-      AGENT_DEVICE_COVERAGE_SHARD: ${{ matrix.shard }}/2
-      OUTPUT_ECONOMY_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
+    timeout-minutes: 30
     steps:
       - name: Checkout
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@@ -304,76 +293,31 @@ jobs:
         uses: ./.github/actions/setup-node-pnpm
 
       - name: Test changed-line coverage gate
-        if: matrix.shard == 1
         uses: ./.github/actions/run-gate
         with: { gate: coverage-model }
 
-      - name: Run coverage shard
+      - name: Run coverage
+        id: run-coverage
+        env:
+          OUTPUT_ECONOMY_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
         uses: ./.github/actions/run-gate
         with: { gate: unit-ci }
 
       # The TMPDIR redirection both test lanes depend on (#1593/#1595). The check is a real
       # package script that no workflow ran: it is reachable only through `check:unit`, an
-      # aggregate CI never invokes, so a leak regression could not fail a PR. Runs per shard,
-      # because a leak lands on whichever runner executed the leaking file.
+      # aggregate CI never invokes, so a leak regression could not fail a PR. Placed here
+      # because this is the lane whose instrumented suite would leak a run directory.
       - name: Check for leaked temp directories
         uses: ./.github/actions/run-gate
         with: { gate: tmpdir-leaks }
 
-      - name: Upload coverage blob
-        if: always()
-        uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
-        with:
-          name: coverage-blob-${{ matrix.shard }}
-          path: .vitest-reports/
-          # The directory is dot-prefixed, which v4 excludes by default.
-          include-hidden-files: true
-          if-no-files-found: error
-
-  coverage-report:
-    name: Coverage Report
-    needs: coverage
-    runs-on: ubuntu-latest
-    timeout-minutes: 10
-    env:
-      AGENT_DEVICE_COVERAGE_MERGE: '1'
-      OUTPUT_ECONOMY_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
-    steps:
-      - name: Checkout
-        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
-        with:
-          fetch-depth: 0
-
-      - name: Setup toolchain
-        uses: ./.github/actions/setup-node-pnpm
-
-      - name: Download coverage blobs
-        uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0
-        with:
-          pattern: coverage-blob-*
-          path: .tmp/coverage-blobs
-
-      # download-artifact nests each artifact in its own subdirectory; the blob
-      # merge reads one flat directory.
-      - name: Collect coverage blobs
-        run: |
-          set -euo pipefail
-          mkdir -p .vitest-reports
-          find .tmp/coverage-blobs -name '*.json' -exec mv {} .vitest-reports/ \;
-          ls .vitest-reports
-
-      # Reuses the blobs the shards wrote (never reruns tests) and fails when
-      # merged changed-line coverage < the threshold in
+      # Reuses the lcov the coverage step just wrote (never runs coverage twice)
+      # and fails when changed-line coverage < the threshold in
       # scripts/coverage-changed/model.ts. The `coverage-waiver` PR label maps to
       # the waiver env, which skips the failure but still prints the numbers.
-      # Gated on the merge step's own outcome (#1781 A5): when it fails,
-      # lcov.info is never written, so this step would just re-report that
+      # Gated on the coverage step's own outcome (#1781 A5): when `Run coverage`
+      # fails, lcov.info is never written, so this step would just re-report that
       # failure as its own red ("no lcov report") instead of a coverage verdict.
-      - name: Merge coverage shards
-        id: run-coverage
-        uses: ./.github/actions/run-gate
-        with: { gate: unit-ci }
-
       - name: Enforce changed-line coverage gate
         if: steps.run-coverage.outcome == 'success' && github.event_name == 'pull_request'
         env:

.gitignore

@@ -48,4 +48,3 @@ android/ime-helper/dist/
 # Workspace package declaration output (tsc -b project references)
 packages/*/dist-types/
 *.tsbuildinfo
-.vitest-reports/

package.json

@@ -177,7 +177,7 @@
     "test:maestro-compat": "vitest run --project unit-core packages/maestro src/daemon/adapters/maestro src/compat/__tests__/replay-input.test.ts",
     "test:coverage": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker",
     "test:coverage:ci": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker",
-    "test:fuzz-worker": "AGENT_DEVICE_COVERAGE_SHARD= AGENT_DEVICE_COVERAGE_MERGE= vitest run --project fuzz-worker",
+    "test:fuzz-worker": "vitest run --project fuzz-worker",
     "test:integration:provider": "vitest run --project provider-integration",
     "test:integration:progress": "node --experimental-strip-types scripts/integration-progress.ts",
     "test:integration:progress:check": "node --experimental-strip-types scripts/integration-progress.ts --check",

scripts/gate/declarations.ts

@@ -3,8 +3,8 @@
 // A script whose Vitest/node-test invocation the loader cannot read, mapped to the units it
 // really runs. Empty right now: the one entry was a coverage wrapper, and `test:coverage:ci`
 // is back to shapes the loader reads directly. It is no longer a single bare run — it is a
-// negated `--project` leg plus a nested script whose body carries an env prefix — but
-// scripts/gate/model.ts resolves both, so the units still come from the script itself.
+// negated `--project` leg plus a nested script — but scripts/gate/model.ts resolves both, so
+// the units still come from the script itself.
 export const OPAQUE_RUNNERS: Readonly<Record<string, readonly string[]>> = {};
 
 export const REPORTING_SCRIPTS: Readonly<Record<string, string>> = {

scripts/gate/model.test.ts

@@ -72,18 +72,17 @@ test('a negated --project subtracts from the configured set, so the skipped one
   );
 });
 
-// The real `test:coverage:ci` shape: the second leg is a nested script whose body carries an env
-// prefix (it blanks the coverage-shard switches). Both indirections have to survive, or the lane
-// stops owning the project it hands to that leg.
+// The real `test:coverage:ci` shape: a negated `--project` leg, then a second leg that is a
+// nested script. Both indirections have to survive, or the lane stops owning the project it
+// hands to that leg.
 test('the two halves of test:coverage:ci together still own every project', () => {
   assert.deepEqual(
     scriptUnits(
       'test:coverage:ci',
       scriptModel({
         'test:coverage:ci':
           'vitest run --coverage --project=!subprocess-stub && pnpm test:subprocess-stub',
-        'test:subprocess-stub':
-          'AGENT_DEVICE_COVERAGE_SHARD= AGENT_DEVICE_COVERAGE_MERGE= vitest run --project subprocess-stub',
+        'test:subprocess-stub': 'vitest run --project subprocess-stub',
       }),
     ),
     ['vitest:unit-core', 'vitest:subprocess-stub'],

vitest.config.ts

@@ -35,14 +35,6 @@ const SUBPROCESS_STUB_TESTS: readonly string[] = [
 // assumed: the cases execute out of process, which the fork's inspector session never
 // instruments, so this file reports the same lines with or without it.
 //
-// The second leg goes through `test:fuzz-worker`, which blanks AGENT_DEVICE_COVERAGE_SHARD and
-// AGENT_DEVICE_COVERAGE_MERGE — the sharding switches read just below. ci.yml sets them as
-// *job*-level env over a single `gate: unit-ci` step, so without the blanking both legs inherit
-// them and the shard dies: Vitest refuses `--shard=1/2` over a one-file project ("must be a
-// smaller than count of test files"), and the blob reporter overwrites the instrumented shard's
-// report on its way out, leaving the Coverage Report job nothing to merge. Measured, not
-// reasoned: the unguarded leg leaves a 1.4 kB blob holding only this project plus that error.
-//
 // Membership is by demonstrated failure, not by a property of the code. In particular it is
 // NOT "constructs a `node:worker_threads` Worker": `session-replay-runtime-maestro.test.ts`
 // does exactly that and stays in `unit-core`, instrumented and green. The proximate cause was
@@ -80,22 +72,8 @@ export const SETUP_FILES = [
   'src/__tests__/process-memo-setup.ts',
 ];
 
-// The CI Coverage lane shards the instrumented suite across runners and merges
-// the results on one of them (see ci.yml). AGENT_DEVICE_COVERAGE_SHARD="<i>/<n>"
-// turns an invocation into shard i of n writing a blob report; both unset means
-// the ordinary full run. AGENT_DEVICE_COVERAGE_MERGE=1 aggregates previously
-// written blobs instead of collecting tests — it still evaluates thresholds and
-// writes every configured coverage report.
-const COVERAGE_SHARD = process.env.AGENT_DEVICE_COVERAGE_SHARD;
-const COVERAGE_MERGE = process.env.AGENT_DEVICE_COVERAGE_MERGE === '1';
-
 export default defineConfig({
   test: {
-    ...(COVERAGE_SHARD ? { shard: COVERAGE_SHARD } : {}),
-    ...(COVERAGE_MERGE ? { mergeReports: '.vitest-reports' } : {}),
-    outputFile: COVERAGE_SHARD
-      ? { blob: `.vitest-reports/blob-${COVERAGE_SHARD.split('/')[0]}.json` }
-      : undefined,
     // Redirects TMPDIR to one per-run directory for the whole invocation (all
     // projects, every worker) and removes it once at the end — see the file
     // for why a single global hook beats per-file cleanup here.
@@ -117,9 +95,7 @@ export default defineConfig({
     // assumption without reducing ordinary file-level parallelism.
     maxConcurrency: 1,
     // Gate reporters for every lane; a `--reporter` flag would replace them, so no lane passes one.
-    // A coverage shard swaps in the blob reporter alongside the default one: its console output is
-    // only per-shard noise anyway, and the merge run below needs the blobs to exist.
-    reporters: COVERAGE_SHARD ? ['default', 'blob'] : ['default', slowTestGateReporter()],
+    reporters: ['default', slowTestGateReporter()],
     projects: [
       {
         test: {
@@ -251,9 +227,10 @@ export default defineConfig({
     coverage: {
       provider: 'v8',
       reporter: ['text', 'html', 'lcov', 'json-summary'],
-      // A shard sees roughly half the suite, so its own numbers sit far below
-      // the gate; thresholds are enforced once on the merged full-suite run.
-      thresholds: COVERAGE_SHARD ? { statements: 0, lines: 0 } : { statements: 78, lines: 80 },
+      thresholds: {
+        statements: 78,
+        lines: 80,
+      },
       include: ['src/**/*.ts', 'packages/*/src/**/*.ts'],
       exclude: [
         'src/**/*.test.ts',