Skip to content

Commit 8b0d753

Browse files
committed
fix(build): migrate loopover-miner/loopover-mcp to out-of-place dist/ emit
Completes the proper fix for the 2026-07-24 codecov/patch (0%/99%) incident. #8564 deleted in-place build output before the coverage run; #8568 replaced that with a vitest resolver plugin forcing in-process resolution to .ts regardless of a coexisting .js. Both were workarounds around the real root cause: packages/loopover-{miner,mcp} compiled in-place (tsconfig outDir "."), so compiled .js sat in the same directory as its .ts source -- the actual, recurring bug class (three prior incidents: stale .js shadowing fresh .ts, a no-op incremental build lying about freshness, then the coverage-attribution bug). This migrates both packages to out-of-place emit (outDir "dist"), matching how packages/loopover-engine already builds. .ts source and compiled .js output now live in physically separate directories, so there is never a same-directory collision for any resolver to work around -- the #8568 plugin is removed as dead code. Turbo caching fixed alongside the directory move (a real, deferred optimization surfaced by grep'ing this exact history): build:tsc's `cache: false` was explicit migration-era caution a prior PR (#7317) already flagged as safe to remove once bin/lib became fully compiler-owned; the dist/ split removes the original hazard entirely. Both packages' build tasks now declare real outputs (including .tsbuildinfo, so a cache HIT keeps tsc's own incremental state consistent with what's restored) instead of never caching at all. Every literal reference to the old in-place bin/lib layout is updated: the shared CLI test harnesses (which had to give up spawning bin/*.ts directly via --experimental-strip-types -- that only worked because Node's resolver found the entry's internal lib/*.js imports sitting right next to it; it does not fall back .js->.ts the way Vite/esbuild do), ~40 test files with hardcoded subprocess/readFileSync paths, 4 fixture scripts, the pack-check scripts' allowlists, check-syntax.mjs in both packages, the DEPLOYMENT.md audit script + its documented paths, and package.json's bin/files fields. Two production-source bugs found and fixed properly, not patched around: bin/loopover-mcp.ts and lib/status.ts both compute paths relative to their own file location (own package.json, CHANGELOG.md, the monorepo-sibling loopover-engine package) -- these files are ALSO imported in-process by tests that resolve against the real .ts source, so a single hardcoded relative depth cannot be correct for both contexts. Both now try the source-relative depth first, falling back to the dist-relative depth, so they work correctly however they're currently loaded. bin/loopover-mcp.ts's dynamic `import("@loopover/miner/lib/claim-ledger.js")` -- a published-package subpath import into miner's internals, invisible to any relative-path grep -- is fixed via an explicit `exports` map in miner's package.json instead of hardcoding "dist/" into mcp's own source, so the public contract between the two packages stays stable independent of miner's internal layout. package-lock.json's bin fields resynced (npm mirrors workspace packages' bin targets there); node_modules/.bin symlinks regenerated via a real `npm install` (--package-lock-only does not relink them). Docs: contributing-to-loopover's reference.md and SKILL.md corrected -- they previously said miner/mcp tests "run straight off the .ts" with no build needed, which is no longer true for the CLI-harness and stdio-transport-spawning tests now that dist/ is a separate directory. Validated: full unsharded suite (21,541/21,541 passing), typecheck clean, both real compiled CLIs execute and self-report their version correctly, turbo cache save+restore verified to actually restore dist/bin, dist/lib, and dist/package.json on a hit, and the monorepo package-check scripts pass against the real built workspace.
1 parent 626bc33 commit 8b0d753

80 files changed

Lines changed: 356 additions & 323 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/skills/contributing-to-loopover/SKILL.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,13 @@ This is where most PRs fail Codecov. The bar is **every changed line AND every c
191191
it is not wired into CI or the Codecov gate. Before pushing, run the whole suite
192192
**unsharded**`npm run test:coverage` — the only faithful local coverage signal (CI shards + merges,
193193
so a single shard under-reports).
194+
- **Touching `packages/loopover-{miner,mcp}`?** Run `npm run build:miner`/`build:mcp` once before
195+
scoping to an individual test file. Most miner/mcp tests import `lib/*.js` in-process (resolves
196+
straight to the `.ts`, no build needed) — but the shared CLI harnesses and every stdio-transport
197+
test (`mcp-cli-*.test.ts`, `mcp-feasibility-gate.test.ts`, etc.) spawn the real compiled
198+
`dist/bin/*.js`, and will fail with a confusing `ENOENT`/module-resolve error against a stale or
199+
missing `dist/` if you skip this. Re-run the build after editing miner/mcp source, same as you'd
200+
re-run any other build before testing its output.
194201
- **Find the uncovered branch.** In the v8 text report, read the **% Branch** column and the
195202
**Uncovered Line #s** for your changed file — a line at 100% lines but <100% branch has an un-taken
196203
`??`/ternary/`&&` side; add that case. Aim for **100% branch on your diff locally** so normal CI

