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

src/api/routes.ts

@@ -176,7 +176,7 @@ import {
   validateOrbRelayEnrollment,
 } from "../orb/relay";
 import { computeFleetAnalytics } from "../orb/analytics";
-import { handleMcpRequest } from "../mcp/server";
+import { handleMcpRequest, isMcpAdminEnabled } from "../mcp/server";
 import { simulateOpenPrPressureSchema } from "../mcp/server";
 import { simulateOpenPrPressure, type OpenPrPressureInput } from "../services/open-pr-pressure-scenarios";
 import { DISCOVERY_PATHS, discoveryDocumentsFor, respondWithDocument, toolsForDeployment } from "../mcp/discovery-routes";
@@ -644,11 +644,15 @@ export function createApp() {
       // app (src/server.ts serves this very Hono instance), so the deployment has to be read at request
       // time rather than assumed.
       const deployment = isSelfHostedReviewRuntime(c.env) ? "selfhost" : "cloud";
+      // #10039: same request-time read `createServer()` uses to gate admin-tool REGISTRATION, so a
+      // self-host card never advertises the five admin tools when the flag that would register them is off.
+      const adminEnabled = isMcpAdminEnabled(c.env);
       const documents = discoveryDocumentsFor({
         version: LATEST_RECOMMENDED_MCP_VERSION,
         deployment,
+        adminEnabled,
         baseUrl: c.env.PUBLIC_API_ORIGIN ?? new URL(c.req.url).origin,
-        tools: toolsForDeployment(deployment),
+        tools: toolsForDeployment(deployment, adminEnabled),
       });
       return respondWithDocument(documents[path]!, c.req.header("if-none-match") ?? null);
     });

src/mcp/discovery-routes.ts

@@ -8,7 +8,8 @@
 // mounts the SAME routes over its own availability-filtered tool list. That is what makes a self-hosted
 // card truthful rather than a copy of the cloud one — a `cloud`-only tool is absent from a self-host card
 // because it is absent from that deployment's list, not because a second implementation remembered to
