Skip to content

chore(mcp): add tsc build pipeline + convert small lib utilities to TypeScript (Phase 1 of #7291) - #7404

Closed
oktofeesh1 wants to merge 1 commit into
JSONbored:mainfrom
oktofeesh1:claude/issue-7328-mcp-phase1
Closed

chore(mcp): add tsc build pipeline + convert small lib utilities to TypeScript (Phase 1 of #7291)#7404
oktofeesh1 wants to merge 1 commit into
JSONbored:mainfrom
oktofeesh1:claude/issue-7328-mcp-phase1

Conversation

@oktofeesh1

Copy link
Copy Markdown
Contributor

Summary

  • Adds packages/loopover-mcp/tsconfig.json and switches the package's build script from a node --check-only chain to a real tsc compile (in-place .ts -> .js emit, import paths unchanged), plus a glob-driven scripts/check-syntax.mjs syntax-verification pass -- mirroring packages/loopover-miner's own build pipeline exactly (build:tsc && build:verify).
  • Converts the four smallest, lowest-risk lib/ files to real TypeScript: cli-error.js, format-table.js, redact-local-path.js, telemetry.js.
  • No published-package shape change: declaration: false (no new .d.ts artifact type) and "!lib/**/*.ts" / "!scripts/check-syntax.mjs" exclusions in package.json's files keep the npm tarball's file list byte-identical to before -- verified against test/unit/check-mcp-package.test.ts's allowlist.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves.

Closes #7328

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally -- 100% lines/branches/functions on all 4 converted files, including a new test/unit/mcp-cli-error.test.ts for cli-error.js (previously untested at all) and added branch-coverage tests to test/unit/format-table.test.ts.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Not skipped, but note: npm audit --audit-level=moderate reports a pre-existing high-severity adm-zip advisory (no fix available) via github-actionlint, inherited unchanged from main and unrelated to this PR's packages/loopover-mcp scope.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A -- no such changes.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A -- pure internal helper refactor, no MCP tool surface changed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI changes.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • No behavior change, as the issue requires: every converted function's runtime logic and exported signature is unchanged from the original hand-written .js (only JSDoc-implicit types became real TypeScript types, with exactOptionalPropertyTypes/noUncheckedIndexedAccess fixes where the stricter compiler settings required them -- e.g. non-null assertions in format-table.ts where same-length-array indexing is provably in bounds).
  • Three existing test files (format-table.test.ts, redact-local-path.test.ts, mcp-local-telemetry.test.ts) had a @ts-expect-error suppression for dynamically importing what was previously an untyped .js module; these are now stale (TypeScript resolves the import to the new .ts source directly) and were removed.

…ypeScript

Adds packages/loopover-mcp/tsconfig.json and switches the package's build
script from a node --check-only chain to a real tsc compile (in-place
.ts -> .js emit, import paths unchanged) plus a glob-driven syntax
verification pass, mirroring loopover-miner's own build pipeline.

Converts the four smallest, lowest-risk lib files: cli-error.js,
format-table.js, redact-local-path.js, telemetry.js.

Refs JSONbored#7328
@oktofeesh1
oktofeesh1 requested a review from JSONbored as a code owner July 20, 2026 07:05
@superagent-security

Copy link
Copy Markdown
Contributor

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

@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 - fixes required

Review updated: 2026-07-20 07:09:01 UTC

16 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This is a mechanical, low-risk TypeScript conversion of 4 small `lib/` files (cli-error, format-table, redact-local-path, telemetry) plus a new tsc-based build pipeline that mirrors `loopover-miner`'s existing pattern exactly. The compiled `.js` output is functionally identical to the hand-written originals (whitespace/brace-style differences only from tsc's emit), and the `files` allowlist negation (`!lib/**/*.ts`) plus `declaration: false` keeps the published npm tarball byte-identical, which is verified by the existing `check-mcp-package.test.ts` allowlist test. New coverage (`mcp-cli-error.test.ts`, expanded `format-table.test.ts`) plausibly exercises the added type-narrowing branches (nullish rows, header-object fallback, align key/label fallback).

Nits — 5 non-blocking
  • Confirm `.tsbuildinfo` (written to the package root by `tsBuildInfoFile: "./.tsbuildinfo"` in packages/loopover-mcp/tsconfig.json:23) is gitignored so it doesn't get accidentally committed or picked up by a future `files` glob change.
  • The debug-leftover flags on `console.log`/`console.error` in `cli-error.ts` are false positives — those are the intended CLI output paths, not leftover debugging statements.
  • `scripts/check-syntax.mjs` has no test coverage of its own (it's a build-time script, not `src/**`, so this is outside the 97% patch-coverage bar, but worth a quick manual sanity check that it correctly reports failures).
  • If a future phase converts `bin/loopover-mcp.js`, consider whether `moduleResolution: NodeNext` in tsconfig.json will require explicit `.js` extension imports there too, consistent with the lib/ conversions here.
  • Nothing else to change here — this closely mirrors the loopover-miner tsc pipeline (repo precedent), which is the right pattern to reuse rather than inventing a new one.

CI checks failing

  • validate
  • changes

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 #7328
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: 84 registered-repo PR(s), 67 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor oktofeesh1; Gittensor profile; 84 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds tsconfig.json, converts a real tsc-based build/typecheck pipeline, and converts all four named files (cli-error, format-table, redact-local-path, telemetry) to .ts with matching compiled .js output and passing existing tests plus new coverage.

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: 84 PR(s), 0 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #7329)
  • Related work: Titles/paths share 11 meaningful terms. (issue #7329, issue #7328)
  • Related work: Titles/paths share 10 meaningful terms. (issue #7329, issue #7330)
  • 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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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 (CI is failing (validate, changes)). 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.

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): add tsc build pipeline + convert small lib utilities to TypeScript (Phase 1 of #7291)

1 participant