.claude/skills/contributing-to-loopover/reference.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,16 @@ jobs run only if their path filter matched; on push to `main`, everything runs.
4242
| miner → pack | tarball hygiene | `npm run test:miner-pack` | unexpected/forbidden file in the miner npm tarball |
4343

4444
`packages/loopover-{miner,mcp}` are `.ts`-only in git: editing their `bin`/`lib` source means editing `.ts`
45-
and nothing else — the compiled `.js`/`.d.ts` these two `build` commands produce is gitignored, never
46-
committed, and never something a PR needs to touch. Tests (in-process imports and the CLI-harness
47-
subprocess spawns alike) run straight off the `.ts`; `build:mcp`/`build:miner` above exist only to
48-
validate the real publishable artifact still compiles and packs cleanly.
45+
and nothing else — the compiled `.js`/`.d.ts` these two `build` commands produce lands in a gitignored
46+
`dist/` (out-of-place, mirroring `packages/loopover-engine`'s own `src/``dist/` split), never committed,
47+
and never something a PR needs to touch. Tests that **import** miner/mcp `lib/*.js` in-process (the vast
48+
majority) resolve straight to the sibling `.ts` with no build needed. Tests that **spawn a real subprocess**
49+
— the shared CLI harnesses (`test/unit/support/{miner,mcp}-cli-harness.ts`) and every `mcp-cli-*.test.ts`/
50+
`mcp-feasibility-gate.test.ts`-style file connecting a `StdioClientTransport` — run the actual compiled
51+
`dist/bin/*.js`, so **`npm run build:mcp`/`build:miner` must run before `test:coverage`** for those to pass;
52+
`npm run test:ci` already sequences this correctly, so this only bites if you invoke `vitest`/`test:coverage`
53+
directly without it. `build:mcp`/`build:miner`'s pack-check role (validating the real publishable artifact
54+
still compiles and packs cleanly) is unchanged.
4955

5056
| rees → test | review-enrichment-service's own suite | `npm run rees:test` | any failing test under `review-enrichment/` |
5157
| ui → openapi drift | spec check | `npm run ui:openapi:check` | committed `openapi.json` is stale (run `npm run ui:openapi`) |

.github/workflows/ci.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -544,11 +544,11 @@ jobs:
544544
run: npm run test:mcp-pack
545545
# Same "ships .ts only, this build is for the published npm tarball + the pack-check below, not for
546546
# tests" story as MCP's own "Build MCP" step above. Invokes turbo.json's @loopover/miner#build:tsc
547-
# (cache: false -- historically to avoid a cache restore stomping this package's hand-written .js
548-
# files while the #7290 migration was still in-flight; now that #7317 closed it out, every bin/lib
549-
# file is compiler-owned and that original risk no longer applies, but re-enabling caching here is a
550-
# separate change this PR doesn't make) and @loopover/miner#build:verify (real caching win: skips
551-
# re-`node --check`-ing all 121 bin/lib files when neither changed) DIRECTLY as their own task names,
547+
# (real caching win since the 2026-07-24 dist/ migration -- previously cache: false, historically to
548+
# avoid a cache restore stomping this package's hand-written .js files while the #7290 migration was
549+
# still in-flight; #7317 closed that out, and the dist/ split removed the original in-place-emit
550+
# hazard entirely, so caching was finally enabled) and @loopover/miner#build:verify (real caching
551+
# win: skips re-`node --check`-ing all bin/lib files when neither changed) DIRECTLY as their own task names,
552552
# deliberately NOT via the aggregate `@loopover/miner#build` task (which exists for standalone/local
553553
# `npm run build` callers) -- that aggregate's own script is `npm run build:tsc && npm run
554554
# build:verify`, i.e. it re-invokes both of these exact scripts a second time even when turbo already