-// exclude it.
+// exclude it. The same truthfulness applies to a tool that is available but not REGISTERED (#10039's
+// admin category): a self-host card omits it too, because it is absent from what `/mcp` actually serves.
 import {
   buildAgentToolsIndex,
   buildAnthropicTools,
@@ -26,19 +27,31 @@ export type DiscoveryContext = {
   deployment: DiscoveryDeployment;
   baseUrl: string;
   tools: readonly McpToolDefinition[];
+  /** Whether THIS deployment currently registers the "admin" category (#10039's `isMcpAdminEnabled`).
+   *  Carried on the context (rather than re-derived from `tools`) so it can also feed the memo key below. */
+  adminEnabled: boolean;
 };
 
 /**
- * The tools a deployment truthfully serves: `both` plus its own kind.
+ * The tools a deployment truthfully serves: `both` plus its own kind, minus whatever it does not actually
+ * REGISTER (#10039).
  *
  * Locality is deliberately NOT filtered here. A `local-git` tool is still part of the catalog a client
  * discovers — the remote simply expects the caller to supply the branch metadata rather than reading a
  * checkout — so hiding it would under-describe the server.
+ *
+ * `availability` is not the only thing that decides whether a deployment serves a tool: "admin" is
+ * registered in `createServer()` only when `isMcpAdminEnabled(env)` is true, so a card built without regard
+ * to that flag would advertise five tools `/mcp` refuses as unknown on a default self-host deployment. This
+ * mirrors that same registration condition rather than re-deriving a category allowlist, so a second
+ * conditionally-registered category needs only a change here, not a new hardcoded filter at each caller.
  */
-export function toolsForDeployment(deployment: DiscoveryDeployment): McpToolDefinition[] {
+export function toolsForDeployment(deployment: DiscoveryDeployment, adminEnabled: boolean): McpToolDefinition[] {
   // `both` is not listed: the registry's filter treats it as the ABSENCE of a restriction, so it already
   // satisfies either constraint. Naming it here would read as if it were a third deployment.
-  return listToolDefinitions({ availability: [deployment] });
+  const tools = listToolDefinitions({ availability: [deployment] });
+  if (adminEnabled) return tools;
+  return tools.filter((tool) => tool.category !== "admin");
 }
 
 /**
@@ -100,7 +113,10 @@ export function respondWithDocument(document: DiscoveryDocument, ifNoneMatch: st
 const DOCUMENT_CACHE = new Map<string, Record<string, DiscoveryDocument>>();
 
 export function discoveryDocumentsFor(context: DiscoveryContext): Record<string, DiscoveryDocument> {
-  const key = `${context.deployment}|${context.version}|${context.baseUrl}`;
+  // `adminEnabled` rides along: without it, the first request on an isolate would pick a tool list and the
+  // memo would keep serving it to every later request on the same (deployment, version, baseUrl), even one
+  // that arrives after the flag flips.
+  const key = `${context.deployment}|${context.version}|${context.baseUrl}|${context.adminEnabled}`;
   let documents = DOCUMENT_CACHE.get(key);
   if (!documents) {
     documents = buildDiscoveryDocuments(context);

src/mcp/server.ts

@@ -785,8 +785,10 @@ void _INTERNAL_JOB_MESSAGE_TYPES_ARE_REAL;
 
 /** Master opt-in for the "admin" tool category (#7721), default OFF. Same truthy-string convention as every
  *  other LOOPOVER_* flag in this repo. Gates tool REGISTRATION in createServer() below; each admin tool
- *  handler additionally requires actor === "mcp-admin" at call time regardless of this flag. */
-function isMcpAdminEnabled(env: Env): boolean {
+ *  handler additionally requires actor === "mcp-admin" at call time regardless of this flag. Exported so the
+ *  `.well-known` discovery routes (#10039) can mirror this exact registration gate instead of growing a
+ *  second copy of the truthy-string regex. */
+export function isMcpAdminEnabled(env: Env): boolean {
   return /^(1|true|yes|on)$/i.test((env.LOOPOVER_MCP_ADMIN_ENABLED ?? "").trim());
 }
 

test/integration/mcp-discovery-routes.test.ts

@@ -122,3 +122,85 @@ describe("discovery routes (#9526)", () => {
     expect(card.remotes[0]!.url).toBe("https://api.loopover.ai/mcp");
   });
 });
+
+// #10039: a self-host card must describe only what THIS deployment's /mcp actually registers. "admin" is
+// the one category gated behind LOOPOVER_MCP_ADMIN_ENABLED (createServer's isMcpAdminEnabled) rather than
+// availability alone, so a card built from availability filtering only would advertise five tools the
+// server refuses as unknown on a default (flag-unset) self-host deployment.
+describe("admin-tool exclusion from a self-host card when the admin surface is not enabled (#10039)", () => {
+  const ADMIN_TOOL_NAMES = [
+    "loopover_admin_get_config",
+    "loopover_admin_write_config",
+    "loopover_admin_list_config_backups",
+    "loopover_admin_trigger_redeploy",
+    "loopover_admin_rotate_secret",
+  ];
+
+  it("lists none of the five admin tools when LOOPOVER_MCP_ADMIN_ENABLED is unset on a self-host env", async () => {
+    const env = createTestEnv();
+    expect(env.SELFHOST_TRANSIENT_CACHE, "the test env is the self-host runtime").toBeTruthy();
+    expect(env.LOOPOVER_MCP_ADMIN_ENABLED, "the flag defaults off").toBeFalsy();
+
+    const card = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), env)).json()) as {
+      tools: Array<{ name: string }>;
+    };
+    const index = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/agent-tools/index.json"), env)).json()) as {
+      tools: Array<{ name: string }>;
+    };
+    const cardNames = card.tools.map((tool) => tool.name);
+    const indexNames = index.tools.map((tool) => tool.name);
+    for (const name of ADMIN_TOOL_NAMES) {
+      expect(cardNames, `${name} must not be on the flag-off self-host card`).not.toContain(name);
+      expect(indexNames, `${name} must not be on the flag-off self-host index`).not.toContain(name);
+    }
+  });
+
+  it("lists all five admin tools when LOOPOVER_MCP_ADMIN_ENABLED=1 on the same self-host env", async () => {
+    const env = createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "1" });
+    expect(env.SELFHOST_TRANSIENT_CACHE, "the test env is the self-host runtime").toBeTruthy();
+
+    const card = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), env)).json()) as {
+      tools: Array<{ name: string }>;
+    };
+    const index = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/agent-tools/index.json"), env)).json()) as {
+      tools: Array<{ name: string }>;
+    };
+    const cardNames = card.tools.map((tool) => tool.name);
+    const indexNames = index.tools.map((tool) => tool.name);
+    for (const name of ADMIN_TOOL_NAMES) {
+      expect(cardNames, `${name} must be on the flag-on self-host card`).toContain(name);
+      expect(indexNames, `${name} must be on the flag-on self-host index`).toContain(name);
+    }
+  });
+
+  it("the flag-on and flag-off documents do not leak through the memo, with no reset needed", async () => {
+    // Same module instance, same (deployment, version, baseUrl) -- only the flag differs. Deliberately does
+    // NOT call resetDiscoveryCacheForTesting between the two requests: that would mask a memo key that
+    // forgot to carry the flag, since a fresh cache always misses regardless.
+    const off = await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), createTestEnv());
+    const on = await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "1" }));
+
+    const offBody = await off.text();
+    const onBody = await on.text();
+    expect(onBody).not.toBe(offBody);
+    expect(on.headers.get("etag")).not.toBe(off.headers.get("etag"));
+
+    const offNames = (JSON.parse(offBody) as { tools: Array<{ name: string }> }).tools.map((tool) => tool.name);
+    const onNames = (JSON.parse(onBody) as { tools: Array<{ name: string }> }).tools.map((tool) => tool.name);
+    for (const name of ADMIN_TOOL_NAMES) {
+      expect(offNames).not.toContain(name);
+      expect(onNames).toContain(name);
+    }
+  });
+
+  it("does not affect the cloud deployment's documents, which never listed the selfhost-only admin tools", async () => {
+    const cloudEnv = { ...createTestEnv({ LOOPOVER_MCP_ADMIN_ENABLED: "1" }), SELFHOST_TRANSIENT_CACHE: undefined } as unknown as Env;
+    const card = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), cloudEnv)).json()) as {
+      deployment: string;
+      tools: Array<{ name: string }>;
+    };
+    expect(card.deployment).toBe("cloud");
+    const names = card.tools.map((tool) => tool.name);
+    for (const name of ADMIN_TOOL_NAMES) expect(names).not.toContain(name);
+  });
+});

