Skip to content

feat(mcp): add REST + CLI + stdio surfaces for loopover_plan_repo_issues - #7846

Closed
xfodev wants to merge 1 commit into
JSONbored:mainfrom
xfodev:feat/plan-repo-issues-three-surfaces-7764
Closed

feat(mcp): add REST + CLI + stdio surfaces for loopover_plan_repo_issues#7846
xfodev wants to merge 1 commit into
JSONbored:mainfrom
xfodev:feat/plan-repo-issues-three-surfaces-7764

Conversation

@xfodev

@xfodev xfodev commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

loopover_plan_repo_issues (backed by generateIssuePlanDrafts) shipped only as a remote MCP tool (via Epic #7424, after the MCP-parity milestone closed), so it never got the REST + CLI + local-stdio mirror every other repo-scoped tool has. This adds all three surfaces:

  • REST: POST /v1/repos/:owner/:repo/issue-plan-drafts/generate (src/api/routes.ts + src/openapi/spec.ts), gated by requireAppRole + requireSessionRepoAccess, with the explicit_create_requires_dry_run_false create-safety guard (a bare create is rejected; only {create:true, dryRun:false} reaches the write path).
  • CLI: loopover-mcp maintain plan-issues --goal "..." (dry-run by default; --create/--limit/--json).
  • stdio tool: loopover_plan_repo_issues, proxying the REST mirror.

Coverage / testability

packages/loopover-mcp/bin/loopover-mcp.ts is a CLI dispatcher otherwise only exercised via subprocess spawn (which v8 can't instrument). To get real Codecov-measured coverage on the new lines, it adds an isProcessEntrypoint() guard + runAsCliEntrypoint flag so a unit test can import the module and drive runCli/maintainCli in-process without hijacking argv or binding stdin (the untestable server.connect(new StdioServerTransport()) line carries a /* v8 ignore */). test/unit/mcp-cli-plan-issues.test.ts drives both the maintain plan-issues CLI and the stdio tool in-process (against both the built .js and .ts), and test/unit/routes-issue-plan-draft.test.ts covers the REST route (session + static-token auth, cross-repo rejection, create-safety).

Verified locally on latest main: the affected mcp-cli + routes + openapi tests pass, tsc --noEmit clean, ui:openapi:check / command-reference:check / test:mcp-pack all clean (no drift), git diff --check clean, and every changed line in routes.ts / spec.ts / loopover-mcp.ts is covered.

Closes #7764

(Supersedes #7834, which the gate auto-closed on a base conflict: the miner/mcp bin migration removed the committed loopover-mcp.js from main, so the old PR's .js edit became a modify/delete conflict. This branch is .ts-only, rebased onto current main, with a maintain---help coverage test added.)

…lan_repo_issues

loopover_plan_repo_issues (generateIssuePlanDrafts) shipped only as a remote MCP
tool and never got the REST + CLI + local-stdio mirror surfaces its repo-scoped,
requireRepoManageAccess-gated siblings all have. Mirrors
loopover_generate_contributor_issue_drafts exactly: a POST
/v1/repos/:owner/:repo/issue-plan-drafts/generate route (same gate), a
maintain plan-issues CLI command calling it, and a loopover_plan_repo_issues
stdio tool. generateIssuePlanDrafts and the remote tool are unchanged.

Closes JSONbored#7764
@xfodev
xfodev requested a review from JSONbored as a code owner July 21, 2026 14:57
@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 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-21 15:09:44 UTC

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

🛑 Suggested Action - Fix Blockers

Review summary
Adds the REST + CLI + stdio mirror for `loopover_plan_repo_issues` (closes #7764), consistently threading the `explicit_create_requires_dry_run_false` create-safety guard through the new route (src/api/routes.ts), the `maintain plan-issues` CLI subcommand, and the stdio tool proxy in loopover-mcp.ts. The CLI always computes `dryRun: !create` and the stdio schema defaults `dryRun: true`/`create: false`, so a bare `--create`/`create:true` alone still resolves to the exact `{create:true, dryRun:false}` shape the route requires — traced end to end and consistent. The `isProcessEntrypoint()` guard is a reasonable, narrowly-scoped mechanism to get real coverage on the CLI dispatcher without hijacking test-runner argv or binding stdin, mirroring the precedent cited for `loopover-miner-mcp.ts`.

Nits — 6 non-blocking
  • packages/loopover-mcp/bin/loopover-mcp.ts:948-951 — the `goal` max length (2000) and default `limit` (5) are magic numbers; a named constant would make the create-safety/cost rationale (already explained in the comment) easier to keep in sync with src/api/routes.ts's matching literals.
  • src/api/routes.ts:2805-2842 — same magic-number duplication (2000/5/10) between the Zod schema here and the mirrored shape in loopover-mcp.ts; consider a shared constant if these are meant to always match.
  • packages/loopover-mcp/bin/loopover-mcp.ts:2651/3282/3527 — new `any`-typed handler params (`{ owner, repo, goal, dryRun, create, limit }: any`, etc.) widen the surface the same way the rest of the file already does; acceptable given file precedent but worth a `nit:` flag for anyone tightening the file later.
  • CI's `validate-tests (2)` and `validate` failed with no detail provided; this PR's branch is 2 commits behind the default branch, which is a plausible cause worth ruling out with a rebase before assuming a defect in this diff.
  • Consider extracting the repeated `{2000, 5, 10}` goal/limit constraints into one shared constant used by both src/api/routes.ts and loopover-mcp.ts's `planRepoIssuesShape`, since the two are explicitly meant to mirror each other.
  • Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.

CI checks failing

  • validate
  • validate-tests (2)

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 #7764
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low 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: 80 registered-repo PR(s), 46 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 80 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds a REST route calling generateIssuePlanDrafts gated by requireAppRole/requireSessionRepoAccess (mirroring the contributor-issue-drafts route's pattern), a CLI 'maintain plan-issues' command in loopover-mcp.ts, and a registerStdioTool block for loopover_plan_repo_issues that proxies the new route, matching all three requirements plus added regression tests.

Review context
  • Author: xfodev
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript
  • Official Gittensor activity: 80 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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.

Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before /
before /
after /
after /
/ mobile before / (mobile)
before / (mobile)
after / (mobile)
after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

Scroll preview
Route Before (production) After (this PR's preview)
/ before / (scroll)
before / (scroll)
after / (scroll)
after / (scroll)

A short scroll-through clip (desktop) — click either thumbnail to open the full animation. Evidence for scroll-linked behavior a single screenshot can't show.

🟩 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

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
1649 2 1647 0
View the top 2 failed test(s) by shortest run time
test/unit/mcp-tool-rename-aliases.test.ts > MCP legacy alias retirement (#4777) — discovery invariants > lists exactly 79 loopover_ tools and zero gittensory_-prefixed aliases
Stack Traces | 1.57s run time
AssertionError: expected 80 to be 79 // Object.is equality

- Expected
+ Received

- 79
+ 80

 ❯ test/unit/mcp-tool-rename-aliases.test.ts:76:28
test/unit/mcp-tool-rename-aliases.test.ts > MCP legacy alias retirement (#4777) — discovery invariants > `loopover-mcp tools --json` reports the same 79-tool count the live server registers
Stack Traces | 3.3s run time
AssertionError: expected 80 to be 79 // Object.is equality

- Expected
+ Received

- 79
+ 80

 ❯ test/unit/mcp-tool-rename-aliases.test.ts:97:27

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 21, 2026

Copy link
Copy Markdown
Contributor

This pull request changes UI/visual code but its screenshot evidence is incomplete. Every required viewport × theme combination needs its own before/after image pair in a labeled table row (e.g. "Desktop · Light | before | after"). Still missing: Desktop · Dark, Tablet · Dark, Mobile · Dark.

Please resubmit with the remaining rows filled in.

See https://github.com/JSONbored/loopover/blob/main/.claude/skills/contributing-to-loopover/SKILL.md for the exact format and examples. This is an automated maintenance action.

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.

loopover_plan_repo_issues has no REST route, CLI command, or local stdio MCP tool

1 participant