Skip to content

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

Closed
oktofeesh1 wants to merge 2 commits into
JSONbored:mainfrom
oktofeesh1:claude/issue-7330-mcp-phase3
Closed

chore(mcp): convert bin/loopover-mcp.js to TypeScript (Phase 3 of #7291)#7431
oktofeesh1 wants to merge 2 commits into
JSONbored:mainfrom
oktofeesh1:claude/issue-7330-mcp-phase3

Conversation

@oktofeesh1

Copy link
Copy Markdown
Contributor

Summary

  • Converts packages/loopover-mcp/bin/loopover-mcp.js (the MCP stdio server + CLI entry point) to bin/loopover-mcp.ts, a straight 1:1 rename compiled in place to the same bin/loopover-mcp.js the package has always shipped — no consumer-facing behavior change.
  • Typing is pragmatic throughout: explicit any where the original plain JS carried no type information (parameters, destructured options, empty-object-literal accumulators, catch-clause errors), matching the scope of the earlier mcp migration phases rather than a type-safety redesign.
  • Adds !bin/**/*.ts to package.json's files field so the new source file is excluded from the published npm tarball, mirroring the existing !lib/**/*.ts exclusion.

Fixes #7330.

Test plan

  • npx tsc -p packages/loopover-mcp/tsconfig.json --noEmit — 0 errors
  • npm run build --workspace @loopover/mcp (tsc + check-syntax.mjs) — passes
  • npm run typecheck (root) — passes
  • Full existing bin/loopover-mcp.js test suite (test/unit/mcp-cli-*.test.ts, mcp-discovery.test.ts, check-mcp-package.test.ts, mcp-release-candidate.test.ts, coding-agent-miner.test.ts, forbidden-content.test.ts, worker-entry-boundary.test.ts, etc.) — all passing against the recompiled output
  • npm run test:ci (full local gate, incl. git diff --check, test:coverage, test:mcp-pack) — green
  • npm audit --audit-level=moderate — pre-existing adm-zip/github-actionlint dev-only advisory only, unrelated to this change (no lockfile diff)

…Nbored#7291)

Straight 1:1 rename to bin/loopover-mcp.ts, compiled in place to the
same bin/loopover-mcp.js the package has always shipped. Pragmatic
typing throughout (explicit `any` where the original plain JS carried
no type information) rather than a type-safety redesign, matching the
scope of the prior mcp migration phases.

Excludes bin/*.ts from the published tarball via package.json's files
field, mirroring the existing lib/*.ts exclusion.
@oktofeesh1
oktofeesh1 requested a review from JSONbored as a code owner July 20, 2026 10:37
@superagent-security

Copy link
Copy Markdown
Contributor

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

bin/loopover-mcp.js became bin/loopover-mcp.ts; its two pre-existing
fallback-API-host mentions need a baseline entry under the new path.
@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 91.14%. Comparing base (25decd9) to head (a8e79c7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7431      +/-   ##
==========================================
- Coverage   91.35%   91.14%   -0.21%     
==========================================
  Files         716      716              
  Lines       72990    73152     +162     
  Branches    21628    22454     +826     
==========================================
  Hits        66678    66678              
- Misses       5272     5434     +162     
  Partials     1040     1040              
Flag Coverage Δ
shard-1 32.57% <ø> (-0.11%) ⬇️
shard-2 36.00% <ø> (-0.12%) ⬇️
shard-3 34.42% <ø> (-0.11%) ⬇️
shard-4 42.75% <ø> (-0.14%) ⬇️
shard-5 36.85% <ø> (-0.12%) ⬇️
shard-6 33.44% <ø> (-0.11%) ⬇️

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.ts 0.00% <ø> (ø)

... and 1 file with indirect coverage changes

@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

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-20 10:54:19 UTC

4 files · 1 AI reviewer · 1 blocker · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This is a mechanical 1:1 conversion of bin/loopover-mcp.js to a TypeScript source (bin/loopover-mcp.ts) that compiles back to the same shipped bin/loopover-mcp.js, with a package.json files-field exclusion added so the .ts source isn't published, mirroring the existing !lib/**/*.ts pattern. The PR description states the existing mcp-cli/mcp-discovery/check-mcp-package test suites were run against the recompiled output and pass, and the branding-drift baseline was updated for the new .ts path, which is consistent with a pure rename+typing pass. I cannot verify the actual diff content (both large files are omitted as 'too large to inline'), so I can't confirm the conversion is truly behavior-preserving beyond taking the PR's own test-plan claims at face value.

Nits — 5 non-blocking
  • No test files were added or modified alongside this conversion; for a large behavioral-surface file like the MCP CLI entry point, it'd be safer to add or update a lightweight compile/type-check smoke test in this PR rather than relying solely on pre-existing suites.
  • packages/loopover-mcp/bin/loopover-mcp.js and .ts are both marked as omitted/too-large-to-inline, so the actual diff correctness (e.g. whether typing changes altered runtime behavior anywhere) can't be independently verified from this review alone — worth a careful manual diff review before merge.
  • Confirm CI's full mcp test suite (mcp-cli-*, mcp-discovery, check-mcp-package, mcp-release-candidate) actually passes on this commit once checks complete, since that's the primary verification available given the file sizes.
  • Double check the !bin/**/*.ts exclusion in packages/loopover-mcp/package.json correctly excludes only the new .ts source and not any needed .d.ts or map files if those exist under bin/.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Why this is blocked

  • Patch-less file(s) could not be fully scanned for secrets (1) — Ensure patch-less files are within scan limits or split the change so secrets can be verified.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. Patch-less file(s) could not be fully scanned for secrets (1) — Ensure patch-less files are within scan limits or split the change so secrets can be verified.

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard 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: 83 registered-repo PR(s), 64 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor oktofeesh1; Gittensor profile; 83 PR(s), 0 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: minor
Linked issue satisfaction

Addressed
The PR makes the explicit design decision (single-file 1:1 rename, matching the phasing precedent), converts the full 6,594-line bin/loopover-mcp.js to bin/loopover-mcp.ts, and provides evidence of typecheck/build/test passes plus a files-field exclusion so the published package still ships the compiled JS unchanged.

Review context
  • Author: oktofeesh1
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, MDX, Python, TypeScript
  • Official Gittensor activity: 83 PR(s), 0 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 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (Patch-less file(s) could not be fully scanned for secrets (1)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 20, 2026
JSONbored pushed a commit that referenced this pull request Jul 20, 2026
…) (#7509)

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 #7431).

Co-authored-by: Cursor <cursoragent@cursor.com>
JSONbored added a commit that referenced this pull request Jul 26, 2026
…ing the PR (#9082) (#9103)

incompletePatchLessSecretScanFinding emitted code "secret_leak" whenever a
patch-less file's content couldn't be fetched or verified within the scan cap
(a Contents API 404/5xx, a >4MB file, a rate limit, or the fetch budget being
exhausted) -- sharing the code with a REAL, matched credential. secret_leak is
unconditional and breaker-exempt by design (CONCRETE_EVIDENCE_BLOCKER_CODES),
so an unreadable file auto-closed a legitimate PR with no live re-check ever
getting a chance to clear it. Confirmed live: 4 contributor PRs were closed
this way (metagraphed#4600, loopover#7155/#7431/#8376).

Gives the incomplete case its own code, secret_scan_incomplete, and routes it
through evaluateGateCheckCore's no-deterministic-blocker branch -- mirroring
ai_review_inconclusive's existing pattern exactly, and deliberately NOT the
unconditional isEvaluationBlocker check: it only holds when nothing else
already hard-blocked, so a real secret_leak on a DIFFERENT file in the same PR
still fails the gate rather than being buried in this new hold. Re-evaluates
automatically on the next sync/push, same as pre_merge_check_unresolved.

secret_leak itself is completely unchanged: a genuine match still hard-blocks
unconditionally, with no opt-in.

Tests: a gate-level regression pinning the neutral-vs-failure split (including
the real-leak-still-fails case with both finding codes present on one PR), the
end-to-end maybeAddSecretLeakFinding wiring test rewritten to assert the new
code and that secret_leak is absent, and the unit-level code assertion in
patchless-secret-scan.test.ts.

Closes #9082
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)

1 participant