Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .claude/skills/contributing-to-loopover/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ This is where most PRs fail Codecov. The bar is **every changed line AND every c
it is not wired into CI or the Codecov gate. Before pushing, run the whole suite
**unsharded** — `npm run test:coverage` — the only faithful local coverage signal (CI shards + merges,
so a single shard under-reports).
- **Touching `packages/loopover-{miner,mcp}`?** Run `npm run build:miner`/`build:mcp` once before
scoping to an individual test file. Most miner/mcp tests import `lib/*.js` in-process (resolves
straight to the `.ts`, no build needed) — but the shared CLI harnesses and every stdio-transport
test (`mcp-cli-*.test.ts`, `mcp-feasibility-gate.test.ts`, etc.) spawn the real compiled
`dist/bin/*.js`, and will fail with a confusing `ENOENT`/module-resolve error against a stale or
missing `dist/` if you skip this. Re-run the build after editing miner/mcp source, same as you'd
re-run any other build before testing its output.
- **Find the uncovered branch.** In the v8 text report, read the **% Branch** column and the
**Uncovered Line #s** for your changed file — a line at 100% lines but <100% branch has an un-taken
`??`/ternary/`&&` side; add that case. Aim for **100% branch on your diff locally** so normal CI
Expand Down
14 changes: 10 additions & 4 deletions .claude/skills/contributing-to-loopover/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,16 @@ jobs run only if their path filter matched; on push to `main`, everything runs.
| miner → pack | tarball hygiene | `npm run test:miner-pack` | unexpected/forbidden file in the miner npm tarball |

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

