ci(build): catch stale committed miner/mcp compiled output before merge - #7690
Merged
Conversation
packages/loopover-miner and packages/loopover-mcp compile real TypeScript in place and commit the emitted .js/.d.ts (both ship as installable CLIs, so a runnable .js has to exist without a build step). Nothing enforced that the committed output actually matched a fresh build: root-level tests import the emitted .js by its literal specifier, and the job that builds these packages runs as a separate GitHub Actions job from the one that tests them, so a contributor who edited .ts without rebuilding got tests silently running old behavior with zero CI signal -- the same drift class the Selfhost/Miner env-reference checks were previously added to close. Adds scripts/check-build-drift.mjs (git-status-based, so a compiled file that was never committed at all is caught too, not just a stale one), wires it into ci.yml right after each package's real build step, and into package.json's local test:ci for pre-push parity. Also fixes three comments left stale by the #7290/#7291 TypeScript migrations completing, and adds loopover-mcp's missing migration-complete guard test (mirroring loopover-miner's existing one).
…path defaultPrepareExecutionWorkspace built its scratch directory straight from mkdtempSync, but coding-task-spec.ts's writeAcceptanceCriteriaFile separately realpath's the working directory before writing into it (a deliberate containment-check canonicalization, not something to remove). On macOS, os.tmpdir() resolves under a symlink (/var/folders/... -> /private/var/folders/...), so the two disagreed on which string names the same directory: a plain acceptanceCriteriaPath.startsWith(workingDirectory) check failed even though the file genuinely was inside the working directory. Resolving once at the source, right after mkdtempSync, keeps every downstream path in the same canonical form with no other changes needed, and is a no-op on Linux CI runners where /tmp isn't itself a symlink. Discovered while validating the build-drift check against #7668's own newly-merged full-execution mode.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | d8998e7 | Commit Preview URL Branch Preview URL |
Jul 21 2026, 08:30 AM |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7690 +/- ##
=======================================
Coverage 88.55% 88.55%
=======================================
Files 725 725
Lines 76188 76188
Branches 22679 22679
=======================================
Hits 67466 67466
Misses 7680 7680
Partials 1042 1042
Flags with carried forward coverage won't be shown. Click here to find out more.
|
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id.
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id.
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id.
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id.
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id.
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
* build(mcp,miner): stop committing compiled .js/.d.ts entirely The #7690 drift-check made staleness loud, but contributors still had to generate and commit compiled output for every .ts change -- churn that made no sense once the #7290/#7291 migrations finished making these two packages real TypeScript. Both were the only packages in the repo using in-place tsc emit; everything else already builds to a gitignored dist/. Turns out nothing forced that: Vite/esbuild and Wrangler's own production bundler already resolve a .js-suffixed relative import specifier to its sibling .ts by default when no literal .js exists on disk -- the same behavior packages/loopover-engine/src/** has always silently relied on (it has never had a single committed .js). Verified against the real production bundler, not just tests: `wrangler deploy --dry-run` bundles cleanly with zero compiled output present anywhere, confirming src/mcp/find-opportunities.ts's direct import of packages/loopover-miner/lib/opportunity-fanout.ts resolves fine at deploy time too. That leaves exactly one place still needing a real file: the two CLI-harness test suites that spawn the binary as an actual OS subprocess (not a Vite-mediated import). Those now spawn the .ts directly through Node's own --experimental-strip-types instead of requiring a prior `tsc` build -- explicit rather than relying on its default-on state, and faster than routing through tsx (measured). So: .gitignore both packages' bin/lib compiled output, git rm --cached the ~250 files that were tracked, and remove everything #7690 added that's now pointless (check-build-drift.mjs, its test, the build:{mcp,miner}:check scripts, the two migration-complete guard tests -- there's nothing left to drift-check once nothing is committed). build:mcp/build:miner still exist and still run in CI -- they're what validates the actual publishable artifact still compiles and packs, same as before. One real bug found along the way: vitest.config.ts's coverage.include still needs BOTH the .js and .ts glob per file, not just .ts. @vitest/coverage-v8 tracks an executed module under the id Vite resolved it FROM (the requested .js specifier) rather than the .ts file actually read off disk -- confirmed by removing the .js entry once and watching genuinely-tested files (5 passing tests, real function calls) report a flat 0% because coverage.include no longer matched their reported id. * build(scripts): convert every remaining .mjs/.d.mts pair to real .ts Same motivation as the prior commit, extended to scripts/: a hand-maintained .d.mts alongside a .mjs is exactly the duplicated-declaration problem the TypeScript migration was supposed to eliminate, and it can silently drift from the real implementation with no compiler ever checking it. Converts all 39 remaining scripts/*.mjs files that had one -- the mcp-release/ orb-release family, every docs/settings/schema drift checker, and the rest of the standalone generators -- to real .ts, inferring each function's types from its actual behavior rather than trusting the old declaration, per the pattern proven out on the miner/mcp packages. Nothing in scripts/ was ever part of Codecov's coverage surface, so this adds no coverage obligation; it's a straight type-safety and drift-elimination win. Real drift the old .d.mts files had already accumulated, found while converting: - check-schema-drift.ts read a table's name via SQLiteTable.Symbol.Name, an @internal drizzle-orm symbol never in its public type exports (which is exactly why the old hand-written declaration typed it without complaint) -- switched to the public getTableName(). - ci-duration-report.ts's WorkflowRun type was missing the `event` field the code actually filters on. - orb-release-core.ts's IMAGE_RELEVANT_PREFIXES still named two sibling scripts by their old .mjs filenames, now renamed here too -- a commit touching either file under its real name would have silently stopped counting as image-relevant. Every consumer updated to match: .js-suffixed import specifiers (Vite/ esbuild/Wrangler already resolve these to the sibling .ts, same as the prior commit), test imports, and every real invocation site. A script whose own file stays .mjs but now imports something converted here (e.g. check-mcp-package.mjs importing forbidden-content.ts) needs tsx instead of plain node, since only tsx (not node --experimental-strip-types) resolves a same-directory .ts import transitively; a script with zero local imports uses --experimental-strip-types directly, cheaper than spawning tsx. Covers every affected npm script, the three release-watch GitHub workflows (which previously needed no npm install at all -- added ./.github/actions/setup-workspace to each), the Dockerfile, and deploy-selfhost-prebuilt.sh. That last category caught two live regressions already sitting on this branch from the prior commit, beyond the one this commit's own check-miner-deployment-docs.ts conversion fixes (that one's what's been failing this PR's own CI): packages/loopover-miner/scripts/ generate-env-reference.mjs (npm run miner:env-reference, part of test:ci) and the Dockerfile/deploy-selfhost-prebuilt.sh's validate-selfhost-sourcemap invocations were both silently broken the same way -- caught by grepping for every remaining literal .mjs reference to a converted filename repo-wide, not by any test, since the one existing test for the miner env-reference generator imports it through Vite (which already tolerates the mismatch) rather than spawning it as the real subprocess the npm script actually runs. * fix(scripts): make miner/selfhost env-reference source scanning build-independent npm run miner:env-reference:check was reporting the committed docs stale in CI (validate-code's "Miner env-reference drift check" step, which runs before "Build miner CLI" in the same job) but clean locally, because gen-selfhost-env-reference.ts's directory walk scans both .js and .ts extensions and let whichever sorts first win the firstReference attribution for a given env var. That was harmless while packages/loopover-miner's compiled .js was always committed (#7290/#7291) -- every environment saw the same file set. Once it became gitignored, build-on-demand output (#7705), the walk's result started depending on whether a build happened to run before it: a dev machine with a recent `npm run build:miner` sees both lib/foo.js and lib/foo.ts and picks .js (alphabetically first); a fresh CI checkout before its own build step sees only lib/foo.ts. Skip a .js/.mjs/.cjs file when a same-basename .ts/.tsx sibling exists in the same directory listing, in both gen-selfhost-env-reference.ts's walker and generate-env-reference.mjs's own duplicate of it (used for the generated doc's Default column) -- .ts is always the real, always-present source; its compiled sibling is redundant now, not a second independent reference. Verified deterministic by generating with the compiled output physically removed and again with it present: identical output both times, regenerated docs to match. * fix(ci): build MCP/miner CLI binaries in validate-tests before running their harness tests The compiled packages/loopover-{mcp,miner} bin/lib output stopped being committed in this branch's own first commit (build(mcp,miner): stop committing compiled .js/.d.ts entirely) -- validate-code's job already builds both via its "Build MCP"/"Build miner CLI" steps, but validate-tests only ever built @loopover/engine, since the compiled binaries used to just be present from checkout for free. Its own SKIP_MCP_CLI_HARNESS/SKIP_MINER_TEST_HARNESS gates can both be false (true whenever a PR touches the relevant paths, which this branch does by construction), spawning the real loopover-mcp.js/loopover-miner.js CLI as a subprocess -- a child-process spawn has no bundler to resolve a .js specifier to sibling .ts, unlike every other consumer of these packages, so every mcp-cli-*/miner-* test failed with MODULE_NOT_FOUND across all 3 shards. Mirrors validate-code's own two build steps, unconditional like this job's pre-existing "Build engine package" -- turbo's own content-hash caching keeps an unrelated PR's cost near zero. * fix(build): declare @loopover/mcp#build's real turbo outputs outputs: [] was correct back when bin/lib's compiled .js was always committed -- present via git regardless of whether turbo actually ran tsc or replayed a cached log entry, so a cache hit was harmless. Once compiled output stopped being committed, that stopped being true: a cache hit now means turbo trusts a previously-cached log and skips running tsc again, but with no outputs declared it never snapshotted or restores the actual files either, so the compiled output can end up silently missing on a runner that only ever saw a "hit." Confirmed live in this PR's own CI: validate-tests' 3 parallel shards share one turbo-tests- GitHub Actions cache key/prefix. On the run that added the "Build MCP" step, shard 1's @loopover/mcp:build showed a genuine cache miss (real tsc execution) while shard 2 showed a hit for the identical content hash and never ran tsc at all -- leaving packages/loopover-mcp/bin/loopover-mcp.js missing on that shard's runner despite the step reporting success, and every mcp-cli-* test that reads or spawns it failing with ENOENT/MODULE_NOT_FOUND. Declaring the real outputs (bin/**/*.js, lib/**/*.js) doesn't disable caching -- unchanged source still skips a real tsc run -- it makes a hit correct: turbo now restores the actual files from its snapshot instead of only the log. @loopover/miner#build:tsc doesn't share this risk (cache: false forces real execution every time, by design). * fix(ci): force-build MCP/miner CLI binaries in validate-tests + fail fast if missing Two more layers on top of the last two fixes (adding the build steps, then declaring turbo's real outputs), after live CI still showed a sporadic single-shard failure: validate-tests' 3 shards race on one shared turbo-tests- GitHub Actions cache key, so even with correct outputs declared, a shard's cache "hit" can still be for content a sibling shard built moments earlier on a different runner -- valid in principle, but one more moving part than a job with hundreds of downstream tests depending on the result should lean on. Also traced (and ruled out as a separate bug) why this looked, for one run, like a genuine test regression rather than a build issue: test/unit/support/mcp-cli-harness.ts spawns the CLI's .ts entry directly via `node --experimental-strip-types` instead of requiring a prior build, but --experimental-strip-types only strips types from the directly-executed 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 every mcp-cli-*/miner-mcp-* test -- including ones already using the "fixed" harness -- still needs bin/lib's compiled .js physically on disk; the harness's own .ts-entry change never removed that dependency, it only changed how the entry file itself gets its types stripped. --force on both Build MCP/Build miner CLI steps makes this job's correctness independent of turbo cache state entirely -- always executes for real, never trusts a hit from any source. The packages are small (a couple seconds each, confirmed locally), a fixed cost worth paying to eliminate the whole cache-hit-but-file-missing failure class rather than keep re-verifying it shard by shard. A new verification step right after both builds fails fast with a clear message if a binary is somehow still missing, instead of dozens of confusing per-test MODULE_NOT_FOUND/ENOENT failures far downstream that don't obviously point back here. Verified locally end to end: deleted all compiled mcp/miner output and .tsbuildinfo, ran both --force build commands from that clean state, confirmed both binaries exist, then ran the full mcp-cli-*/mcp-*/ miner-mcp-*/miner-cli-* test suite (120 files, 974 tests) against that real build -- all green, including mcp-cli-doctor.test.ts's "runs doctor against a local health/session fixture", the one test that had looked like a genuine assertion regression in CI. * fix(test): stop asserting client_path always passes in mcp-cli-doctor.test.ts Genuinely pre-existing, unrelated to any of the build/caching work on this branch (confirmed: zero commits since this branch's base touch this file). Only surfaced now because earlier CI failures on this branch were catastrophic enough (every mcp-cli-*/miner-* test failing on a missing binary) to mask this one underneath. doctor's "local_repo_readiness" group includes a "client_path" check that scans process.env.PATH for a "loopover-mcp" executable via a plain findExecutable() -- in practice this resolves through npm's own workspace bin-link at node_modules/.bin/loopover-mcp. npm only creates that symlink if the bin target (bin/loopover-mcp.js) already exists at `npm ci` time. Since build(mcp,miner): stop committing compiled .js/ .d.ts entirely stopped committing that file, every CI job's npm ci now runs before any build step ever does, so the symlink is never created there -- regardless of a later build succeeding. The check itself already treats this gracefully (status "warn" with remediation guidance, never "fail"), but the test hardcoded status: "pass" for the whole group, which only happened to hold locally because of a stray global @loopover/mcp install left on this machine's own PATH from earlier, unrelated testing -- not something any CI runner, or a fresh contributor checkout, would ever have. Verified by literally reproducing the CI condition: moved my own machine's global loopover-mcp symlink aside, confirmed `which loopover-mcp` finds nothing (matching a clean CI PATH), ran this exact test file against that state -- all 19 tests pass, including this one. Restored the symlink afterward.
Closed
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#7290/#7291TypeScript migrations forpackages/loopover-minerandpackages/loopover-mcpare complete — every.js/.d.tsin both packages is now compiler-owned, not hand-maintained. But both packages compile in place and commit the emitted output (they ship as installable CLIs, so a runnable.jshas to exist without a build step for consumers), and nothing verified that the committed output actually matched a fresh build: root-level tests import the emitted.jsby its literal specifier, and the job that builds these packages runs as a separate GitHub Actions job from the one that tests them — so a contributor who edited.tswithout rebuilding got tests silently exercising old behavior with zero CI signal. This is the same drift class the Selfhost/Miner env-reference checks were previously added to close.scripts/check-build-drift.mjs(git-status-based, so a compiled file that was never committed at all is caught too, not just a stale one) and wires it intoci.ymlright after each package's real build step, plus intopackage.json's localtest:cifor pre-push parity.packages/loopover-mcp/tsconfig.json,.github/workflows/publish-miner.yml, and one inci.yml), and addsloopover-mcp's missing migration-complete guard test (it had none;loopover-mineralready had one).#7668's freshly-merged full-execution mode:defaultPrepareExecutionWorkspacebuilt its scratch directory straight frommkdtempSync, butcoding-task-spec.ts'swriteAcceptanceCriteriaFileseparatelyrealpaths the working directory before writing into it (a deliberate containment-check canonicalization). On macOS,os.tmpdir()resolves under a symlink (/var/folders/...→/private/var/folders/...), so the two disagreed on which string names the same directory, andacceptanceCriteriaPath.startsWith(workingDirectory)failed even though the file genuinely was inside the working directory. Fixed by resolving once at the source; no-op on Linux CI runners.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not touchsite/,CNAME, or**/lovable/**.Validation
git diff --check— clean.npm run actionlint— clean (validates theci.yml/publish-miner.ymledits).npm run typecheck— clean.npm run build:mcp/npm run build:miner(via the new:checkvariants) — clean, both pre- and post-commit.npm audit --audit-level=moderate— 0 vulnerabilities.check-build-drift-script.test.tscovers the clean/dirty/untracked-file/unknown-package-name cases (including a real scratch-git-repo mechanics test, not just injected fakes); the realpath fix has a direct regression test (workspace.pathis already realpath-canonical) plus the pre-existing integration test it was breaking now passes (60/60 inminer-cross-repo-evaluation.test.ts).packages/loopover-miner/lib/cross-repo-evaluation.ts(the real file from#7668) without rebuilding, confirmedbuild:miner:checkfailed with a clear message, then reverted cleanly before making the real change.If any required check was skipped, explain why:
npm run test:ci(the full local gate, which includestest:coverage,test:workers,ui:*) ran fully green once on this branch before the last two commits (a bug fix + a merge frommainthat picked up#7668) landed. Since then, validation was targeted: the affected test files,typecheck,actionlint,npm audit, and the new drift-checks themselves (pre- and post-commit) — all clean. Neither commit touches UI, workers, or anything else that full run covered. Real CI will run the authoritative version of the full suite against this exact pushed commit.Safety
Notes
ci.yml/publish-miner.ymlcomment fixes and the newmcp-typescript-migration-complete.test.tsare small, low-risk hygiene bundled in because they're the same "stale migration bookkeeping" class this PR is otherwise about, and were found in the course of tracing exactly whypackages/loopover-miner/lib/cross-repo-evaluation.js/.d.tsstill show up in contributor diffs like#7668.