Skip to content

Commit c4a6426

Browse files
authored
fix(test): resolve in-place package emit to .ts source in vitest, restoring both coverage attribution and subprocess artifacts (#8568)
Second, complete fix for the 2026-07-24 codecov/patch (0%/99%) incident. The first fix (#8564) deleted the miner/MCP in-place build output before the coverage run, which restored .ts coverage attribution and --changed tracing -- but broke the other class of tests: subprocess-spawning ones (the CLI harnesses and MCP stdio tests) run under plain Node outside Vite's resolver and genuinely need the built lib/bin .js at spawn time (node's ESM resolver does not fall back from a .js specifier to a .ts sibling). Scoped post-fix runs failed in miner-discover-cli/--help, miner-init-wizard e2e, and all 14 mcp-feasibility-gate stdio tests. This replaces that approach: a resolve-stage vitest plugin rewrites relative .js-suffixed imports that land inside packages/loopover-{miner,mcp}/{lib,bin} to their .ts sibling when it exists, so in-process imports always carry the source identity (correct v8 attribution + import-graph tracing) while the built .js stays on disk for spawned subprocesses. The #8564 ci.yml surgery (pre-coverage clean, unconditional check-miner-package exclusion, trailing pack-integrity step) is reverted -- the workflow returns to the #8514 shape, with the plugin making it correct.
1 parent bd273c1 commit c4a6426

2 files changed

Lines changed: 31 additions & 39 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -949,26 +949,6 @@ jobs:
949949
with:
950950
path: .turbo/cache
951951
key: turbo-tests-${{ hashFiles('package-lock.json') }}-${{ github.run_id }}
952-
# The miner/MCP packages compile IN PLACE (tsconfig outDir "." -- lib/foo.ts emits lib/foo.js next to
953-
# its source, gitignored). The two --force builds above therefore leave compiled .js siblings for every
954-
# .ts under packages/loopover-{miner,mcp}/{lib,bin} -- and once those exist, Vite resolves the packages'
955-
# NodeNext .js-suffixed import specifiers to the REAL compiled files instead of falling back to the .ts
956-
# source (the fallback vitest.config.ts's coverage globs document depending on). That breaks the
957-
# coverage run twice over: vitest --changed's import-graph tracing can't relate a changed .ts to the
958-
# tests importing it (the module graph holds the .js identity), and executed-module coverage is
959-
# attributed to the .js ids, flatlining every changed .ts to 0% -- the 2026-07-24 "(0%/99%)
960-
# codecov/patch on every packages/** PR" incident, introduced when the 3-shard consolidation moved
961-
# these builds into the same job as the coverage run (the shards never built these packages).
962-
# Reproduced/bisected locally on vitest 4.1.9: with lib/*.js present the scoped run selected 1 test
963-
# file and reported the changed attempt-cli.ts at 0%; after this clean it selected 4 and reported 100%.
964-
# Nothing in the coverage run needs the built output at runtime: both CLI harnesses spawn the .ts
965-
# entrypoints via --experimental-strip-types (test/unit/support/{mcp,miner}-cli-harness.ts), and the
966-
# one artifact-dependent test (check-miner-package.test.ts's npm-pack dry-run) is excluded from the
967-
# coverage run below and re-run after a rebuild in "Pack-integrity test" at the end of this job.
968-
# -fdX removes ONLY gitignored files, so tracked sources are untouchable by construction; the binary
969-
# verification above already ran, and "Save Turborepo cache" already captured the build outputs.
970-
- name: Remove in-place package build output before coverage
971-
run: git clean -fdX packages/loopover-miner packages/loopover-mcp
972952
- name: Prepare test reports dir
973953
run: mkdir -p reports/junit
974954
- name: Test with coverage
@@ -1032,16 +1012,10 @@ jobs:
10321012
--exclude "test/unit/mcp-discovery.test.ts"
10331013
)
10341014
fi
1035-
# check-miner-package.test.ts is ALWAYS excluded from the coverage run -- its npm-pack dry-run
1036-
# needs the in-place build output that "Remove in-place package build output before coverage"
1037-
# just deleted (see that step's comment for why the emit must not exist here). It runs in
1038-
# "Pack-integrity test" at the end of this job instead, after a rebuild, with coverage disabled --
1039-
# it only spawns scripts/check-miner-package.ts as a subprocess, so it never contributed
1040-
# v8-visible coverage in this run anyway.
1041-
EXCLUDE_ARGS+=(--exclude "test/unit/check-miner-package.test.ts")
10421015
if [ "$SKIP_MINER_TEST_HARNESS" = "true" ]; then
10431016
echo "Skipping the self-contained miner-package tests (no packages/loopover-miner-relevant paths changed)."
10441017
EXCLUDE_ARGS+=(
1018+
--exclude "test/unit/check-miner-package.test.ts"
10451019
--exclude "test/unit/miner-calibration-types.test.ts"
10461020
)
10471021
fi
@@ -1176,18 +1150,6 @@ jobs:
11761150
override_commit: ${{ github.event.pull_request.head.sha }}
11771151
override_pr: ${{ github.event.pull_request.number }}
11781152
fail_ci_if_error: false
1179-
# Rebuild the in-place emit (deleted before the coverage run -- see "Remove in-place package build
1180-
# output before coverage") and run the one test that genuinely needs it: check-miner-package.test.ts's
1181-
# npm-pack dry-run over the built package layout. Coverage stays disabled -- the test only spawns
1182-
# scripts/check-miner-package.ts as a subprocess, so excluding it from the coverage run loses nothing.
1183-
# Gated exactly like its old in-suite inclusion: every push, or a PR whose changed paths matched the
1184-
# minerTestHarness filter (the same condition SKIP_MINER_TEST_HARNESS negates above). The implicit
1185-
# success() on this if means it never runs after a failed suite, matching the old in-suite behavior.
1186-
- name: Pack-integrity test (rebuilds in-place emit)
1187-
if: ${{ github.event_name == 'push' || needs.changes.outputs.minerTestHarness == 'true' }}
1188-
run: |
1189-
npx turbo run build:tsc build:verify --filter=@loopover/miner --force
1190-
npx vitest run --coverage.enabled=false test/unit/check-miner-package.test.ts
11911153

11921154
# Single required status check. Branch protection points at "validate"; this
11931155
# aggregates the path-aware jobs (the PR-only dependency review gate lives inside

vitest.config.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,38 @@
1+
import { existsSync } from "node:fs";
2+
import { dirname, isAbsolute, resolve as resolvePath } from "node:path";
13
import { defineConfig } from "vitest/config";
24

35
const junitPath = process.env.VITEST_JUNIT_PATH;
46

7+
// In-process imports must resolve the miner/mcp packages' NodeNext ".js"-suffixed specifiers to the
8+
// .ts SOURCE even when the in-place compiled .js exists on disk (both packages emit next to their
9+
// sources -- tsconfig outDir "."; CI builds them before the coverage run, and local checkouts
10+
// accumulate the same gitignored emit). Without this, Vite resolves the literal .js file, v8
11+
// coverage attributes every hit to the built-file identity (flatlining the .ts to 0% -- the
12+
// 2026-07-24 codecov/patch (0%/99%) incident), and --changed's import-graph tracing can't relate a
13+
// changed .ts to the tests importing it. Deleting the emit before the coverage run instead (the
14+
// first attempt at fixing that incident) broke the OTHER class of tests: subprocess-spawning ones
15+
// (the CLI harnesses and the MCP stdio tests) run under plain Node outside Vite's resolver and
16+
// genuinely need the built .js at spawn time. This plugin serves both: in-process resolution always
17+
// lands on the .ts (correct identity for coverage + tracing), while spawned subprocesses keep the
18+
// built artifacts on disk. Scoped to exactly the two in-place-emit packages -- engine emits to
19+
// dist/ and must keep resolving through its package boundary unchanged.
20+
const IN_PLACE_EMIT_RE = /packages\/loopover-(?:miner|mcp)\/(?:lib|bin)\/.*\.js$/;
21+
const preferTsSourceForInPlaceEmit = {
22+
name: "loopover:prefer-ts-source-for-in-place-emit",
23+
enforce: "pre" as const,
24+
resolveId(source: string, importer: string | undefined) {
25+
if (!importer || !source.endsWith(".js")) return null;
26+
if (!source.startsWith("./") && !source.startsWith("../") && !isAbsolute(source)) return null;
27+
const candidate = isAbsolute(source) ? source : resolvePath(dirname(importer), source);
28+
if (!IN_PLACE_EMIT_RE.test(candidate)) return null;
29+
const tsSibling = `${candidate.slice(0, -3)}.ts`;
30+
return existsSync(tsSibling) ? tsSibling : null;
31+
},
32+
};
33+
534
export default defineConfig({
35+
plugins: [preferTsSourceForInPlaceEmit],
636
ssr: {
737
noExternal: ["agents", "partyserver"],
838
},

0 commit comments

Comments
 (0)