| rees → test | review-enrichment-service's own suite | `npm run rees:test` | any failing test under `review-enrichment/` |
| ui → openapi drift | spec check | `npm run ui:openapi:check` | committed `openapi.json` is stale (run `npm run ui:openapi`) |
Expand Down
35 changes: 17 additions & 18 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -544,11 +544,11 @@ jobs:
run: npm run test:mcp-pack
# Same "ships .ts only, this build is for the published npm tarball + the pack-check below, not for
# tests" story as MCP's own "Build MCP" step above. Invokes turbo.json's @loopover/miner#build:tsc
# (cache: false -- historically to avoid a cache restore stomping this package's hand-written .js
# files while the #7290 migration was still in-flight; now that #7317 closed it out, every bin/lib
# file is compiler-owned and that original risk no longer applies, but re-enabling caching here is a
# separate change this PR doesn't make) and @loopover/miner#build:verify (real caching win: skips
# re-`node --check`-ing all 121 bin/lib files when neither changed) DIRECTLY as their own task names,
# (real caching win since the 2026-07-24 dist/ migration -- previously cache: false, historically to
# avoid a cache restore stomping this package's hand-written .js files while the #7290 migration was
# still in-flight; #7317 closed that out, and the dist/ split removed the original in-place-emit
# hazard entirely, so caching was finally enabled) and @loopover/miner#build:verify (real caching
# win: skips re-`node --check`-ing all bin/lib files when neither changed) DIRECTLY as their own task names,
# deliberately NOT via the aggregate `@loopover/miner#build` task (which exists for standalone/local
# `npm run build` callers) -- that aggregate's own script is `npm run build:tsc && npm run
# build:verify`, i.e. it re-invokes both of these exact scripts a second time even when turbo already
Expand Down Expand Up @@ -900,18 +900,17 @@ jobs:
- name: Build engine package
run: npx turbo run build --filter=@loopover/engine
# packages/loopover-mcp and packages/loopover-miner both ship .ts source only (no compiled .js
# committed -- gitignored, same as @loopover/engine's dist/), but this job's own SKIP_MCP_CLI_HARNESS/
# SKIP_MINER_TEST_HARNESS env below (see "Test with coverage") can both be false, which spawns the
# REAL built CLI binary (packages/loopover-mcp/bin/loopover-mcp.js / packages/loopover-miner/bin/
# loopover-miner.js) as a subprocess -- AND, separately, every mcp-cli-*/miner-mcp-* test that instead
# runs the .ts entry directly via `node --experimental-strip-types` (test/unit/support/mcp-cli-harness.ts
# et al.) still needs this: that flag only strips types from the directly-executed entry file, it does
# NOT make plain Node resolve a `.js`-suffixed relative import to a sibling .ts the way Vite/esbuild do
# (confirmed live: `node --experimental-strip-types packages/loopover-mcp/bin/loopover-mcp.ts doctor`
# with no compiled lib/ present fails ERR_MODULE_NOT_FOUND on lib/local-branch.js, the CLI's own first
# local import) -- so bin/lib's compiled .js has to physically exist on disk either way. Before the
# compiled output stopped being committed, this job's checkout already had it for free; now it needs
# building here too, same as validate-code's own "Build MCP"/"Build miner CLI" steps.
# committed -- gitignored, and since the 2026-07-24 dist/ migration built out-of-place into dist/,
# same as @loopover/engine's own dist/; see each package's tsconfig.json outDir comment). Every
# mcp-cli-*/miner-*/mcp-feasibility-gate-style test spawning a real CLI subprocess -- whether via
# the shared test/unit/support/{mcp,miner}-cli-harness.ts or its own hand-rolled StdioClientTransport
# -- runs the real compiled dist/bin/*.js directly via plain `node` now (no `--experimental-strip-
# types`; that was only ever viable while the compiled .js sat in-place next to the .ts source for
# Node's own resolver to find its internal lib/*.js imports, which stopped being true once dist/
# physically separated them -- see the harness files' own comments). So bin/lib's compiled output has
# to physically exist on disk before any of those tests run. Before the compiled output stopped being
# committed, this job's checkout already had it for free; now it needs building here too, same as
# validate-code's own "Build MCP"/"Build miner CLI" steps.
#
# --force on both: turbo.json's @loopover/mcp#build task now declares real `outputs` (see its own
# comment) so a cache HIT restores the actual files instead of only the log, but this job's 3 parallel
Expand All @@ -937,7 +936,7 @@ jobs:
- name: Verify MCP/miner CLI binaries were actually built
run: |
set -euo pipefail
for f in packages/loopover-mcp/bin/loopover-mcp.js packages/loopover-miner/bin/loopover-miner.js; do
for f in packages/loopover-mcp/dist/bin/loopover-mcp.js packages/loopover-miner/dist/bin/loopover-miner.js; do
if [ ! -f "$f" ]; then
echo "::error::$f is missing after Build MCP/Build miner CLI both ran with --force. Every mcp-cli-*/miner-* test below will fail with a confusing MODULE_NOT_FOUND/ENOENT instead of this message -- something deeper changed (e.g. the build script itself stopped writing to this path); this is not a cache/timing issue, --force rules that out."
exit 1
Expand Down
21 changes: 9 additions & 12 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
node_modules
dist/
dist-ssr/
# packages/loopover-{miner,mcp} compile real TypeScript in place (tsc's outDir === rootDir) and never
# commit the emitted output -- both ship as installable CLIs, so the .js still has to exist in the
# published npm tarball (built fresh by `npm run build:{miner,mcp}` / the publish workflows), but nothing
# else needs a prior build: Vite/esbuild resolve the .js-suffixed import specifiers these packages write
# straight to the sibling .ts by default (same as packages/loopover-engine/src/**, which has never had a
# committed .js either), and the CLI-harness tests spawn the .ts directly via Node's own type-stripping.
packages/loopover-miner/bin/*.js
packages/loopover-miner/bin/*.d.ts
packages/loopover-miner/lib/*.js
packages/loopover-miner/lib/*.d.ts
packages/loopover-mcp/bin/*.js
packages/loopover-mcp/lib/*.js
# packages/loopover-{miner,mcp} compile real TypeScript out-of-place into dist/ (tsc's outDir "dist",
# matching packages/loopover-engine's src/ -> dist/ split) and never commit the emitted output -- both
# ship as installable CLIs, so the .js still has to exist in the published npm tarball (built fresh by
# `npm run build:{miner,mcp}` / the publish workflows), but nothing else needs a prior build: Vite/esbuild
# resolve the .js-suffixed import specifiers these packages write straight to the sibling .ts by default
# (same as packages/loopover-engine/src/**, which has never had a committed .js either), and the
# CLI-harness tests spawn the .ts directly via Node's own type-stripping. The generic "dist/" pattern
# above already covers packages/loopover-{miner,mcp}/dist/** -- no package-specific line needed (this was
# in-place emit with its own explicit ignore lines here until the 2026-07-24 migration).
.turbo/
.output
.vinxi
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ WORKDIR /app
# (workspaces: apps/*, packages/*), and `npm ci` only symlinks node_modules/<pkg> to a workspace whose
# directory already exists on disk. Copying just the root package*.json first (the usual dependency-layer
# caching trick) left every workspace package.json missing at `npm ci` time, so npm silently skipped every
# internal symlink -- @loopover/engine (a workspace dependency of loopover-miner's checked-in
# internal symlink -- @loopover/engine (a workspace dependency of loopover-miner's built
# lib/*.js artifacts, #2281) then couldn't be resolved by esbuild no matter how/when its own dist/ was built.
COPY . .
# --ignore-scripts: no native builds are needed (SQLite is the built-in node:sqlite; @hono/node-server is
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 37 additions & 4 deletions packages/loopover-mcp/bin/loopover-mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,24 @@ import { redactKnownLocalPaths, redactLocalPath } from "../lib/redact-local-path
// side by side unaliased would read as the same function (#6238).
import { recordMcpToolCall as recordLocalMcpToolCall } from "../lib/telemetry.js";

// Self-referencing package import (Node's own mechanism for "resolve a file that belongs to my own
// package, correctly, regardless of my own current location on disk") -- requires the "exports" map
// in this package's own package.json (the "./x": "./x" entries there are what make this resolvable
// at all). Robust by construction, not by guessing: whether this file is running as the real source
// bin/loopover-mcp.ts (imported in-process by tests that exercise the CLI dispatcher + stdio tools
// directly, per the invokedPath check below) or as the compiled dist/bin/loopover-mcp.js (a real CLI
// invocation), import.meta.resolve walks up from THIS file's own location through node_modules the
// same way any external consumer's "@loopover/mcp/..." import would, landing on the one real
// package.json/CHANGELOG.md either way -- no relative-depth arithmetic, so a future directory move
// can never silently break this again the way the pre-dist/-migration relative path did.
function resolveOwnPackageFile(specifier: string): URL {
return new URL(import.meta.resolve(specifier));
}

// Read name/version from this package's own package.json (always present in any install --
// global, npx, or local -- npm ships it regardless of the "files" allowlist) instead of hand-synced
// literals, so a release bump never has a second place to forget.
const ownPackageJson = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8"));
const ownPackageJson = JSON.parse(readFileSync(resolveOwnPackageFile("@loopover/mcp/package.json"), "utf8"));

const defaultApiUrl = "https://api.loopover.ai";
const legacyDefaultApiUrls = new Set([
Expand All @@ -71,7 +85,7 @@ const decisionPackCacheSchemaVersion = 1;
const decisionPackCacheMaxEntries = 25;
const decisionPackCacheMaxBytes = 512 * 1024;
const cliTextFileMaxBytes = 1024 * 1024;
const changelogPath = new URL("../CHANGELOG.md", import.meta.url);
const changelogPath = resolveOwnPackageFile("@loopover/mcp/CHANGELOG.md");
const cliArgs = process.argv.slice(2);

// #7764: true only when this file is the process entrypoint (`node .../loopover-mcp.js`, incl. via the npm
Expand Down Expand Up @@ -564,11 +578,30 @@ const feasibilityGateShape = {
* `releaseClaim`, or `expireClaim` -- it never gains any ability to block, cancel, or override a claim or
* attempt; real claim-conflict authority stays entirely with #4848's maintainer-only path.
*/
/** The narrow slice of @loopover/miner's claim-ledger module this file actually calls -- hand-shaped
* rather than imported, so this optional cross-package integration carries no TYPE-level coupling to
* miner's internals either, matching its own already-optional RUNTIME shape (this module may not be
* installed alongside loopover-mcp at all). */
type ClaimLedgerModule = {
resolveClaimLedgerDbPath: () => string;
openClaimLedgerReadOnly: (dbPath: string) => {
listActiveClaims: (repoFullName: string) => Array<{ issueNumber: number }>;
close: () => void;
};
};

