Skip to content

fix(mcp): exclude admin tools from a self-host discovery card when the admin surface is off - #10172

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10039
Jul 31, 2026
Merged

fix(mcp): exclude admin tools from a self-host discovery card when the admin surface is off#10172
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10039

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(mcp): exclude admin tools from a self-host discovery card when the admin surface is off

toolsForDeployment only filtered on availability, so a self-hosted deployment's
.well-known documents advertised the five admin tools even when
LOOPOVER_MCP_ADMIN_ENABLED is unset and /mcp never registers them. Mirror the
same registration gate createServer uses, read it at request time alongside
the deployment, and fold it into the discovery memo key so the two documents
don't leak into each other on one isolate.

Closes #10039

UI Evidence

Viewport · Theme Before After
Desktop · Dark Desktop · Dark before
before
Desktop · Dark after
after
Tablet · Dark Tablet · Dark before
before
Tablet · Dark after
after
Mobile · Dark Mobile · Dark before
before
Mobile · Dark after
after

Validation

Verified locally on this branch before opening:

  • npm run typecheck
  • npx turbo run build:tsc build:verify
  • npm run test:coverage — patch coverage 100.0% of changed lines

…e admin surface is off

toolsForDeployment only filtered on availability, so a self-hosted deployment's
.well-known documents advertised the five admin tools even when
LOOPOVER_MCP_ADMIN_ENABLED is unset and /mcp never registers them. Mirror the
same registration gate createServer uses, read it at request time alongside
the deployment, and fold it into the discovery memo key so the two documents
don't leak into each other on one isolate.
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 31, 2026 10:44
@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:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.42%. Comparing base (0a67db2) to head (045d481).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #10172       +/-   ##
===========================================
- Coverage   92.21%   81.42%   -10.80%     
===========================================
  Files         932      285      -647     
  Lines      114114    62604    -51510     
  Branches    27566     8083    -19483     
===========================================
- Hits       105229    50974    -54255     
- Misses       7583    11229     +3646     
+ Partials     1302      401      -901     
Flag Coverage Δ
backend 96.35% <100.00%> (+0.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/api/routes.ts 95.62% <100.00%> (+<0.01%) ⬆️
src/mcp/discovery-routes.ts 100.00% <100.00%> (ø)
src/mcp/server.ts 97.66% <ø> (ø)

... and 779 files with indirect coverage changes

@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 11:09:29 UTC

5 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR correctly threads a request-time `isMcpAdminEnabled(env)` read into `toolsForDeployment` and the discovery-document memo key, so a self-host card built with the flag off now filters out the five admin-category tools that `createServer()` never registers — mirroring the same gate rather than duplicating an allowlist. The memo key change (`context.adminEnabled` appended) correctly prevents the flag-off and flag-on documents from leaking into each other on the same isolate, and the new integration tests explicitly assert this via a deliberate no-reset two-request test. The `isMcpAdminEnabled` export is a clean single-source-of-truth move (avoiding a second copy of the truthy-string regex), and existing call sites (`toolsForDeployment`, `DiscoveryContext`) were updated consistently across production and test code.

Nits — 5 non-blocking
  • src/mcp/discovery-routes.ts:37 and src/mcp/server.ts:789 reference 'mcp(discovery): exclude admin tools from a self-host card when the admin surface is not enabled #10039' inline in prose rather than a named constant, which is a purely cosmetic/maintainability nit, not a defect.
  • The admin-tool category filter in `toolsForDeployment` uses a string literal `"admin"` rather than importing a shared constant from the contract package, so a future rename of the category value would silently break this filter without a type error — worth double-checking whether `TOOL_CATEGORIES` (imported elsewhere in `server.ts`) exposes a typed constant to use here instead.
  • Consider referencing `TOOL_CATEGORIES.ADMIN` (if such a typed export exists in `@​loopover/contract`) in `toolsForDeployment`'s filter instead of the raw string `"admin"` for compile-time safety against category renames.
  • The PR body says 'Closes mcp(discovery): exclude admin tools from a self-host card when the admin surface is not enabled #10039' and the diff's scope (admin-tool exclusion + memo key fix) matches that issue precisely — no scope creep observed.
  • Possible screenshot-table issue: pair 1 — Advisory only — verify the screenshot-table images against the stated change before deciding.

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 #10039
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: 119 registered-repo PR(s), 82 merged, 3 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 119 PR(s), 3 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
toolsForDeployment now takes adminEnabled and filters out category "admin" tools when the flag is off, the flag is read at request time via the exported isMcpAdminEnabled(c.env) alongside the deployment read, and it's folded into the discoveryDocumentsFor memo key, matching the required pattern.

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 119 PR(s), 3 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 &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

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit c1484f5 into JSONbored:main Jul 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mcp(discovery): exclude admin tools from a self-host card when the admin surface is not enabled

1 participant