.gitignore

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
11
node_modules
22
dist/
33
dist-ssr/
4-
# packages/loopover-{miner,mcp} compile real TypeScript in place (tsc's outDir === rootDir) and never
5-
# commit the emitted output -- both ship as installable CLIs, so the .js still has to exist in the
6-
# published npm tarball (built fresh by `npm run build:{miner,mcp}` / the publish workflows), but nothing
7-
# else needs a prior build: Vite/esbuild resolve the .js-suffixed import specifiers these packages write
8-
# straight to the sibling .ts by default (same as packages/loopover-engine/src/**, which has never had a
9-
# committed .js either), and the CLI-harness tests spawn the .ts directly via Node's own type-stripping.
10-
packages/loopover-miner/bin/*.js
11-
packages/loopover-miner/bin/*.d.ts
12-
packages/loopover-miner/lib/*.js
13-
packages/loopover-miner/lib/*.d.ts
14-
packages/loopover-mcp/bin/*.js
15-
packages/loopover-mcp/lib/*.js
4+
# packages/loopover-{miner,mcp} compile real TypeScript out-of-place into dist/ (tsc's outDir "dist",
5+
# matching packages/loopover-engine's src/ -> dist/ split) and never commit the emitted output -- both
6+
# ship as installable CLIs, so the .js still has to exist in the published npm tarball (built fresh by
7+
# `npm run build:{miner,mcp}` / the publish workflows), but nothing else needs a prior build: Vite/esbuild
8+
# resolve the .js-suffixed import specifiers these packages write straight to the sibling .ts by default
9+
# (same as packages/loopover-engine/src/**, which has never had a committed .js either), and the
10+
# CLI-harness tests spawn the .ts directly via Node's own type-stripping. The generic "dist/" pattern
11+
# above already covers packages/loopover-{miner,mcp}/dist/** -- no package-specific line needed (this was
12+
# in-place emit with its own explicit ignore lines here until the 2026-07-24 migration).
1613
.turbo/
1714
.output
1815
.vinxi

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ WORKDIR /app
1313
# (workspaces: apps/*, packages/*), and `npm ci` only symlinks node_modules/<pkg> to a workspace whose
1414
# directory already exists on disk. Copying just the root package*.json first (the usual dependency-layer
1515
# caching trick) left every workspace package.json missing at `npm ci` time, so npm silently skipped every
16-
# internal symlink -- @loopover/engine (a workspace dependency of loopover-miner's checked-in
16+
# internal symlink -- @loopover/engine (a workspace dependency of loopover-miner's built
1717
# lib/*.js artifacts, #2281) then couldn't be resolved by esbuild no matter how/when its own dist/ was built.
1818
COPY . .
1919
# --ignore-scripts: no native builds are needed (SQLite is the built-in node:sqlite; @hono/node-server is

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/loopover-mcp/bin/loopover-mcp.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { createHash } from "node:crypto";
55
import { closeSync, constants as fsConstants, existsSync, fstatSync, mkdirSync, openSync, readdirSync, readFileSync, readSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
66
import { homedir } from "node:os";
77
import { delimiter, dirname, join } from "node:path";
8-
import { fileURLToPath } from "node:url";
8+
import { fileURLToPath, pathToFileURL } from "node:url";
99
import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js";
1010
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
1111
import { buildFeasibilityVerdict, buildPrTextLint, buildGateDispositions, buildPublicPrBodyDraft } from "@loopover/engine";
@@ -48,10 +48,25 @@ import { redactKnownLocalPaths, redactLocalPath } from "../lib/redact-local-path
4848
// side by side unaliased would read as the same function (#6238).
4949
import { recordMcpToolCall as recordLocalMcpToolCall } from "../lib/telemetry.js";
5050

51+
// Path to a file next to this package's own root, computed relative to THIS module's own on-disk
52+
// location -- which differs by one directory level depending on how this file is currently running:
53+
// imported in-process (e.g. by the vitest unit tests that exercise the CLI dispatcher + stdio tools
54+
// directly against bin/loopover-mcp.ts, per the invokedPath check below) resolves relative to the
55+
// real source bin/, while a real CLI invocation resolves relative to the compiled dist/bin/ (one
56+
// level deeper, since the 2026-07-24 dist/ migration; see tsconfig.json's outDir comment). A single
57+
// hardcoded relative depth can only ever be correct for one of those two contexts, so this tries
58+
// both, preferring whichever the current on-disk layout actually has.
59+
function resolveOwnPackageSiblingPath(...segments: string[]): URL {
60+
const here = dirname(fileURLToPath(import.meta.url));
61+
const fromBin = join(here, "..", ...segments);
62+
if (existsSync(fromBin)) return pathToFileURL(fromBin);
63+
return pathToFileURL(join(here, "..", "..", ...segments));
64+
}
65+
5166
// Read name/version from this package's own package.json (always present in any install --
5267
// global, npx, or local -- npm ships it regardless of the "files" allowlist) instead of hand-synced
5368
// literals, so a release bump never has a second place to forget.
54-
const ownPackageJson = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8"));
69+
const ownPackageJson = JSON.parse(readFileSync(resolveOwnPackageSiblingPath("package.json"), "utf8"));
5570

5671
const defaultApiUrl = "https://api.loopover.ai";
5772
const legacyDefaultApiUrls = new Set([
@@ -71,7 +86,7 @@ const decisionPackCacheSchemaVersion = 1;
7186
const decisionPackCacheMaxEntries = 25;
7287
const decisionPackCacheMaxBytes = 512 * 1024;
7388
const cliTextFileMaxBytes = 1024 * 1024;
74-
const changelogPath = new URL("../CHANGELOG.md", import.meta.url);
89+
const changelogPath = resolveOwnPackageSiblingPath("CHANGELOG.md");
7590
const cliArgs = process.argv.slice(2);
7691

7792
// #7764: true only when this file is the process entrypoint (`node .../loopover-mcp.js`, incl. via the npm

packages/loopover-mcp/package.json

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,12 @@
2626
"access": "public"
2727
},
2828
"bin": {
29-
"loopover-mcp": "bin/loopover-mcp.js"
29+
"loopover-mcp": "dist/bin/loopover-mcp.js"
3030
},
3131
"files": [
32-
"bin",
33-
"lib",
32+
"dist",
3433
"scripts",
3534
"CHANGELOG.md",
36-
"!bin/**/*.ts",
37-
"!lib/**/*.ts",
3835
"!scripts/check-syntax.mjs",
3936
"!scripts/strip-bin-sourcemap.mjs"
4037
],

