Skip to content

feat(mcp): add remote + stdio tool surfaces for loopover_get_repo_focus_manifest#7919

Closed
xfodev wants to merge 2 commits into
JSONbored:mainfrom
xfodev:feat/get-repo-focus-manifest-two-surfaces-7808
Closed

feat(mcp): add remote + stdio tool surfaces for loopover_get_repo_focus_manifest#7919
xfodev wants to merge 2 commits into
JSONbored:mainfrom
xfodev:feat/get-repo-focus-manifest-two-surfaces-7808

Conversation

@xfodev

@xfodev xfodev commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the two missing MCP surfaces for loopover_get_repo_focus_manifest (a repo's own persisted focus manifest + compiled policy): a remote MCP tool in src/mcp/server.ts and a local stdio MCP tool in packages/loopover-mcp/bin. Mirrors the two-surface loopover_get_maintainer_noise shape but replicates the underlying GET /v1/repos/:owner/:repo/focus-manifest route's own auth — requireRepoAccess (the read-level maintainer/owner/operator + session-repo-access mirror), not the stricter requireRepoApprovalQueueAccess (which adds a live-write check the GET route does not perform).

Read-only, per the issue's scope boundary: no tool for the refresh(POST)/PUT write routes, no new REST route, no new human CLI verb.

What changed

  • src/mcp/server.ts: register loopover_get_repo_focus_manifest (input ownerRepoShape, focusManifestOutputSchema returning { repoFullName, manifest, policy }), add its MCP_TOOL_CATEGORIES entry ("maintainer"), and a getRepoFocusManifest handler calling loadRepoFocusManifest + compileFocusManifestPolicy exactly as the GET route does, behind requireRepoAccess.
  • packages/loopover-mcp/bin/loopover-mcp.ts: register the local stdio tool proxying GET ${repoBase}/focus-manifest via apiGet.

Testing / coverage

  • test/unit/mcp-output-schemas.test.ts: authorized success (seeded manifest → { repoFullName, manifest.present, policy }) and the forbidden branch (static mcp identity outside the read allowlist → cannot access this repository); added the tool to TOOLS_WITH_OUTPUT_SCHEMA. Both run in-process against LoopoverMcp(env).createServer(), so every changed line/branch in src/mcp/server.ts is Codecov-covered.
  • test/unit/mcp-cli-repo-focus-manifest.test.ts: in-process stdio invocation (via loopover_plan_repo_issues has no REST route, CLI command, or local stdio MCP tool #7764's isProcessEntrypoint guard + InMemoryTransport) so the bin tool's apiGet-proxy body gets real Codecov-measured coverage — a subprocess spawn can't be v8-instrumented.
  • test/unit/mcp-tool-rename-aliases.test.ts: tool-count invariant → 83. This also reconciles a pin drift — feat(mcp): add remote + stdio tool surfaces for loopover_get_activation_preview #7887 registered loopover_get_activation_preview's stdio tool without bumping this pin (live count had reached 82), documented inline in the same convention as the earlier #6942 note.

Incidental regression fix (required for CI)

This PR's packages/loopover-mcp/** change pulls the whole mcp-cli-*.test.ts suite into CI's scoped test selection, which surfaced a pre-existing failure: test/unit/mcp-cli-activation-preview.test.ts (added by #7887) hangs to its hook timeout on main. #7764 later gated the bin's top-level await server.connect(...) behind isProcessEntrypoint() (realpath(argv[1]) === realpath(this module)); that test set argv[1] to the bare string "loopover-mcp", so realpathSync throws → the guard is false → the server never connects → client.connect hangs. #7887 merged just before #7764, so its own CI never ran against the guard. Fixed by pointing argv[1] at the bin's resolved path via join(process.cwd(), ...) (test-only change).

No REST/OpenAPI change (the focus-manifest route already exists), no migrations, no wrangler changes.

Closes #7808

xfodev added 2 commits July 21, 2026 19:00
…us_manifest

Register loopover_get_repo_focus_manifest as a remote MCP tool (src/mcp/server.ts)
and a local stdio MCP tool (packages/loopover-mcp/bin), mirroring the two-surface
loopover_get_maintainer_noise shape but replicating the GET
/v1/repos/:owner/:repo/focus-manifest route's own auth: requireRepoAccess (the
read-level maintainer/owner/operator + session-repo-access mirror), not the
stricter requireRepoApprovalQueueAccess. Read-only: no refresh/PUT tool, no new
REST route, no new human CLI verb.

Closes JSONbored#7808
…SONbored#7764's entrypoint guard

JSONbored#7764 gated the bin's top-level `await server.connect(new StdioServerTransport())` behind
isProcessEntrypoint() (realpath(argv[1]) === realpath(this module)). JSONbored#7887's activation-preview
test set argv[1] to the bare string "loopover-mcp", so realpathSync throws, the guard is false,
the top-level connect is skipped, its mocked in-memory transport is never wired to the server,
and client.connect hangs to the 10s hook timeout. JSONbored#7887 merged just before JSONbored#7764, so its CI never
saw the guard; main has been latently red on this test since. Point argv[1] at the bin's own
resolved path so the guard is satisfied on import. Surfaced here because this PR's bin change pulls
the mcp-cli-*.test.ts suite into scoped selection.
@xfodev
xfodev requested a review from JSONbored as a code owner July 21, 2026 17:46
@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 17:56:37 UTC

7 files · 1 AI reviewer · no blockers · CI failing · dirty

🛑 Suggested Action - Fix Blockers

Review summary
This PR adds the two missing MCP surfaces (remote + stdio) for loopover_get_repo_focus_manifest, faithfully mirroring the GET /v1/repos/:owner/:repo/focus-manifest route's own requireRepoAccess gating rather than the stricter approval-queue check, consistent with the existing loopover_get_maintainer_noise two-surface pattern. The change is narrow and well-tested: both the authorized-success and forbidden branches are covered in-process for the remote tool, and the stdio bin tool gets real coverage via the isProcessEntrypoint-guarded in-process harness rather than a subprocess spawn. The tool-count pin bump (81→83) and fixture-server addition are updated consistently across all touched test files.

Nits — 6 non-blocking
  • packages/loopover-mcp/bin/loopover-mcp.ts:1556 uses an explicit `any` for the handler args (`async ({ owner, repo }: any) =>`), matching the existing convention in this file but still worth tightening to a typed shape derived from ownerRepoShape.
  • The external brief flags a possible secret at test/unit/mcp-cli-repo-focus-manifest.test.ts:29, but that line is just `process.env.LOOPOVER_API_TOKEN = "in-process-token"`, a clearly fake test fixture token, not a real leaked credential.
  • src/mcp/server.ts and packages/loopover-mcp/bin/loopover-mcp.ts are both very large files, but this diff only adds ~36/~17 lines respectively in the established per-tool pattern, so the file-size flag isn't specific to this change.
  • Consider asserting `data.manifest.wantedPaths` or `preferredLabels` content (not just `.present`) in mcp-output-schemas.test.ts to more tightly pin the manifest shape returned through the tool.
  • In loopover-mcp.ts, a small typed interface for the `{ owner, repo }` stdio handler args (matching the shape used elsewhere for other owner/repo tools, if one exists) would remove the `any` nit without much churn.
  • Possible secret-shaped assignment in the diff (generic_secret_assignment) — Verify the value is not a real credential.

CI checks failing

  • codecov/patch — 45.45% of diff hit (target 99.00%)

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 #7808
Related work ⚠️ 1 scoped overlap 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: 88 registered-repo PR(s), 47 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 88 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR registers loopover_get_repo_focus_manifest as both a remote MCP tool (src/mcp/server.ts) with ownerRepoShape input, the maintainer category entry, and a handler calling loadRepoFocusManifest + compileFocusManifestPolicy exactly as the GET route does, plus a local stdio tool proxying GET .../focus-manifest, while explicitly avoiding the refresh/PUT routes and any new CLI verb. It uses requir

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: 88 PR(s), 0 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (PR #7921)
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 21, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (codecov/patch); conflicts with the base branch — resolve and open a fresh PR). 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 21, 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.

loopover_get_repo_focus_manifest (read-only) has a REST route but no remote MCP tool or local stdio MCP tool

1 participant