Skip to content

feat(mcp): register plan-DAG tools + local scorer in packages/loopover-mcp - #6462

Closed
galuis116 wants to merge 2 commits into
JSONbored:mainfrom
galuis116:feat/mcp-register-plan-dag-and-scorer-tools
Closed

feat(mcp): register plan-DAG tools + local scorer in packages/loopover-mcp#6462
galuis116 wants to merge 2 commits into
JSONbored:mainfrom
galuis116:feat/mcp-register-plan-dag-and-scorer-tools

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Closes #6150

Summary

src/mcp/server.ts registers loopover_run_local_scorer, loopover_build_plan, loopover_plan_status, loopover_record_step_result, and loopover_predict_gate on the remote server, and packages/loopover-mcp/bin/loopover-mcp.js's miner-auto-dev profile listed all five in recommendedTools — but none were actually registered as local stdio tools, only the string literals existed. A contributor relying on the local server for this profile couldn't invoke any of them.

  • loopover_run_local_scorer: computeLocalScorerTokens imported directly from @loopover/engine (already exported at the package root) — same pattern as the existing loopover_check_slop_risk/loopover_lint_pr_text pure in-process tools. Pure, deterministic, no repo/network access.
  • loopover_build_plan / loopover_plan_status / loopover_record_step_result: the plan-DAG state machine (src/services/plan-dag.ts) was never extracted to @loopover/engine's export map, so there's nothing to import — hand-duplicated here following the exact same precedent this file already uses for MAINTAIN_ACTION_CLASSES/AUTONOMY_LEVELS when the published package's export map doesn't cover something. Pure + stateless (no DB, no network) — the harness runs each step and calls loopover_record_step_result to report it back.
  • loopover_predict_gate: cannot be pure-local — it needs live repo/issue/PR/manifest data only the server can assemble (env.DB-backed). Proxies to the existing POST /v1/local/branch-analysis route, which already computes predictedGate via buildPredictedGateVerdict (the identical logic the remote tool uses) and returns it as a top-level response field — no new backend endpoint needed. Uses a metadata-only input shape (no git/workspace context), unlike the sibling branch-analysis tools that shell out to git.

Incidental fix

While testing, found packages/loopover-mcp/node_modules/@loopover/engine was a stale, non-symlinked directory shadowing the correct root-level workspace symlink, breaking the CLI's own @loopover/engine/signals/slop etc. subpath imports — confirmed pre-existing and unrelated to this change via git stash comparison against a clean checkout. Removed it; the root symlink resolves correctly.

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck (root) — reliably OOMs on this shared sandbox regardless of what changed (reproduced repeatedly this session). packages/loopover-mcp is plain JS with its own npm run build (node --check across every lib/bin file) — ran it directly and it passes clean, and confirmed via direct execution that loopover-mcp --help and loopover-mcp tools --json (listing all 5 new tools) both run without error.
  • npm run test:coverage — not run repo-wide (same OOM risk). Added test/unit/mcp-cli-plan-scorer-tools.test.ts (15 tests: registration + success/rejection paths for all 5 tools, including the API-failure path for loopover_predict_gate) — all passing. Ran the full existing MCP CLI test suite (15 files, 143 tests) to confirm no regressions, including mcp-cli-tools.test.ts's "lists every registered stdio tool with a non-empty description" guard (would have caught a missing STDIO_TOOL_DESCRIPTORS entry) and mcp-cli-analyze-branch.test.ts (exercises the localBranchAnalysisFixture() I extended with a predictedGate field).
  • npm run test:workers — N/A, no Worker-facing code changed (this is the local CLI, not src/).
  • npm run build:mcp / npm run test:mcp-pack — both run directly and pass clean.
  • npm run ui:openapi:check / ui:lint / ui:typecheck / ui:build — N/A, no apps/loopover-ui changes.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • New/changed behavior has tests — 15 new tests covering all 5 tools' success paths, zod-rejection paths, and (for the HTTP-backed tool) an API-failure path via a new localBranchAnalysisStatus fixture-server option added to test/unit/support/mcp-cli-harness.ts, mirroring the existing intakeStatus pattern.

If any required check was skipped, explain why:

  • Root npm run typecheck / npm run test:coverage: reliably OOMs on this shared sandbox under memory pressure from concurrent sessions, independent of the diff. Substituted with packages/loopover-mcp's own build (clean), direct CLI execution confirming all 5 tools register and respond correctly, and the full MCP CLI test suite (158 tests total across this PR's own new file plus the broader regression sweep, all passing).

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 auth changes; the one HTTP-backed tool does have a negative-path (API-failure) test.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — New local MCP tools added and tested; no new backend API surface (reuses the existing /v1/local/branch-analysis route).
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section below with screenshots. — N/A, no visible UI change (CLI tool registration only).
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — CHANGELOG.md untouched.

Notes

  • loopover_build_plan/loopover_plan_status/loopover_record_step_result's hand-duplicated plan-DAG logic in loopover-mcp.js is a deliberate architectural choice, not an oversight: this file already documents (in the MAINTAIN_ACTION_CLASSES/AUTONOMY_LEVELS comment block) that it resolves @loopover/engine through the published package, whose export map exposes only a curated set of subpaths — widening that public API is a separate, larger decision than "register these 5 tools locally," so this follows the existing precedent rather than introducing a new one.

…r-mcp

The miner-auto-dev profile's recommendedTools listed
loopover_run_local_scorer/loopover_build_plan/loopover_plan_status/
loopover_record_step_result/loopover_predict_gate, but none were
registered as local stdio tools -- only the string literals existed.

- loopover_run_local_scorer: computeLocalScorerTokens imported
  directly from @loopover/engine (already exported), same pattern as
  the existing loopover_check_slop_risk/loopover_lint_pr_text pure
  in-process tools.