packages/loopover-mcp/scripts/check-syntax.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ function listFiles(dir, extension) {
1919
.map((entry) => join(dir, entry.name));
2020
}
2121

22-
const files = [...listFiles("bin", ".js"), ...listFiles("lib", ".js"), ...listFiles("scripts", ".mjs")].sort();
22+
const files = [...listFiles("dist/bin", ".js"), ...listFiles("dist/lib", ".js"), ...listFiles("scripts", ".mjs")].sort();
2323

2424
const failures = [];
2525
for (const file of files) {
@@ -38,4 +38,4 @@ if (failures.length > 0) {
3838
process.exit(1);
3939
}
4040

41-
console.log(`node --check passed for all ${files.length} files in bin/, lib/, and scripts/.`);
41+
console.log(`node --check passed for all ${files.length} files in dist/bin/, dist/lib/, and scripts/.`);

packages/loopover-mcp/scripts/strip-bin-sourcemap.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#!/usr/bin/env node
2-
// Strip the trailing inline sourceMappingURL from compiled bin/*.js after tsc.
2+
// Strip the trailing inline sourceMappingURL from compiled dist/bin/*.js after tsc.
33
//
44
// The package tsconfig uses inlineSourceMap so small lib modules stay coverage-remapable without
5-
// publishing a new *.map file type. For bin/loopover-mcp.js (~6.5k lines) the inline map roughly
5+
// publishing a new *.map file type. For dist/bin/loopover-mcp.js (~6.5k lines) the inline map roughly
66
// doubles the shipped file past LoopOver's patch-less secrets-scan fetch cap (512KB), which made
77
// the prior Phase 3 attempt (#7431) fail closed. Bin is subprocess-only tested (mcp-cli harness),
88
// so v8 never remaps through this file anyway — stripping the map keeps the published/committed
@@ -12,7 +12,7 @@ import { dirname, join } from "node:path";
1212
import { fileURLToPath } from "node:url";
1313

1414
const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..");
15-
const BIN = join(ROOT, "bin");
15+
const BIN = join(ROOT, "dist/bin");
1616
const MARKER = "\n//# sourceMappingURL=";
1717

1818
let stripped = 0;
@@ -26,4 +26,4 @@ for (const entry of readdirSync(BIN, { withFileTypes: true })) {
2626
stripped += 1;
2727
}
2828

29-
console.log(`stripped inline sourcemaps from ${stripped} bin/*.js file(s)`);
29+
console.log(`stripped inline sourcemaps from ${stripped} dist/bin/*.js file(s)`);

0 commit comments

Comments
 (0)