Skip to content

chore(mcp): convert lib/local-branch to TypeScript - #7413

Closed
lourincedaging0-commits wants to merge 1 commit into
JSONbored:mainfrom
lourincedaging0-commits:chore/mcp-local-branch-ts
Closed

chore(mcp): convert lib/local-branch to TypeScript#7413
lourincedaging0-commits wants to merge 1 commit into
JSONbored:mainfrom
lourincedaging0-commits:chore/mcp-local-branch-ts

Conversation

@lourincedaging0-commits

Copy link
Copy Markdown
Contributor

Phase 2 of the packages/loopover-mcp TypeScript migration (#7291), building on the tsc build pipeline that landed in Phase 1 (#7409).

What

Port lib/local-branch.jslib/local-branch.ts. The tsconfig include glob (lib/**/*.ts) picks the new source up automatically, and it compiles in place to lib/local-branch.js (inline source map, no .d.ts) — exactly the layout the package already ships, so no consumer import path changes.

Behavior

Runtime behavior and the public export surface are unchanged — every exported function keeps its name and semantics, and bin/loopover-mcp.js consumes the compiled .js exactly as before. The shipped lib/local-branch.js is now compiler-emitted rather than hand-maintained. Types were added at the process/JSON boundaries as open records (matching the existing converted lib/ files), keeping the strict build clean without narrowing the dynamic scorer/git-diff shapes.

Closes #7329

Phase 2 of the packages/loopover-mcp TypeScript migration (JSONbored#7291): port
lib/local-branch.js to lib/local-branch.ts, compiled in place by the tsc
pipeline landed in Phase 1. Runtime behavior and the public export surface
are unchanged; the shipped lib/local-branch.js is now compiler-emitted.

Closes JSONbored#7329
@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 80.83%. Comparing base (0d896ce) to head (575b10b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7413       +/-   ##
===========================================
- Coverage   91.22%   80.83%   -10.40%     
===========================================
  Files         716       84      -632     
  Lines       72801    22642    -50159     
  Branches    20885     3648    -17237     
===========================================
- Hits        66414    18302    -48112     
+ Misses       5347     4162     -1185     
+ Partials     1040      178      -862     
Flag Coverage Δ
shard-1 0.00% <ø> (-32.07%) ⬇️
shard-2 0.00% <ø> (-41.71%) ⬇️
shard-3 0.00% <ø> (-37.58%) ⬇️
shard-4 0.00% <ø> (-42.93%) ⬇️
shard-5 0.00% <ø> (-34.77%) ⬇️
shard-6 0.00% <ø> (-25.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 632 files 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 08:14:39 UTC

2 files · 1 AI reviewer · 1 blocker · CI failing · blocked

🛑 Suggested Action - Reject/Close

Review summary
This is a mechanical JS→TS port of lib/local-branch.js, preserving every exported function's name/signature and runtime behavior (confirmed by diffing the emitted .js against the old hand-written .js — logic is line-for-line identical, just re-emitted through tsc). Types at the process/JSON boundaries are kept as open Record<string, unknown> shapes rather than narrowed, consistent with the stated Phase 1 convention. The compiled .js silently drops the `join` import that the new .ts source still declares but never uses, which is the kind of thing that trips a strict TS/eslint unused-import check — and that lines up with this commit's validate/validate-code CI failures.

Blockers

  • packages/loopover-mcp/lib/local-branch.ts imports `join` from "node:path" (line 3) but the function body never calls it — that's a live unused-import that a `noUnusedLocals`/`no-unused-vars` check will reject, which is consistent with this commit's validate-code and validate checks failing.
Nits — 8 non-blocking
  • packages/loopover-mcp/lib/local-branch.ts:~528 types `execError` as `any` in `classifyScorerExecFailure` — narrow it to `NodeJS.ErrnoException & { stdout?, stderr?, status?, killed?, signal? }` instead of an explicit any, matching the rest of the file's open-record convention.
  • `splitCommand`'s regex `(?:[^\s"]+|"[^"]*")+` (local-branch.ts:594) nests two unbounded quantifiers; the input here is an operator-configured `GITTENSOR_SCORE_PREVIEW_CMD`, not attacker-controlled network input, so this is low-severity, but consider a linear alternative (e.g. iterative char-scan) if that assumption ever changes.
  • `workspaceRoots[0]!` (resolveWorkspaceCwd) uses a non-null assertion right after an explicit `.length === 0` early-return — safe today, but a stray reorder of that guard would silently reintroduce a runtime crash; a short comment or a `.at(0)` + explicit check would make the invariant self-documenting.
  • No test files are touched for either the .ts source or its compiled output, so none of the new type annotations or the (unused) `join` import would have been caught by a test run — only by the build/lint step that's currently red.
  • `master` is referenced in `defaultBaseRef` fallback logic (local-branch.ts:481) — purely a git-branch-name check, not a naming choice, so leave as-is; flagging only because it showed up in the terminology scan.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.
  • Configured validation evidence missing — Add regression/invariant coverage, update relevant tests, or attach passing validation output that satisfies the repo's configured expectations. Maintainers can also comment @loopover generate-tests for an AI-generated Playwright test.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.

Why this is blocked

  • packages/loopover-mcp/lib/local-branch.ts imports `join` from "node:path" (line 3) but the function body never calls it — that's a live unused-import that a `noUnusedLocals`/`no-unused-vars` check will reject, which is consistent with this commit's validate-code and validate checks failing.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. packages/loopover-mcp/lib/local-branch.ts imports \`join\` from "node:path" \(line 3\) but the function body never calls it — that's a live unused-import that a \`noUnusedLocals\`/\`no-unused-vars\` check will reject, which is consistent with this commit's validate-code and validate checks failing.

CI checks failing

  • validate
  • validate-tests-merge
  • validate-code

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 #7329
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 ⚠️ 12/25 Preflight needs author follow-up before maintainer review.
Contributor workload ✅ 10/10 Author activity: 94 registered-repo PR(s), 46 merged, 5 issue(s).
Contributor context ✅ Confirmed Gittensor contributor lourincedaging0-commits; Gittensor profile; 94 PR(s), 5 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: moderate
Linked issue satisfaction

Addressed
The PR converts lib/local-branch.js to a TypeScript source file (lib/local-branch.ts) that compiles in place to the existing lib/local-branch.js output, matching the issue's phased migration plan and building on the Phase 1 tsc pipeline, with typed process/JSON boundaries as open records per the stated convention.

Review context
  • Author: lourincedaging0-commits
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: HTML
  • Official Gittensor activity: 94 PR(s), 5 issue(s).
  • Related work: Titles/paths share 5 meaningful terms. (issue #7328)
  • 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 3 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.
Test coverage
  • No changed test files or passing validation evidence were detected for this PR.
  • Check the box below to generate an AI Playwright test for this PR, or comment @loopover generate-tests.
🧪 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
  • [BETA] Generate an AI Playwright test for this PR

@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, validate-tests-merge, validate-code); AI reviewers agree on a likely critical defect: packages/loopover-mcp/lib/local-branch.ts imports `join` from "node:path" (line 3) but the function body never calls it — that's a live unused-import that a `noUnusedLocals`/`no-unused-vars` check will reject, which is consistent with this commit's validate-code and validate checks failing.). 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
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 lib/local-branch.js to TypeScript (Phase 2 of #7291)

1 participant