Skip to content

chore(mcp): convert bin/loopover-mcp.js to TypeScript (Phase 3 of #7291) - #7509

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
RealDiligent:chore/mcp-bin-typescript-7330
Jul 20, 2026
Merged

chore(mcp): convert bin/loopover-mcp.js to TypeScript (Phase 3 of #7291)#7509
JSONbored merged 1 commit into
JSONbored:mainfrom
RealDiligent:chore/mcp-bin-typescript-7330

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

  • Design decision (explicit): single-file 1:1 conversion of bin/loopover-mcp.jsbin/loopover-mcp.ts (not a split). Natural seams exist, but splitting is a separate refactor; this PR only routes the CLI through tsc so @loopover/mcp is 100% compiler-verified TypeScript and closes Phase 3 of chore(mcp): migrate packages/loopover-mcp from plain .js to real TypeScript #7291 / chore(mcp): convert bin/loopover-mcp.js to TypeScript (Phase 3 of #7291) #7330.
  • Typing is pragmatic (any where plain JS had no types), matching Phase 1/2 scope — not a type-safety redesign.
  • Build: tsc emits bin/loopover-mcp.js, then scripts/strip-bin-sourcemap.mjs removes the inline sourcemap from bin only. Without that strip, the shipped JS (~635KB) exceeds LoopOver's 512KB patch-less secrets-scan fetch cap — the hard blocker that closed #7431. Lib modules keep inline maps for coverage remap; bin is subprocess-only tested so v8 never remaps it.
  • Also: !bin/**/*.ts in package.json files (mirror miner / !lib/**/*.ts); Windows-safe fileURLToPath in check-syntax.mjs; branding-drift baseline updated for the new .ts path.

Closes #7330.

Test plan

  • npx tsc -p packages/loopover-mcp/tsconfig.json --noEmit — 0 errors
  • npm run build --workspace @loopover/mcp (tsc + strip + check-syntax) — passes; bin/loopover-mcp.js ~319KB (under 512KB)
  • loopover-mcp --version / --help unchanged smoke
  • Focused CLI suite: mcp-cli-help, mcp-cli-completion-spec, worker-entry-boundary
  • node scripts/check-branding-drift.mjs — ok after baseline update
  • CI full gate (mcp-cli-*, mcp-discovery, check-mcp-package, coverage/pack)

Notes vs #7431

Same 1:1 approach as the closed attempt; the intentional delta is stripping the bin inline sourcemap so secrets-scan can fetch the compiled JS under the 512KB cap.

@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 20, 2026 18:16
…Nbored#7291)

Single-file 1:1 conversion through tsc, with bin sourcemap stripped so the
shipped JS stays under LoopOver'\''s 512KB patch-less secrets-scan fetch cap
(the failure mode that closed JSONbored#7431).

Co-authored-by: Cursor <cursoragent@cursor.com>
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.47%. Comparing base (98a8371) to head (db60091).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7509      +/-   ##
==========================================
- Coverage   91.43%   88.47%   -2.97%     
==========================================
  Files         718      719       +1     
  Lines       73214    75664    +2450     
  Branches    21692    22523     +831     
==========================================
  Hits        66944    66944              
- Misses       5227     7677    +2450     
  Partials     1043     1043              
Flag Coverage Δ
shard-1 25.02% <ø> (-1.17%) ⬇️
shard-2 35.66% <ø> (-1.67%) ⬇️
shard-3 34.74% <ø> (-1.62%) ⬇️
shard-4 41.01% <ø> (-1.92%) ⬇️
shard-5 36.49% <ø> (-1.71%) ⬇️
shard-6 32.21% <ø> (-1.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/loopover-mcp/bin/loopover-mcp.js 0.00% <ø> (ø)
packages/loopover-mcp/bin/loopover-mcp.ts 0.00% <ø> (ø)

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 20, 2026
@loopover-orb

loopover-orb Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-20 18:26:10 UTC

6 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a 1:1 TS conversion of bin/loopover-mcp.js with a new post-tsc step that strips the inline sourcemap from the compiled bin JS to stay under the 512KB secrets-scan cap, plus a Windows-safe fileURLToPath fix in check-syntax.mjs and a package.json files-list/branding-baseline update. The strip script's marker-based approach (lastIndexOf a literal sourceMappingURL comment) is a reasonable, low-risk technique, and the PR explicitly calls out the design tradeoff (bin untested for coverage remap, so stripping the map is safe). The large .js/.ts diffs are compiler output and can't be line-reviewed here, but CI passed including check-mcp-package and branding-drift, and the described test plan covers the CLI smoke paths.

Nits — 6 non-blocking
  • packages/loopover-mcp/scripts/strip-bin-sourcemap.mjs:16-20 strips only the LAST occurrence of the sourceMappingURL marker via lastIndexOf — if tsc ever emits multiple such comments (unlikely but not impossible with certain emit configs) earlier ones would remain; consider a regex-based strip-all if you want to be defensive.
  • packages/loopover-mcp/scripts/strip-bin-sourcemap.mjs:29 leaves a console.log in the build pipeline; fine for a build script but worth confirming it doesn't pollute output consumed by other tooling.
  • No test file changes accompany this large source diff; given it's a 1:1 mechanical conversion verified by tsc --noEmit and existing CLI smoke suites, that's acceptable, but worth calling out explicitly in the PR if reviewers expect new coverage.
  • Consider asserting in strip-bin-sourcemap.mjs that at least one file was stripped (or the count matches expectation) to fail loudly if the tsc emit format changes and the marker no longer matches.
  • Since this mirrors the loopover-miner bin pattern for fileURLToPath, a short comment or link between the two files would help future maintainers keep them in sync.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7330
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 382 registered-repo PR(s), 179 merged, 34 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 382 PR(s), 34 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: moderate
Linked issue satisfaction

Addressed
The PR makes an explicit design decision (single-file conversion), adds bin/loopover-mcp.ts as a 1:1 TypeScript conversion of the 6,594-line JS file, and addresses the build/size concern that sank the prior attempt via a sourcemap-stripping script, with tsc --noEmit and CLI smoke tests reported passing. The old bin/loopover-mcp.js is removed (large diff shown) and package.json/build scripts are up

Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 382 PR(s), 34 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (issue #7329)
  • Related work: Titles/paths share 6 meaningful terms. (issue #7328)
  • Related work: Titles/paths share 11 meaningful terms. (issue #7329, issue #7328)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@JSONbored
JSONbored merged commit e37ac71 into JSONbored:main Jul 20, 2026
15 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 21, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(mcp): convert bin/loopover-mcp.js to TypeScript (Phase 3 of #7291)

2 participants