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
6 changes: 3 additions & 3 deletions .claude/skills/contributing-to-loopover/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ npm audit --audit-level=moderate # the dependency-review job's local eq

`npm run test:ci` runs, and must pass, **all of**: `actionlint`, `db:migrations:check`,
`db:schema-drift:check`, `selfhost:env-reference:check`, `selfhost:validate-observability`,
`cf-typegen:check`, `typecheck`, `test:coverage`, `test:engine-parity`, `test:live-gate-parity`, `test:driver-parity`, the
`@loopover/engine` workspace's own test run, `test:workers`, `build:mcp`, `test:mcp-pack`,
`build:miner`, `test:miner-pack`, `rees:test`, `ui:openapi:check`, `ui:openapi:settings-parity`,
`cf-typegen:check`, `build:mcp:check`, `build:miner:check`, `typecheck`, `test:coverage`, `test:engine-parity`, `test:live-gate-parity`, `test:driver-parity`, the
`@loopover/engine` workspace's own test run, `test:workers`, `test:mcp-pack`,
`test:miner-pack`, `rees:test`, `ui:openapi:check`, `ui:openapi:settings-parity`,
`ui:version-audit`, `docs:drift-check`, `manifest:drift-check`, `engine-parity:drift-check`,
`command-reference:check`, `ui:lint`, `ui:typecheck`, `ui:test`, `ui:build`. If any step fails, fix it
and re-run — do not push a red tree. (Full per-check table in `reference.md`; check `package.json`'s
Expand Down
30 changes: 26 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -470,13 +470,29 @@ jobs:
- name: Build MCP
if: ${{ github.event_name == 'push' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' }}
run: npx turbo run build --filter=@loopover/mcp
# packages/loopover-mcp/{bin,lib} compile real TypeScript in place (tsc's outDir === rootDir, so
# e.g. lib/foo.ts emits lib/foo.js right next to it) and commit that emitted output -- the package
# ships as an installable CLI (npm install -g), so a runnable .js has to exist in the published
# tarball, and test/**/*.test.ts imports the emitted .js by its literal specifier, not the .ts
# source. "Build MCP" above (this job) and the "Test with coverage" job below run as separate GitHub
# Actions jobs with independent checkouts, so nothing in this job's build step can ever self-heal a
# stale commit before the coverage job's tests run against it -- a contributor who edits .ts and
# forgets to rebuild+commit the .js gets tests silently exercising old behavior with no CI signal,
# same drift-class gap the Selfhost/Miner env-reference checks above were added to close. This
# fails loudly instead: the build one step up already regenerated bin/lib in place, so any git
# status output here means the committed .js/.d.ts didn't match what that build just produced.
- name: MCP build drift check
if: ${{ github.event_name == 'push' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' }}
run: node scripts/check-build-drift.mjs mcp
- name: MCP package check
if: ${{ github.event_name == 'push' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' }}
run: npm run test:mcp-pack
# Invokes turbo.json's @loopover/miner#build:tsc (cache: false, since its committed lib/**/*.js
# output can't be safely restored without risking a stomp of hand-written files) 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, deliberately NOT via the aggregate
# 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, 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 ran them as cached prerequisite tasks
Expand All @@ -488,6 +504,12 @@ jobs:
- name: Build miner CLI
if: ${{ github.event_name == 'push' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.engine == 'true' }}
run: npx turbo run build:tsc build:verify --filter=@loopover/miner
# Same "separate job, no self-heal" drift-class gap as MCP's own build drift check above, and the
# ORIGINAL motivating case for it: packages/loopover-miner/lib/cross-repo-evaluation.{ts,js,d.ts}
# are real files a contributor edits directly, not obscure internals.
- name: Miner build drift check
if: ${{ github.event_name == 'push' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.engine == 'true' }}
run: node scripts/check-build-drift.mjs miner
# loopover-miner depends on @loopover/engine for real too (same relationship as loopover-mcp above
# -- packages/loopover-miner/package.json lists it as a dependency), so this and "Build miner CLI"
# above now also trigger on an engine-only PR, matching "Build MCP"/"MCP package check"'s existing
Expand Down
10 changes: 7 additions & 3 deletions .github/workflows/publish-miner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,13 @@ jobs:
- name: Install dependencies
run: npm ci

# Miner ships hand-written, checked-in JS (no tsc build) -- "build" here is node --check syntax
# validation over every bin/lib file, the same script test:ci already runs on every PR.
- name: Miner syntax validation
# Real tsc build (in-place emit -- see packages/loopover-miner/tsconfig.json), same as ci.yml's own
# "Build miner CLI" step, followed by node --check syntax validation over every bin/lib file (the
# same two-part script test:ci already runs on every PR). Re-running the compile here means this
# job always packs from a freshly-built tree even if the tagged commit's committed .js/.d.ts had
# somehow drifted from its .ts source (ci.yml's build-drift check on every PR is what keeps that
# from happening in the first place).
- name: Miner build + syntax validation
run: npm run build --workspace @loopover/miner

# Reuses the exact allowlist/required-files/forbidden-content check test:ci already runs on
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
"db:migrate:remote": "wrangler d1 migrations apply loopover --remote",
"drizzle:generate": "drizzle-kit generate",
"build:mcp": "npm --workspace @loopover/mcp run build",
"build:mcp:check": "npm run build:mcp && node scripts/check-build-drift.mjs mcp",
"build:miner": "turbo run build --filter=@loopover/engine && npm --workspace @loopover/miner run build",
"build:miner:check": "npm run build:miner && node scripts/check-build-drift.mjs miner",
"test:mcp-pack": "node scripts/check-mcp-package.mjs",
"test:miner-pack": "node scripts/check-miner-package.mjs",
"test:miner-deployment-docs-audit": "node scripts/check-miner-deployment-docs.mjs",
Expand Down Expand Up @@ -109,7 +111,7 @@
"test:smoke:browser:install": "playwright install chromium",
"test:smoke:browser": "node scripts/smoke-ui-browser.mjs",
"pretest:ci": "npm run check-node-version",
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run build:mcp && npm run test:mcp-pack && npm run build:miner && npm run test:miner-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
"test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build:mcp:check && npm run build:miner:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
"test:release": "npm run test:ci && npm run changelog:check",
"test:release:mcp": "npm run test:ci",
"test:watch": "vitest",
Expand Down
6 changes: 3 additions & 3 deletions packages/loopover-mcp/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
// repo root and corrupt each other's incremental state.
"tsBuildInfoFile": "./.tsbuildinfo"
},
// Only files already converted to real TypeScript are included -- everything else in bin/lib stays
// plain, hand-maintained .js until its own migration phase lands (#7291). No edits needed here as
// later phases convert more files: the glob picks them up automatically.
// Every bin/lib runtime module is real TypeScript (#7291, phased across #7328/#7329/#7330): tsc owns
// the in-place .js emit. The glob stays the include surface; test/unit/mcp-typescript-migration-complete.test.ts
// fails closed if a hand-maintained .js orphan ever reappears.
"include": ["bin/**/*.ts", "lib/**/*.ts"],
// Without this, tsc's default exclude list (which always adds outDir) resolves to "." -- the whole
// package root -- and silently excludes every include match, since outDir is "." for in-place emit.
Expand Down
10 changes: 9 additions & 1 deletion packages/loopover-miner/lib/cross-repo-evaluation.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,15 @@ export type EvaluateRepoExecutionOptions = EvaluateRepoReadinessOptions & {
commandTimeoutMs?: number;
};
/** Copy the benchmark clone into a discardable temp tree — the agent and the repo's test suite only ever touch
* the copy, so the clone stays pristine and cleanup is a single recursive remove. */
* the copy, so the clone stays pristine and cleanup is a single recursive remove. realpathSync's the fresh
* mkdtemp directory immediately: on macOS, os.tmpdir() resolves under a symlink (/var/folders/... ->
* /private/var/folders/...), so without this, the raw mkdtempSync path and coding-task-spec.ts's own
* realpathSync(workingDirectory) (a deliberate containment-check canonicalization in writeAcceptanceCriteriaFile,
* not something to remove) disagree on which string names the same directory -- acceptanceCriteriaPath then
* fails a plain acceptanceCriteriaPath.startsWith(workingDirectory) check even though the file genuinely is
* inside the working directory. Resolving once here, at the source, keeps every downstream path (workspace.path,
* task.workingDirectory, the acceptance-criteria path) in the same canonical form with no further changes
* needed. A no-op on Linux CI runners, where /tmp is not itself a symlink. */
export declare function defaultPrepareExecutionWorkspace(repoPath: string): CrossRepoExecutionWorkspace;
/** Command runner for the stack's inferred build/test commands. detectRepoStack only ever emits simple
* `tool subcommand` forms ("npm test", "cargo build", "npm run build"), so the command is tokenized on
Expand Down
16 changes: 12 additions & 4 deletions packages/loopover-miner/lib/cross-repo-evaluation.js

Large diffs are not rendered by default.

14 changes: 11 additions & 3 deletions packages/loopover-miner/lib/cross-repo-evaluation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// readiness gaps, leaked loopover assumptions in agent instructions, clone/setup problems, or other.

import { spawn as nodeSpawn } from "node:child_process";
import { cpSync, existsSync, mkdtempSync, rmSync } from "node:fs";
import { cpSync, existsSync, mkdtempSync, realpathSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
// Already a transitive dependency via coding-task-spec.js's own engine imports -- this adds no load weight.
Expand Down Expand Up @@ -553,9 +553,17 @@ export type EvaluateRepoExecutionOptions = EvaluateRepoReadinessOptions & {
};

/** Copy the benchmark clone into a discardable temp tree — the agent and the repo's test suite only ever touch
* the copy, so the clone stays pristine and cleanup is a single recursive remove. */
* the copy, so the clone stays pristine and cleanup is a single recursive remove. realpathSync's the fresh
* mkdtemp directory immediately: on macOS, os.tmpdir() resolves under a symlink (/var/folders/... ->
* /private/var/folders/...), so without this, the raw mkdtempSync path and coding-task-spec.ts's own
* realpathSync(workingDirectory) (a deliberate containment-check canonicalization in writeAcceptanceCriteriaFile,
* not something to remove) disagree on which string names the same directory -- acceptanceCriteriaPath then
* fails a plain acceptanceCriteriaPath.startsWith(workingDirectory) check even though the file genuinely is
* inside the working directory. Resolving once here, at the source, keeps every downstream path (workspace.path,
* task.workingDirectory, the acceptance-criteria path) in the same canonical form with no further changes
* needed. A no-op on Linux CI runners, where /tmp is not itself a symlink. */
export function defaultPrepareExecutionWorkspace(repoPath: string): CrossRepoExecutionWorkspace {
const scratchRoot = mkdtempSync(join(tmpdir(), "loopover-cross-repo-exec-"));
const scratchRoot = realpathSync(mkdtempSync(join(tmpdir(), "loopover-cross-repo-exec-")));
const path = join(scratchRoot, "repo");
cpSync(repoPath, path, { recursive: true });
return {
Expand Down
8 changes: 8 additions & 0 deletions scripts/check-build-drift.d.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export function pathsForPackage(name: string): string[];

export function checkBuildDrift(
paths: string[],
options?: { cwd?: string; run?: (paths: string[], cwd: string) => string },
): string;

export function main(argv: string[]): void;
62 changes: 62 additions & 0 deletions scripts/check-build-drift.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#!/usr/bin/env node
// packages/loopover-{miner,mcp} both compile real TypeScript in place (tsc's outDir === rootDir, so
// e.g. lib/foo.ts emits lib/foo.js + lib/foo.d.ts right next to it) and commit that emitted output --
// both packages ship as installable CLIs (npm install -g), so a runnable .js has to exist in the
// published tarball, and root-level tests import the emitted .js by its literal specifier (NodeNext
// resolution), not the .ts source, so a stale commit makes test:coverage silently exercise old
// behavior instead of loudly failing. package.json composes this AFTER the package's real build
// (`npm run build:{miner,mcp} && node scripts/check-build-drift.mjs {miner,mcp}`) -- this script's own
// job is just: did that build change anything relative to what's committed. Mirrors cf-typegen:check's
// "regenerate for real, then diff" shape for a package whose generated output is emitted in place
// across a whole directory rather than to one named file.
import { execFileSync } from "node:child_process";
import { fileURLToPath } from "node:url";

const PACKAGE_PATHS = {
miner: ["packages/loopover-miner/bin", "packages/loopover-miner/lib"],
mcp: ["packages/loopover-mcp/bin", "packages/loopover-mcp/lib"],
};

/** The bin/lib directories a package name checks. Exported (rather than inlined into main()) so a test
* can validate the real PACKAGE_PATHS table itself -- e.g. that "miner" resolves to real repo paths --
* not just a fake stand-in for it. Throws on an unknown name instead of silently checking nothing. */
export function pathsForPackage(name) {
const paths = PACKAGE_PATHS[name];
if (!paths) {
throw new Error(`check-build-drift: unknown package "${name}" (expected one of: ${Object.keys(PACKAGE_PATHS).join(", ")})`);
}
return paths;
}

function defaultGitStatus(paths, cwd) {
return execFileSync("git", ["status", "--porcelain", "--", ...paths], { cwd, encoding: "utf8" });
}

/** Trimmed `git status --porcelain` text for `paths` under `cwd` (a real git worktree) -- "" when
* clean. Porcelain (not plain `git diff --exit-code`) deliberately: a .ts file whose emitted .js/.d.ts
* was never committed at all shows up as an untracked file, which `git diff` alone never reports.
* `run` is injectable so tests can fake the git call entirely; the default shells out for real. */
export function checkBuildDrift(paths, { cwd = process.cwd(), run = defaultGitStatus } = {}) {
return run(paths, cwd).trim();
}

export function main(argv) {
try {
const name = argv[0];
const paths = pathsForPackage(name);
const drift = checkBuildDrift(paths);
if (drift.length > 0) {
process.stderr.write(
`check-build-drift: packages/loopover-${name}'s committed .js/.d.ts is stale relative to its .ts source -- run \`npm run build:${name}\` and commit the result:\n${drift}\n`,
);
process.exit(1);
return;
}
process.stdout.write(`check-build-drift: packages/loopover-${name}'s committed .js/.d.ts matches its .ts source.\n`);
} catch (err) {
process.stderr.write(`${err instanceof Error ? err.message : String(err)}\n`);
process.exit(1);
}
}

if (process.argv[1] === fileURLToPath(import.meta.url)) main(process.argv.slice(2));
Loading
Loading