test/unit/mcp-discovery-surfaces.test.ts

@@ -30,16 +30,16 @@ import { buildOpenApiSpec } from "../../src/openapi/spec";
 // that is answering, it is stable enough to cache, and it never under- or over-states the tool set.
 
 const TOOLS = listToolDefinitions({ availability: ["cloud"] });
-const CONTEXT = { version: "3.15.2", deployment: "cloud" as const, baseUrl: "https://api.loopover.ai", tools: TOOLS };
+const CONTEXT = { version: "3.15.2", deployment: "cloud" as const, baseUrl: "https://api.loopover.ai", tools: TOOLS, adminEnabled: false };
 
 beforeEach(() => {
   resetDiscoveryCacheForTesting();
 });
 
 describe("availability filtering (#9526)", () => {
   it("a cloud card excludes selfhost-only tools, and a selfhost card excludes cloud-only ones", () => {
-    const cloud = new Set(toolsForDeployment("cloud").map((tool) => tool.name));
-    const selfhost = new Set(toolsForDeployment("selfhost").map((tool) => tool.name));
+    const cloud = new Set(toolsForDeployment("cloud", false).map((tool) => tool.name));
+    const selfhost = new Set(toolsForDeployment("selfhost", false).map((tool) => tool.name));
 
     // The registry's availability filter is INCLUSIVE (`both` satisfies any constraint), so "only" has to
     // be derived from the raw field rather than by filtering.
@@ -54,8 +54,8 @@ describe("availability filtering (#9526)", () => {
 
   it("both deployments carry every `both` tool", () => {
     const shared = listToolDefinitions().filter((tool) => tool.availability === "both").map((tool) => tool.name);
-    const cloud = new Set(toolsForDeployment("cloud").map((tool) => tool.name));
-    const selfhost = new Set(toolsForDeployment("selfhost").map((tool) => tool.name));
+    const cloud = new Set(toolsForDeployment("cloud", false).map((tool) => tool.name));
+    const selfhost = new Set(toolsForDeployment("selfhost", false).map((tool) => tool.name));
     for (const name of shared) {
       expect(cloud.has(name)).toBe(true);
       expect(selfhost.has(name)).toBe(true);
@@ -67,7 +67,7 @@ describe("availability filtering (#9526)", () => {
     // reading a checkout. Hiding them would under-describe the server.
     const localGit = listToolDefinitions({ locality: ["local-git"] }).filter((tool) => tool.availability === "both").map((tool) => tool.name);
     expect(localGit.length).toBeGreaterThan(0);
-    const cloud = new Set(toolsForDeployment("cloud").map((tool) => tool.name));
+    const cloud = new Set(toolsForDeployment("cloud", false).map((tool) => tool.name));
     for (const name of localGit) expect(cloud.has(name)).toBe(true);
   });
 });
@@ -215,7 +215,7 @@ describe("the per-origin memo (#9526)", () => {
   it("keeps deployments separate, so a self-host card is not a copy of the cloud one", () => {
     const cloud = JSON.parse(discoveryDocumentsFor(CONTEXT)["/.well-known/mcp.json"]!.body);
     const selfhost = JSON.parse(
-      discoveryDocumentsFor({ ...CONTEXT, deployment: "selfhost", tools: toolsForDeployment("selfhost") })["/.well-known/mcp.json"]!.body,
+      discoveryDocumentsFor({ ...CONTEXT, deployment: "selfhost", tools: toolsForDeployment("selfhost", false) })["/.well-known/mcp.json"]!.body,
     );
     expect(selfhost.deployment).toBe("selfhost");
     expect(selfhost.tools.length).not.toBe(cloud.tools.length);