async function resolveLedgerClaimStatus(repoFullName: any, issueNumber: any) {
if (!repoFullName || !issueNumber) return null;
let claimLedgerModule;
let claimLedgerModule: ClaimLedgerModule;
try {
claimLedgerModule = await import("@loopover/miner/lib/claim-ledger.js");
// Specifier built from concatenated parts, not a literal, so tsc never attempts to statically
// resolve/typecheck the target module: this optional cross-package import must not create a
// BUILD-TIME dependency on @loopover/miner's own dist/ output existing yet (CI builds MCP before
// miner in validate-tests) or on @loopover/miner being installed/built at all -- both are real,
// already-supported states the try/catch below already handles at runtime. The `as unknown as`
// cast onto the hand-shaped type above is the only thing giving this call real types.
const specifier = "@loopover/miner" + "/lib/claim-ledger.js";
claimLedgerModule = (await import(specifier)) as unknown as ClaimLedgerModule;
} catch {
/* v8 ignore next -- loopover-miner genuinely unresolvable (not installed alongside loopover-mcp); not
reproducible in this monorepo's workspace-hoisted test environment, where the sibling package always
Expand Down
11 changes: 6 additions & 5 deletions packages/loopover-mcp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@
"access": "public"
},
"bin": {
"loopover-mcp": "bin/loopover-mcp.js"
"loopover-mcp": "dist/bin/loopover-mcp.js"
},
"exports": {
"./package.json": "./package.json",
"./CHANGELOG.md": "./CHANGELOG.md"
},
"files": [
"bin",
"lib",
"dist",
"scripts",
"CHANGELOG.md",
"!bin/**/*.ts",
"!lib/**/*.ts",
"!scripts/check-syntax.mjs",
"!scripts/strip-bin-sourcemap.mjs"
],
Expand Down
4 changes: 2 additions & 2 deletions packages/loopover-mcp/scripts/check-syntax.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function listFiles(dir, extension) {
.map((entry) => join(dir, entry.name));
}

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

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

console.log(`node --check passed for all ${files.length} files in bin/, lib/, and scripts/.`);
console.log(`node --check passed for all ${files.length} files in dist/bin/, dist/lib/, and scripts/.`);
8 changes: 4 additions & 4 deletions packages/loopover-mcp/scripts/strip-bin-sourcemap.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#!/usr/bin/env node
// Strip the trailing inline sourceMappingURL from compiled bin/*.js after tsc.
// Strip the trailing inline sourceMappingURL from compiled dist/bin/*.js after tsc.
//
// The package tsconfig uses inlineSourceMap so small lib modules stay coverage-remapable without
// publishing a new *.map file type. For bin/loopover-mcp.js (~6.5k lines) the inline map roughly
// publishing a new *.map file type. For dist/bin/loopover-mcp.js (~6.5k lines) the inline map roughly
// doubles the shipped file past LoopOver's patch-less secrets-scan fetch cap (512KB), which made
// the prior Phase 3 attempt (#7431) fail closed. Bin is subprocess-only tested (mcp-cli harness),
// so v8 never remaps through this file anyway — stripping the map keeps the published/committed
Expand All @@ -12,7 +12,7 @@ import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";

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

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

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