- loopover_build_plan / loopover_plan_status /
  loopover_record_step_result: the plan-DAG state machine
  (src/services/plan-dag.ts) was never extracted to @loopover/engine's
  export map, so it's hand-duplicated here following the same
  MAINTAIN_ACTION_CLASSES/AUTONOMY_LEVELS precedent this file already
  uses for exactly this situation. Pure + stateless -- no DB, no
  network access.
- loopover_predict_gate: cannot be pure-local (needs live repo/issue/
  PR/manifest data only the server can assemble). Proxies to the
  existing POST /v1/local/branch-analysis route, which already
  computes predictedGate via buildPredictedGateVerdict -- the same
  logic the remote tool uses -- and returns it as a top-level field.
  No new backend endpoint needed. Metadata-only input (no git
  required), unlike the branch-analysis tools that shell out to git.

Along the way, found and fixed a stale, non-symlinked
packages/loopover-mcp/node_modules/@loopover/engine directory
shadowing the correct root-level workspace symlink, which was
breaking the CLI's own subpath imports (unrelated to this change --
confirmed pre-existing via git stash).

Added test/unit/mcp-cli-plan-scorer-tools.test.ts (15 tests) covering
all 5 tools' success + rejection paths, and a
localBranchAnalysisStatus fixture-server option +
predictedGate field on localBranchAnalysisFixture in
test/unit/support/mcp-cli-harness.ts to test loopover_predict_gate's
API-failure path, mirroring the existing intakeStatus pattern.

Closes JSONbored#6150
@galuis116
galuis116 requested a review from JSONbored as a code owner July 16, 2026 09:15
@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 16, 2026
JSONbored#6150 registered loopover_run_local_scorer, loopover_build_plan,
loopover_plan_status, loopover_record_step_result, and
loopover_predict_gate, taking the total loopover_-prefixed stdio tool
count from 55 to 60. mcp-tool-rename-aliases.test.ts hardcodes this
count as a regression guard against silent alias/registration drift;
update it to match.
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
3483 2 3481 0
View the top 2 failed test(s) by shortest run time
test/unit/backfill.test.ts > GitHub backfill > repair diagnostics require contents:write for merge autonomy (#audit-install-health display)
Stack Traces | 0.548s run time
AssertionError: expected 'write' to be 'read' // Object.is equality

Expected: "read"
Received: "write"

 ❯ test/unit/backfill.test.ts:1267:54
test/unit/backfill.test.ts > GitHub backfill > marks comment, label, and check repair impacts disabled by repo settings
Stack Traces | 0.741s run time
AssertionError: expected { metadata: 'read', …(3) } to not have property "contents"

- Expected:
undefined

+ Received:
"write"

 ❯ test/unit/backfill.test.ts:1203:44

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-16 10:02:53 UTC

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

🛑 Suggested Action - Fix Blockers

Review summary
This registers the five previously-orphaned tools (loopover_run_local_scorer, loopover_build_plan, loopover_plan_status, loopover_record_step_result, loopover_predict_gate) on the local stdio server, closing #6150. The plan-DAG logic in bin/loopover-mcp.js (buildPlanDag/validatePlanDag/applyStepResult/planProgress) is a faithful line-for-line duplication of src/services/plan-dag.ts — I traced both and they match exactly, including the maxAttempts clamp, cycle-detection coloring, and terminal-state handling for failed steps. loopover_predict_gate correctly proxies to the existing /v1/local/branch-analysis route and unwraps result.predictedGate, matching the fixture's new predictedGate field. Tests exercise the real stdio server end-to-end (tool registration, happy path, cycle detection, retry exhaustion, zod-rejection paths, and an HTTP-failure path for the one network-backed tool), and the tool-count assertion is bumped from 55 to 60 to match the 5 new registrations.

Nits — 5 non-blocking
  • packages/loopover-mcp/bin/loopover-mcp.js: the plan-DAG state machine is now hand-duplicated in two places (here and src/services/plan-dag.ts) — any future bugfix to the source module (e.g. the `/* v8 ignore next */` guarded byId lookup) won't propagate here unless someone remembers to sync both copies; worth a comment/tracking issue to extract this into @​loopover/engine's export map rather than re-copying twice.
  • packages/loopover-mcp/bin/loopover-mcp.js: many new magic-number bounds on the zod shapes (400/2000/500/50/300/60/40000) aren't named constants, making it hard to tell at a glance which limits are intentional policy vs copy-paste from sibling shapes — consider a shared LIMITS object.
  • packages/loopover-mcp/bin/loopover-mcp.js: validatePlanDag's cycle-detection DFS lacks the `/* v8 ignore next */` comment the source module has on the `?? []` fallback, so this copy may show a phantom uncovered branch in coverage reports.
  • Extract the plan-DAG functions from src/services/plan-dag.ts into @​loopover/engine's export map (same treatment computeLocalScorerTokens just got) so bin/loopover-mcp.js can import instead of hand-duplicating ~90 lines of state-machine logic.
  • Consider a shared constants module for the zod length bounds (400/2000/500 etc.) since they're mirrored from src/mcp/server.ts's shapes and will drift silently if one side changes.

CI checks failing

  • validate
  • validate-tests (2)
  • validate-tests (6)

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 #6150
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
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: 1934 registered-repo PR(s), 1273 merged, 54 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1934 PR(s), 54 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Python, Dart, TypeScript, HTML, MDX, Rust, C++
  • Official Gittensor activity: 1934 PR(s), 54 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 1 step 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 16, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests (2), validate-tests (6))). 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.

feat(mcp): register plan-DAG tools + local scorer (build_plan/plan_status/record_step_result/run_local_scorer/predict_gate) in packages/loopover-mcp

1 participant