From 6dad0d236f4c42b55c57ee542ddd19a9276b6141 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:35:05 -0700 Subject: [PATCH 1/6] feat(mcp): serve the discovery surfaces and publish server.json The server card and the agent-tools trio are COMPUTED at request time from the contract registry. metagraphed committed its card and paid for it: every concurrent tool PR conflicted on the same generated file. There is nothing here to regenerate and nothing a tool PR can collide on. The same routes serve both deployments over their own availability-filtered tool set, so a self-host card is truthful rather than a copy of the cloud one -- a cloud-only tool is absent because it is absent from that deployment's list, not because a second implementation remembered to exclude it. Locality is deliberately NOT filtered: the remote serves local-git tools too, it just expects the caller to supply the branch metadata. generated_at derives from the version rather than the clock. A wall-clock value would change the body on every request, changing the ETag with it and leaving the 304 path as dead code that never fires. serverInfo.version stops being the hardcoded "0.1.0" it has reported since the server was written, and reads the same @loopover/mcp version the compatibility metadata already did -- so serverInfo, the card, and server.json cannot disagree about what shipped. The manifest check carries the anti-rot guard this issue asked for: it asserts every watched path EXISTS before validating a field. metagraphed's version-sync workflow watched a renamed path and kept passing for months while doing nothing, and a workflow that can quietly watch nothing is worse than no workflow. --- .github/workflows/publish-mcp-registry.yml | 77 +++++++ package.json | 3 +- packages/loopover-contract/package.json | 4 + packages/loopover-contract/src/discovery.ts | 152 ++++++++++++ scripts/check-server-manifest.ts | 125 ++++++++++ server.json | 26 +++ src/api/routes.ts | 17 ++ src/auth/route-auth.ts | 4 + src/mcp/discovery-routes.ts | 123 ++++++++++ src/mcp/server.ts | 6 +- test/integration/mcp-discovery-routes.test.ts | 103 +++++++++ .../unit/check-server-manifest-script.test.ts | 103 +++++++++ test/unit/mcp-discovery-surfaces.test.ts | 218 ++++++++++++++++++ vitest.config.ts | 1 + 14 files changed, 960 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/publish-mcp-registry.yml create mode 100644 packages/loopover-contract/src/discovery.ts create mode 100644 scripts/check-server-manifest.ts create mode 100644 server.json create mode 100644 src/mcp/discovery-routes.ts create mode 100644 test/integration/mcp-discovery-routes.test.ts create mode 100644 test/unit/check-server-manifest-script.test.ts create mode 100644 test/unit/mcp-discovery-surfaces.test.ts diff --git a/.github/workflows/publish-mcp-registry.yml b/.github/workflows/publish-mcp-registry.yml new file mode 100644 index 000000000..59ec2df71 --- /dev/null +++ b/.github/workflows/publish-mcp-registry.yml @@ -0,0 +1,77 @@ +name: Publish MCP Registry + +# Publishes server.json to the official MCP registry (#9526), so `io.github.JSONbored/loopover` is +# discoverable by any registry-aware client. +# +# workflow_dispatch ONLY, and main-only. Publishing announces a version to a public registry, which is not +# something a branch build or a merge should do on its own -- the version it advertises comes from +# @loopover/mcp's package.json, which the release automation bumps, so a publish is a deliberate follow-up to +# a release rather than a side effect of one. +# +# ANTI-ROT: the first step re-runs scripts/check-server-manifest.ts, which asserts every watched path still +# EXISTS before validating any field. metagraphed's version-sync workflow rotted silently for months because +# it watched a path that had been renamed -- it kept passing while doing nothing at all. A workflow that can +# quietly watch nothing is worse than no workflow, so this one fails loudly instead. + +on: + workflow_dispatch: + inputs: + dry_run: + description: "Validate and print what would be published, without publishing" + type: boolean + default: false + +permissions: + contents: read + # GitHub OIDC is how mcp-publisher proves this repo owns the io.github.JSONbored/* namespace. No + # long-lived registry credential exists to leak. + id-token: write + +concurrency: + group: publish-mcp-registry + cancel-in-progress: false + +jobs: + publish: + name: Publish server.json + runs-on: ubuntu-latest + # Belt and braces alongside the dispatch-only trigger: a dispatch can name any ref, and a registry + # publish must only ever describe what is on main. + if: github.ref == 'refs/heads/main' + steps: + - name: Checkout + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7 + with: + node-version-file: .nvmrc + + # Runs BEFORE anything else: a manifest whose version disagrees with the shipped package, or a watched + # path that has been renamed away, must stop the publish rather than announce something untrue. + - name: Validate server.json and its watched paths + run: node --experimental-strip-types scripts/check-server-manifest.ts + + - name: Install mcp-publisher + env: + # SHA-pinned: this binary authenticates as this repo and writes to a public registry, so it is not + # something to float on a moving tag. + MCP_PUBLISHER_VERSION: v1.2.3 + run: | + set -euo pipefail + curl -fsSL "https://github.com/modelcontextprotocol/registry/releases/download/${MCP_PUBLISHER_VERSION}/mcp-publisher_linux_amd64.tar.gz" -o mcp-publisher.tar.gz + tar -xzf mcp-publisher.tar.gz mcp-publisher + chmod +x mcp-publisher + + - name: Login via GitHub OIDC + run: ./mcp-publisher login github-oidc + + - name: Publish + if: ${{ !inputs.dry_run }} + run: ./mcp-publisher publish + + - name: Dry run — report what would be published + if: ${{ inputs.dry_run }} + run: | + echo "DRY RUN: server.json validated and OIDC login succeeded; publish skipped." + cat server.json diff --git a/package.json b/package.json index 0da383817..d4f2f8030 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "validate:no-hand-written-js": "node --experimental-strip-types scripts/validate-no-hand-written-js.ts", "import-specifiers:check": "node --experimental-strip-types scripts/check-import-specifiers.ts", "ui-derived-types:check": "node --experimental-strip-types scripts/check-ui-derived-types.ts", + "server-manifest:check": "node --experimental-strip-types scripts/check-server-manifest.ts", "dead-source-files:check": "node --experimental-strip-types scripts/check-dead-source-files.ts", "regate-sort-key:check": "node --experimental-strip-types scripts/check-regate-sort-key.ts", "command-redelivery-guards:check": "node --experimental-strip-types scripts/check-command-redelivery-guards.ts", @@ -128,7 +129,7 @@ "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts", "pretest:ci": "npm run check-node-version", - "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", + "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run ui-derived-types:check && npm run server-manifest:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run dispatch-gate-reasons:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run release-linked-versions:check && npm run command-reference:check && npm run mcp:tool-reference:check && npm run mcp:client-config:check && npm run contract:api-schemas:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", "test:watch": "vitest", diff --git a/packages/loopover-contract/package.json b/packages/loopover-contract/package.json index 0897051fd..ae939ddc7 100644 --- a/packages/loopover-contract/package.json +++ b/packages/loopover-contract/package.json @@ -59,6 +59,10 @@ "./api-schemas": { "types": "./dist/api-schemas.d.ts", "default": "./dist/api-schemas.js" + }, + "./discovery": { + "types": "./dist/discovery.d.ts", + "default": "./dist/discovery.js" } }, "files": [ diff --git a/packages/loopover-contract/src/discovery.ts b/packages/loopover-contract/src/discovery.ts new file mode 100644 index 000000000..32bc855f4 --- /dev/null +++ b/packages/loopover-contract/src/discovery.ts @@ -0,0 +1,152 @@ +// The discovery projections every runtime serves (#9526). +// +// `/.well-known/mcp.json` and the `/.well-known/agent-tools/*` trio are COMPUTED AT REQUEST TIME from the +// contract registry, never committed. metagraphed learned that the hard way: a committed server card made +// every concurrent tool PR conflict on the same generated file. There is nothing here to commit and nothing +// to regenerate — the registry is the artifact. +// +// Pure by construction: these take the tool list and a few facts and return plain objects, so the Worker, +// the self-host app, and any test can call them identically. Availability filtering is the CALLER's job — +// a self-host deployment passes its own filtered list, which is what makes the same route serve a truthful +// answer on both deployments. +import type { McpToolDefinition } from "./tool-definition.js"; + +/** Which deployment is answering; the card names it so a reader can tell the two apart. */ +export type DiscoveryDeployment = "cloud" | "selfhost"; + +export type ServerCardInput = { + /** The server's semantic version. Sourced from @loopover/mcp's package.json — never hand-bumped here. */ + version: string; + deployment: DiscoveryDeployment; + /** Absolute base URL this deployment answers on, e.g. `https://api.loopover.ai`. */ + baseUrl: string; + tools: readonly McpToolDefinition[]; + /** + * Timestamp for the card. DETERMINISTIC by contract: the caller passes a value derived from the deploy + * (the version), not `Date.now()` — a clock-derived field would change the ETag on every request and + * defeat the 304 path entirely. + */ + generatedAt: string; +}; + +export type ServerCard = { + name: string; + version: string; + description: string; + deployment: DiscoveryDeployment; + generated_at: string; + capabilities: { tools: { listChanged: boolean } }; + remotes: Array<{ type: "streamable-http"; url: string }>; + tools: Array<{ name: string; title: string; description: string; category: string; annotations: McpToolDefinition["annotations"] }>; +}; + +export const SERVER_CARD_NAME = "io.github.JSONbored/loopover"; +export const SERVER_CARD_DESCRIPTION = + "LoopOver's contribution-intelligence tools: issue ranking, branch and PR preflight, gate prediction, review explanation, and maintainer/operator management."; + +/** The MCP server card. Tool entries carry the catalog fields a client picks by, not the full schemas. */ +export function buildServerCard(input: ServerCardInput): ServerCard { + return { + name: SERVER_CARD_NAME, + version: input.version, + description: SERVER_CARD_DESCRIPTION, + deployment: input.deployment, + generated_at: input.generatedAt, + capabilities: { tools: { listChanged: true } }, + remotes: [{ type: "streamable-http", url: `${trimTrailingSlash(input.baseUrl)}/mcp` }], + tools: input.tools.map((tool) => ({ + name: tool.name, + title: tool.title, + description: tool.description, + category: tool.category, + annotations: tool.annotations, + })), + }; +} + +function trimTrailingSlash(value: string): string { + return value.replace(/\/+$/, ""); +} + +/** + * How a caller actually invokes one of these tools. Every agent-tools document carries it, because a tool + * catalog with no executor is a list a reader cannot act on. + */ +export type ToolExecutor = { transport: "streamable-http"; url: string; method: "tools/call" }; + +function executorFor(baseUrl: string): ToolExecutor { + return { transport: "streamable-http", url: `${trimTrailingSlash(baseUrl)}/mcp`, method: "tools/call" }; +} + +export type AgentToolsInput = { baseUrl: string; tools: readonly McpToolDefinition[]; generatedAt: string }; + +/** The neutral index: every tool with both schemas, plus the executor. */ +export function buildAgentToolsIndex(input: AgentToolsInput) { + return { + generated_at: input.generatedAt, + executor: executorFor(input.baseUrl), + tools: input.tools.map((tool) => ({ + name: tool.name, + title: tool.title, + description: tool.description, + category: tool.category, + annotations: tool.annotations, + input_schema: tool.inputSchema, + output_schema: tool.outputSchema, + })), + }; +} + +/** OpenAI's function-tool shape. `parameters` is the input schema verbatim — no second translation. */ +export function buildOpenAiTools(input: AgentToolsInput) { + return { + generated_at: input.generatedAt, + executor: executorFor(input.baseUrl), + tools: input.tools.map((tool) => ({ + type: "function" as const, + function: { name: tool.name, description: tool.description, parameters: tool.inputSchema }, + })), + }; +} + +/** Anthropic's tool shape. Same schemas, different key names; still one source. */ +export function buildAnthropicTools(input: AgentToolsInput) { + return { + generated_at: input.generatedAt, + executor: executorFor(input.baseUrl), + tools: input.tools.map((tool) => ({ + name: tool.name, + description: tool.description, + input_schema: tool.inputSchema, + })), + }; +} + +/** + * A weak ETag over the document. + * + * Weak, not strong: these documents are semantically stable per deploy but their byte encoding is not + * guaranteed across runtimes. The hash is FNV-1a over the serialized body -- non-cryptographic on purpose, + * since this identifies a cache entry rather than authenticating anything, and it must run cheaply inside a + * request on the Workers runtime. + */ +export function weakETag(body: string): string { + let hash = 0x811c9dc5; + for (let index = 0; index < body.length; index += 1) { + hash ^= body.charCodeAt(index); + // FNV prime, via shifts so this stays in 32-bit integer math rather than overflowing to a float. + hash = (hash + ((hash << 1) + (hash << 4) + (hash << 7) + (hash << 8) + (hash << 24))) >>> 0; + } + return `W/"${hash.toString(16)}"`; +} + +/** True when the request's `if-none-match` already names this entity, so the caller may answer 304. */ +export function matchesETag(ifNoneMatch: string | null | undefined, etag: string): boolean { + if (!ifNoneMatch) return false; + // A client may send a list, and may drop the weak prefix; compare on the opaque value alone. + const wanted = etag.replace(/^W\//, ""); + return ifNoneMatch + .split(",") + .map((candidate) => candidate.trim().replace(/^W\//, "")) + .some((candidate) => candidate === wanted || candidate === "*"); +} diff --git a/scripts/check-server-manifest.ts b/scripts/check-server-manifest.ts new file mode 100644 index 000000000..c08182ae5 --- /dev/null +++ b/scripts/check-server-manifest.ts @@ -0,0 +1,125 @@ +// server.json validation + the ANTI-ROT guard (#9526). +// +// metagraphed's version-sync workflow rotted silently for months because it watched a file path that no +// longer existed: the workflow kept passing while doing nothing. That is the failure this script exists to +// make impossible here, so it does two jobs: +// +// 1. Field-by-field validation of server.json against the live sources — the version must equal +// @loopover/mcp's package.json version, the npm identifier must be that package's real name, and the +// remote must point at the deployment the server card advertises. +// 2. WATCHED-PATH existence. Every path this check and the publish workflow depend on is listed here and +// asserted to exist in the working tree. A rename that leaves the list behind fails loudly instead of +// quietly watching nothing. +// +// Wired into test:ci, so a stale manifest cannot reach main and the publish workflow cannot be pointed at a +// file that has moved. +import { existsSync, readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +/** + * Every path the manifest check and the publish workflow read. Listed once, asserted to exist. Adding a + * watched path here without creating it is the same loud failure as deleting one. + */ +export const WATCHED_PATHS = [ + "server.json", + "packages/loopover-mcp/package.json", + ".github/workflows/publish-mcp-registry.yml", +] as const; + +export type ManifestProblem = { field: string; detail: string }; + +type ServerManifest = { + name?: unknown; + version?: unknown; + description?: unknown; + repository?: { url?: unknown; source?: unknown }; + remotes?: Array<{ type?: unknown; url?: unknown }>; + packages?: Array<{ registry_type?: unknown; identifier?: unknown; version?: unknown; transport?: { type?: unknown } }>; +}; + +export const EXPECTED_SERVER_NAME = "io.github.JSONbored/loopover"; +export const EXPECTED_REMOTE_URL = "https://api.loopover.ai/mcp"; +export const EXPECTED_PACKAGE_NAME = "@loopover/mcp"; + +export function checkWatchedPaths(deps: { exists?: (path: string) => boolean } = {}): ManifestProblem[] { + const exists = deps.exists ?? existsSync; + return WATCHED_PATHS.filter((path) => !exists(path)).map((path) => ({ + field: "watched-path", + detail: `${path} is watched by the manifest check or the publish workflow but does not exist — a rename left this list behind`, + })); +} + +/** + * Validate the manifest against the package that actually ships. + * + * The version is the point: it is NOT hand-maintained here. It must equal `@loopover/mcp`'s version, which + * the existing release automation bumps, so publishing can never advertise a version nobody released. + */ +export function checkServerManifest(manifestJson: string, mcpPackageJson: string): ManifestProblem[] { + const problems: ManifestProblem[] = []; + let manifest: ServerManifest; + try { + manifest = JSON.parse(manifestJson) as ServerManifest; + } catch (error) { + return [{ field: "server.json", detail: `is not valid JSON: ${error instanceof Error ? error.message : String(error)}` }]; + } + const mcpPackage = JSON.parse(mcpPackageJson) as { name?: string; version?: string }; + + if (manifest.name !== EXPECTED_SERVER_NAME) { + problems.push({ field: "name", detail: `expected ${EXPECTED_SERVER_NAME}, got ${String(manifest.name)}` }); + } + if (typeof manifest.description !== "string" || manifest.description.trim() === "") { + problems.push({ field: "description", detail: "must be a non-empty string — registry listings show it verbatim" }); + } + if (manifest.repository?.url !== "https://github.com/JSONbored/loopover" || manifest.repository?.source !== "github") { + problems.push({ field: "repository", detail: "must point at github.com/JSONbored/loopover with source: github" }); + } + if (manifest.version !== mcpPackage.version) { + problems.push({ + field: "version", + detail: `must equal @loopover/mcp's ${String(mcpPackage.version)} (release automation owns it), got ${String(manifest.version)}`, + }); + } + + const remote = manifest.remotes?.[0]; + if (!remote || remote.type !== "streamable-http" || remote.url !== EXPECTED_REMOTE_URL) { + problems.push({ field: "remotes[0]", detail: `must be a streamable-http remote at ${EXPECTED_REMOTE_URL}` }); + } + if ((manifest.remotes?.length ?? 0) !== 1) { + problems.push({ field: "remotes", detail: "must declare exactly one remote — a second entry is a second front door nobody tests" }); + } + + const npmPackage = manifest.packages?.[0]; + if (!npmPackage || npmPackage.registry_type !== "npm" || npmPackage.identifier !== (mcpPackage.name ?? EXPECTED_PACKAGE_NAME)) { + problems.push({ field: "packages[0]", detail: `must be the npm package ${String(mcpPackage.name)}` }); + } + if (npmPackage && npmPackage.version !== mcpPackage.version) { + problems.push({ field: "packages[0].version", detail: `must equal @loopover/mcp's ${String(mcpPackage.version)}, got ${String(npmPackage.version)}` }); + } + if (npmPackage?.transport?.type !== "stdio") { + problems.push({ field: "packages[0].transport", detail: "the npm package is the stdio entry point" }); + } + + return problems; +} + +export function collectProblems(deps: { readFile?: (path: string) => string; exists?: (path: string) => boolean } = {}): ManifestProblem[] { + const readFile = deps.readFile ?? ((path: string) => readFileSync(path, "utf8")); + const pathProblems = checkWatchedPaths(deps); + // A missing file makes every field check meaningless, so report the rot and stop. + if (pathProblems.length > 0) return pathProblems; + return checkServerManifest(readFile("server.json"), readFile("packages/loopover-mcp/package.json")); +} + +function main(): void { + const problems = collectProblems(); + if (problems.length === 0) { + process.stdout.write(`server.json: OK (${WATCHED_PATHS.length} watched paths present)\n`); + return; + } + process.stderr.write(`server.json has ${problems.length} problem(s) (#9526):\n`); + for (const problem of problems) process.stderr.write(` ${problem.field}: ${problem.detail}\n`); + process.exit(1); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) main(); diff --git a/server.json b/server.json new file mode 100644 index 000000000..77e933cf6 --- /dev/null +++ b/server.json @@ -0,0 +1,26 @@ +{ + "$schema": "https://static.modelcontextprotocol.io/schemas/2025-07-09/server.schema.json", + "name": "io.github.JSONbored/loopover", + "description": "LoopOver's contribution-intelligence tools: issue ranking, branch and PR preflight, gate prediction, review explanation, and maintainer/operator management.", + "repository": { + "url": "https://github.com/JSONbored/loopover", + "source": "github" + }, + "version": "3.15.2", + "remotes": [ + { + "type": "streamable-http", + "url": "https://api.loopover.ai/mcp" + } + ], + "packages": [ + { + "registry_type": "npm", + "identifier": "@loopover/mcp", + "version": "3.15.2", + "transport": { + "type": "stdio" + } + } + ] +} diff --git a/src/api/routes.ts b/src/api/routes.ts index 57aff8ebe..fce4c6744 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -179,6 +179,7 @@ import { computeFleetAnalytics } from "../orb/analytics"; import { handleMcpRequest } from "../mcp/server"; import { simulateOpenPrPressureShape } from "../mcp/server"; import { simulateOpenPrPressure, type OpenPrPressureInput } from "../services/open-pr-pressure-scenarios"; +import { DISCOVERY_PATHS, discoveryDocumentsFor, respondWithDocument, toolsForDeployment } from "../mcp/discovery-routes"; import { buildOpenApiSpec } from "../openapi/spec"; import { COMMAND_RATE_LIMIT_EVENT_TYPE, generateSignalSnapshots } from "../queue/processors"; import { generateChatQaAnswer } from "../services/ai-chat-qa"; @@ -1251,6 +1252,22 @@ export function createApp() { // enums/analyzer metadata (no DB/env/private data); excluded from requiresApiToken below. app.get("/v1/mcp/finding-taxonomy", (c) => c.json(buildFindingTaxonomyDocument())); app.get("/v1/mcp/enrichment-analyzers", (c) => c.json(buildEnrichmentAnalyzersTaxonomyDocument())); + // #9526: the MCP discovery surfaces, computed at request time from the contract registry (never a + // committed artifact -- a committed card is what made every concurrent tool PR conflict in metagraphed). + // Unauthenticated public metadata: tool names, descriptions, and schemas are already public, so these are + // excluded from requiresApiToken alongside the other unauthenticated document routes. + for (const path of DISCOVERY_PATHS) { + app.get(path, (c) => { + const documents = discoveryDocumentsFor({ + version: LATEST_RECOMMENDED_MCP_VERSION, + deployment: "cloud", + baseUrl: c.env.PUBLIC_API_ORIGIN ?? new URL(c.req.url).origin, + tools: toolsForDeployment("cloud"), + }); + return respondWithDocument(documents[path]!, c.req.header("if-none-match") ?? null); + }); + } + app.get("/openapi.json", (c) => c.json(buildOpenApiSpec())); app.all("/mcp", handleMcpRequest); diff --git a/src/auth/route-auth.ts b/src/auth/route-auth.ts index d19456c76..8df42732f 100644 --- a/src/auth/route-auth.ts +++ b/src/auth/route-auth.ts @@ -68,6 +68,10 @@ export function requiresApiToken(path: string): boolean { if (/^\/v1\/public\/decision-records\/[^/]+\/[^/]+\/[^/]+$/.test(path)) return false; if (path === "/openapi.json") return false; if (path === "/mcp") return false; + // #9526: the MCP discovery surfaces. Every field they carry -- tool names, descriptions, and JSON + // schemas -- is already public via /mcp's own tools/list and the published OpenAPI document, so gating + // them would only stop a registry crawler from finding a server that advertises itself on purpose. + if (path === "/.well-known/mcp.json" || path.startsWith("/.well-known/agent-tools/")) return false; // Public OAuth draft-submission flow (LOOPOVER_REVIEW_DRAFT): the submission entry points are unauthenticated // by design. The handlers themselves 404 when the flag is off, so this exemption is inert flag-OFF. if (path === "/v1/drafts" || path.startsWith("/v1/drafts/")) return false; diff --git a/src/mcp/discovery-routes.ts b/src/mcp/discovery-routes.ts new file mode 100644 index 000000000..819c5de70 --- /dev/null +++ b/src/mcp/discovery-routes.ts @@ -0,0 +1,123 @@ +// The `.well-known` discovery surfaces (#9526). +// +// COMPUTED AT REQUEST TIME from the contract registry — never a committed artifact. metagraphed learned +// that the hard way: a committed server card made every concurrent tool PR conflict on the same generated +// file. There is nothing here to regenerate, and nothing a tool PR can conflict on. +// +// One handler factory, two callers: the Worker mounts these on the cloud deployment, and the self-host app +// 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. +import { + buildAgentToolsIndex, + buildAnthropicTools, + buildOpenAiTools, + buildServerCard, + matchesETag, + weakETag, + type DiscoveryDeployment, +} from "@loopover/contract/discovery"; +import { listToolDefinitions } from "@loopover/contract/tools"; +import type { McpToolDefinition } from "@loopover/contract"; + +export type DiscoveryContext = { + version: string; + deployment: DiscoveryDeployment; + baseUrl: string; + tools: readonly McpToolDefinition[]; +}; + +/** + * The tools a deployment truthfully serves: `both` plus its own kind. + * + * 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. + */ +export function toolsForDeployment(deployment: DiscoveryDeployment): 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] }); +} + +/** + * `generated_at` is derived from the VERSION, not the clock. + * + * A wall-clock timestamp would change the body on every request, changing the ETag with it and making the + * 304 path dead code — the cache would never hit. Deriving it from the deploy means the document is stable + * for as long as the deployment is, which is exactly the cache lifetime a reader wants. + */ +export function deterministicGeneratedAt(version: string): string { + return `version:${version}`; +} + +export type DiscoveryDocument = { body: string; etag: string }; + +function serialize(payload: unknown): DiscoveryDocument { + const body = JSON.stringify(payload); + return { body, etag: weakETag(body) }; +} + +export function buildDiscoveryDocuments(context: DiscoveryContext): Record { + const generatedAt = deterministicGeneratedAt(context.version); + const agentToolsInput = { baseUrl: context.baseUrl, tools: context.tools, generatedAt }; + return { + "/.well-known/mcp.json": serialize(buildServerCard({ ...context, generatedAt })), + "/.well-known/agent-tools/index.json": serialize(buildAgentToolsIndex(agentToolsInput)), + "/.well-known/agent-tools/openai.json": serialize(buildOpenAiTools(agentToolsInput)), + "/.well-known/agent-tools/anthropic.json": serialize(buildAnthropicTools(agentToolsInput)), + }; +} + +/** + * Answer one discovery route: 304 when the caller already has this entity, else the document. + * + * `public` caching with a short max-age plus the ETag: these change only on deploy, but a stale card is + * misleading rather than merely old, so the window stays small and revalidation is cheap. + */ +export function respondWithDocument(document: DiscoveryDocument, ifNoneMatch: string | null): Response { + const headers = { + "content-type": "application/json; charset=utf-8", + etag: document.etag, + "cache-control": "public, max-age=300, must-revalidate", + }; + if (matchesETag(ifNoneMatch, document.etag)) { + // 304 carries no body by definition; the validators still travel so the cache can extend its entry. + return new Response(null, { status: 304, headers: { etag: document.etag, "cache-control": headers["cache-control"] } }); + } + return new Response(document.body, { status: 200, headers }); +} + +/** + * Documents memoized per (deployment, baseUrl, version). + * + * The bodies depend only on the registry and those three facts, all fixed for the life of an isolate, so + * building them once per origin keeps the per-request cost at a Map lookup. Keyed by baseUrl because a + * deployment can legitimately answer on more than one origin (the configured public origin and whatever the + * request actually arrived on), and each must advertise its OWN `/mcp` rather than the other's. + */ +const DOCUMENT_CACHE = new Map>(); + +export function discoveryDocumentsFor(context: DiscoveryContext): Record { + const key = `${context.deployment}|${context.version}|${context.baseUrl}`; + let documents = DOCUMENT_CACHE.get(key); + if (!documents) { + documents = buildDiscoveryDocuments(context); + DOCUMENT_CACHE.set(key, documents); + } + return documents; +} + +/** Every path these routes answer, so a caller can register them without hardcoding the list. */ +export const DISCOVERY_PATHS = [ + "/.well-known/mcp.json", + "/.well-known/agent-tools/index.json", + "/.well-known/agent-tools/openai.json", + "/.well-known/agent-tools/anthropic.json", +] as const; + +/** Test-only: drop the memo so one test's origin cannot leak into the next. */ +export function resetDiscoveryCacheForTesting(): void { + DOCUMENT_CACHE.clear(); +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 736a5bdbc..82ce2d30e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -311,6 +311,7 @@ import { isMcpReadUnscoped, type AuthIdentity, } from "../auth/security"; +import { LATEST_RECOMMENDED_MCP_VERSION } from "../services/mcp-compatibility"; import { canLoginAccessRepo, canWatchRepo, loadControlPanelAccessScope, loadControlPanelRoleSummary, type ControlPanelAccessScope } from "../services/control-panel-roles"; import { countOpenIssues, @@ -1228,7 +1229,10 @@ export class LoopoverMcp { createServer(): McpServer { const server = new McpServer({ name: "loopover", - version: "0.1.0", + // #9526: derived, not a hand-bumped constant. LATEST_RECOMMENDED_MCP_VERSION already reads + // @loopover/mcp's package.json, which the release automation owns -- so serverInfo, the compatibility + // metadata, the server card, and server.json all report the one version that actually shipped. + version: LATEST_RECOMMENDED_MCP_VERSION, }); // #6301 — register every tool through this thin wrapper so its category rides along as MCP diff --git a/test/integration/mcp-discovery-routes.test.ts b/test/integration/mcp-discovery-routes.test.ts new file mode 100644 index 000000000..9ee3c02fb --- /dev/null +++ b/test/integration/mcp-discovery-routes.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { DISCOVERY_PATHS, resetDiscoveryCacheForTesting } from "../../src/mcp/discovery-routes"; +import { SERVER_CARD_NAME } from "@loopover/contract/discovery"; +import { createTestEnv } from "../helpers/d1"; + +// #9526: the discovery routes served by the real app — unauthenticated, cacheable, and describing the +// deployment that answered. The projections themselves are unit-tested; this covers the wiring: routing, +// the auth exemption, and conditional-request handling through Hono. + +const app = createApp(); + +function request(path: string, headers: Record = {}) { + return app.fetch(new Request(`https://api.loopover.ai${path}`, { headers }), createTestEnv()); +} + +beforeEach(() => { + resetDiscoveryCacheForTesting(); +}); + +describe("discovery routes (#9526)", () => { + it.each(DISCOVERY_PATHS)("%s is reachable WITHOUT a credential", async (path) => { + // A registry crawler has no token; gating these would only hide a server that advertises itself. + const response = await request(path); + expect(response.status, `${path} must not require auth`).toBe(200); + expect(response.headers.get("content-type")).toContain("application/json"); + }); + + it("the server card names the registry identity and this deployment's own /mcp", async () => { + const card = (await (await request("/.well-known/mcp.json")).json()) as { + name: string; + deployment: string; + remotes: Array<{ url: string }>; + tools: unknown[]; + }; + expect(card.name).toBe(SERVER_CARD_NAME); + expect(card.deployment).toBe("cloud"); + expect(card.remotes[0]!.url).toBe("https://api.loopover.ai/mcp"); + expect(card.tools.length).toBeGreaterThan(100); + }); + + it("does not hardcode a version — the card reports what the shipped package says", async () => { + const card = (await (await request("/.well-known/mcp.json")).json()) as { version: string }; + // The old serverInfo said "0.1.0" forever; this must track the real release instead. + expect(card.version).not.toBe("0.1.0"); + expect(card.version).toMatch(/^\d+\.\d+\.\d+/); + }); + + it.each(DISCOVERY_PATHS)("%s answers 304 for a matching if-none-match", async (path) => { + const first = await request(path); + const etag = first.headers.get("etag")!; + expect(etag).toMatch(/^W\//); + + const second = await request(path, { "if-none-match": etag }); + expect(second.status).toBe(304); + expect(await second.text()).toBe(""); + }); + + it("answers 200 again when the caller's tag is stale", async () => { + const response = await request("/.well-known/mcp.json", { "if-none-match": 'W/"stale"' }); + expect(response.status).toBe(200); + }); + + it("the agent-tools trio all describe the same catalog the card does", async () => { + const card = (await (await request("/.well-known/mcp.json")).json()) as { tools: Array<{ name: string }> }; + const index = (await (await request("/.well-known/agent-tools/index.json")).json()) as { tools: Array<{ name: string }> }; + const openai = (await (await request("/.well-known/agent-tools/openai.json")).json()) as { tools: Array<{ function: { name: string } }> }; + const anthropic = (await (await request("/.well-known/agent-tools/anthropic.json")).json()) as { tools: Array<{ name: string }> }; + + const expected = card.tools.map((tool) => tool.name); + expect(index.tools.map((tool) => tool.name)).toEqual(expected); + expect(openai.tools.map((tool) => tool.function.name)).toEqual(expected); + expect(anthropic.tools.map((tool) => tool.name)).toEqual(expected); + }); + + it("every agent-tools document points a caller at /mcp tools/call", async () => { + for (const path of DISCOVERY_PATHS.filter((candidate) => candidate.includes("agent-tools"))) { + const document = (await (await request(path)).json()) as { executor: { url: string; method: string } }; + expect(document.executor.url).toBe("https://api.loopover.ai/mcp"); + expect(document.executor.method).toBe("tools/call"); + } + }); + + it("advertises the origin the request ARRIVED on when no public origin is configured", async () => { + // A self-host deployment answers on its own hostname; the card must not advertise the cloud's. The test + // env presets PUBLIC_API_ORIGIN, so this clears it to reach the request-origin fallback. + const response = await app.fetch(new Request("https://selfhost.example/.well-known/mcp.json"), { + ...createTestEnv(), + PUBLIC_API_ORIGIN: undefined, + } as unknown as Env); + const card = (await response.json()) as { remotes: Array<{ url: string }> }; + expect(card.remotes[0]!.url).toBe("https://selfhost.example/mcp"); + }); + + it("prefers the configured public origin over the request's", async () => { + const response = await app.fetch( + new Request("https://internal.example/.well-known/mcp.json"), + createTestEnv({ PUBLIC_API_ORIGIN: "https://api.loopover.ai" }), + ); + const card = (await response.json()) as { remotes: Array<{ url: string }> }; + expect(card.remotes[0]!.url).toBe("https://api.loopover.ai/mcp"); + }); +}); diff --git a/test/unit/check-server-manifest-script.test.ts b/test/unit/check-server-manifest-script.test.ts new file mode 100644 index 000000000..b67e03e59 --- /dev/null +++ b/test/unit/check-server-manifest-script.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from "vitest"; +import { WATCHED_PATHS, checkServerManifest, checkWatchedPaths, collectProblems } from "../../scripts/check-server-manifest"; + +// #9526: the manifest check, and specifically its ANTI-ROT half. +// +// metagraphed's version-sync workflow watched a path that had been renamed away and kept passing for months +// while doing nothing. A workflow that can quietly watch nothing is worse than no workflow, so the guard +// itself needs a test that proves it fails when a watched path disappears. + +const MCP_PACKAGE = JSON.stringify({ name: "@loopover/mcp", version: "3.15.2" }); + +function manifest(overrides: Record = {}): string { + return JSON.stringify({ + name: "io.github.JSONbored/loopover", + description: "LoopOver's contribution-intelligence tools.", + repository: { url: "https://github.com/JSONbored/loopover", source: "github" }, + version: "3.15.2", + remotes: [{ type: "streamable-http", url: "https://api.loopover.ai/mcp" }], + packages: [{ registry_type: "npm", identifier: "@loopover/mcp", version: "3.15.2", transport: { type: "stdio" } }], + ...overrides, + }); +} + +describe("the anti-rot path guard (#9526)", () => { + it("passes when every watched path exists", () => { + expect(checkWatchedPaths({ exists: () => true })).toEqual([]); + }); + + it.each(WATCHED_PATHS)("FAILS LOUDLY when %s has been renamed away", (missing) => { + const problems = checkWatchedPaths({ exists: (path) => path !== missing }); + expect(problems).toHaveLength(1); + expect(problems[0]!.detail).toContain(missing); + expect(problems[0]!.detail, "the message must say WHY this matters").toContain("rename"); + }); + + it("reports the rot and STOPS, rather than validating fields against a file that is not there", () => { + const problems = collectProblems({ + exists: () => false, + readFile: () => { + throw new Error("must not read a file the guard already reported missing"); + }, + }); + expect(problems.length).toBe(WATCHED_PATHS.length); + expect(problems.every((problem) => problem.field === "watched-path")).toBe(true); + }); + + it("the real repository satisfies both halves", () => { + expect(collectProblems()).toEqual([]); + }); +}); + +describe("manifest field validation (#9526)", () => { + it("accepts a manifest that agrees with the shipped package", () => { + expect(checkServerManifest(manifest(), MCP_PACKAGE)).toEqual([]); + }); + + it("REJECTS a version that disagrees with @loopover/mcp — release automation owns it", () => { + // The whole point: publishing must never advertise a version nobody released. + const problems = checkServerManifest(manifest({ version: "9.9.9" }), MCP_PACKAGE); + expect(problems.map((problem) => problem.field)).toContain("version"); + expect(problems.find((problem) => problem.field === "version")!.detail).toContain("3.15.2"); + }); + + it("REJECTS an npm package version that drifts from the top-level one", () => { + const drifted = manifest({ packages: [{ registry_type: "npm", identifier: "@loopover/mcp", version: "3.0.0", transport: { type: "stdio" } }] }); + expect(checkServerManifest(drifted, MCP_PACKAGE).map((problem) => problem.field)).toContain("packages[0].version"); + }); + + it.each([ + ["a wrong server name", { name: "io.github.someone/else" }, "name"], + ["an empty description", { description: " " }, "description"], + ["a wrong repository", { repository: { url: "https://example.com", source: "gitlab" } }, "repository"], + ["a non-streamable remote", { remotes: [{ type: "sse", url: "https://api.loopover.ai/mcp" }] }, "remotes[0]"], + ["a remote pointing elsewhere", { remotes: [{ type: "streamable-http", url: "https://evil.example/mcp" }] }, "remotes[0]"], + ["a non-npm package entry", { packages: [{ registry_type: "pypi", identifier: "@loopover/mcp", version: "3.15.2", transport: { type: "stdio" } }] }, "packages[0]"], + ["a non-stdio transport", { packages: [{ registry_type: "npm", identifier: "@loopover/mcp", version: "3.15.2", transport: { type: "http" } }] }, "packages[0].transport"], + ])("rejects %s", (_label, overrides, field) => { + expect(checkServerManifest(manifest(overrides), MCP_PACKAGE).map((problem) => problem.field)).toContain(field); + }); + + it("rejects a SECOND remote — a second front door is one nobody tests", () => { + const twoRemotes = manifest({ + remotes: [ + { type: "streamable-http", url: "https://api.loopover.ai/mcp" }, + { type: "streamable-http", url: "https://staging.loopover.ai/mcp" }, + ], + }); + expect(checkServerManifest(twoRemotes, MCP_PACKAGE).map((problem) => problem.field)).toContain("remotes"); + }); + + it("reports malformed JSON as one clear problem rather than throwing", () => { + const problems = checkServerManifest("{not json", MCP_PACKAGE); + expect(problems).toHaveLength(1); + expect(problems[0]!.field).toBe("server.json"); + }); + + it("reports a manifest missing its sections entirely", () => { + const problems = checkServerManifest("{}", MCP_PACKAGE); + expect(problems.map((problem) => problem.field).sort()).toEqual( + ["description", "name", "packages[0]", "packages[0].transport", "remotes", "remotes[0]", "repository", "version"].sort(), + ); + }); +}); diff --git a/test/unit/mcp-discovery-surfaces.test.ts b/test/unit/mcp-discovery-surfaces.test.ts new file mode 100644 index 000000000..8d6e69fc9 --- /dev/null +++ b/test/unit/mcp-discovery-surfaces.test.ts @@ -0,0 +1,218 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { + SERVER_CARD_NAME, + buildAgentToolsIndex, + buildAnthropicTools, + buildOpenAiTools, + buildServerCard, + matchesETag, + weakETag, +} from "@loopover/contract/discovery"; +import { listToolDefinitions } from "@loopover/contract/tools"; +import { + DISCOVERY_PATHS, + buildDiscoveryDocuments, + deterministicGeneratedAt, + discoveryDocumentsFor, + resetDiscoveryCacheForTesting, + respondWithDocument, + toolsForDeployment, +} from "../../src/mcp/discovery-routes"; + +// #9526: the discovery surfaces are COMPUTED, never committed — metagraphed's committed server card made +// every concurrent tool PR conflict on one generated file. So there is no golden fixture to compare here; +// what these pin are the properties that make the computed answer trustworthy: it describes the deployment +// 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 }; + +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)); + + // 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. + const cloudOnly = listToolDefinitions().filter((tool) => tool.availability === "cloud").map((tool) => tool.name); + const selfhostOnly = listToolDefinitions().filter((tool) => tool.availability === "selfhost").map((tool) => tool.name); + expect(cloudOnly.length, "the fixture needs at least one cloud-only tool to be meaningful").toBeGreaterThan(0); + expect(selfhostOnly.length).toBeGreaterThan(0); + + for (const name of cloudOnly) expect(selfhost.has(name), `${name} is cloud-only`).toBe(false); + for (const name of selfhostOnly) expect(cloud.has(name), `${name} is selfhost-only`).toBe(false); + }); + + 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)); + for (const name of shared) { + expect(cloud.has(name)).toBe(true); + expect(selfhost.has(name)).toBe(true); + } + }); + + it("does NOT filter by locality — a local-git tool is still part of the catalog a client discovers", () => { + // The remote serves them too; it just expects the caller to supply the branch metadata rather than + // 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)); + for (const name of localGit) expect(cloud.has(name)).toBe(true); + }); +}); + +describe("the server card (#9526)", () => { + it("names the registry identity, the version, and the streamable-http remote", () => { + const card = buildServerCard({ ...CONTEXT, generatedAt: deterministicGeneratedAt(CONTEXT.version) }); + expect(card.name).toBe(SERVER_CARD_NAME); + expect(card.version).toBe("3.15.2"); + expect(card.remotes).toEqual([{ type: "streamable-http", url: "https://api.loopover.ai/mcp" }]); + expect(card.deployment).toBe("cloud"); + }); + + it("trims a trailing slash off the base URL rather than emitting a doubled path", () => { + const card = buildServerCard({ ...CONTEXT, baseUrl: "https://api.loopover.ai/", generatedAt: "x" }); + expect(card.remotes[0]!.url).toBe("https://api.loopover.ai/mcp"); + }); + + it("lists every tool with the catalog fields a client picks by", () => { + const card = buildServerCard({ ...CONTEXT, generatedAt: "x" }); + expect(card.tools.length).toBe(TOOLS.length); + const first = card.tools[0]!; + expect(Object.keys(first).sort()).toEqual(["annotations", "category", "description", "name", "title"]); + }); + + it("declares listChanged, since the gateway re-mounts on that notification", () => { + expect(buildServerCard({ ...CONTEXT, generatedAt: "x" }).capabilities.tools.listChanged).toBe(true); + }); +}); + +describe("the agent-tools trio (#9526)", () => { + const input = { baseUrl: "https://api.loopover.ai", tools: TOOLS, generatedAt: "x" }; + + it("every document carries an executor, so the catalog is actionable rather than a list", () => { + for (const document of [buildAgentToolsIndex(input), buildOpenAiTools(input), buildAnthropicTools(input)]) { + expect(document.executor).toEqual({ transport: "streamable-http", url: "https://api.loopover.ai/mcp", method: "tools/call" }); + } + }); + + it("the index carries BOTH schemas verbatim from the registry", () => { + const index = buildAgentToolsIndex(input); + expect(index.tools.length).toBe(TOOLS.length); + expect(index.tools[0]!.input_schema).toEqual(TOOLS[0]!.inputSchema); + expect(index.tools[0]!.output_schema).toEqual(TOOLS[0]!.outputSchema); + }); + + it("the OpenAI projection is the registry's input schema under `parameters` — no second translation", () => { + const openai = buildOpenAiTools(input); + expect(openai.tools[0]!.type).toBe("function"); + expect(openai.tools[0]!.function.parameters).toEqual(TOOLS[0]!.inputSchema); + expect(openai.tools[0]!.function.name).toBe(TOOLS[0]!.name); + }); + + it("the Anthropic projection is the same schema under `input_schema`", () => { + const anthropic = buildAnthropicTools(input); + expect(anthropic.tools[0]!.input_schema).toEqual(TOOLS[0]!.inputSchema); + expect(anthropic.tools[0]!.name).toBe(TOOLS[0]!.name); + }); + + it("all three describe the SAME tool set — one contract, three shapes", () => { + const names = (list: Array<{ name?: string; function?: { name: string } }>) => list.map((entry) => entry.name ?? entry.function!.name); + expect(names(buildOpenAiTools(input).tools)).toEqual(names(buildAgentToolsIndex(input).tools)); + expect(names(buildAnthropicTools(input).tools)).toEqual(names(buildAgentToolsIndex(input).tools)); + }); +}); + +describe("caching (#9526)", () => { + it("generated_at is derived from the VERSION, not the clock", () => { + // A wall-clock value would change the body every request, changing the ETag with it and making the 304 + // path dead code. + expect(deterministicGeneratedAt("3.15.2")).toBe("version:3.15.2"); + const first = buildDiscoveryDocuments(CONTEXT); + const second = buildDiscoveryDocuments(CONTEXT); + for (const path of DISCOVERY_PATHS) expect(second[path]!.etag).toBe(first[path]!.etag); + }); + + it("a version bump changes every ETag, so a deploy invalidates the caches", () => { + const before = buildDiscoveryDocuments(CONTEXT); + const after = buildDiscoveryDocuments({ ...CONTEXT, version: "3.16.0" }); + for (const path of DISCOVERY_PATHS) expect(after[path]!.etag).not.toBe(before[path]!.etag); + }); + + it("weakETag is stable for equal bodies and differs for different ones", () => { + expect(weakETag("abc")).toBe(weakETag("abc")); + expect(weakETag("abc")).not.toBe(weakETag("abd")); + expect(weakETag("")).toMatch(/^W\/"[0-9a-f]+"$/); + }); + + it.each([ + ['the exact tag', (etag: string) => etag], + ['the tag without its weak prefix', (etag: string) => etag.replace(/^W\//, "")], + ['a list containing it', (etag: string) => `W/"other", ${etag}`], + ['a wildcard', () => "*"], + ])("matchesETag accepts %s", (_label, build) => { + const etag = weakETag("body"); + expect(matchesETag(build(etag), etag)).toBe(true); + }); + + it.each([ + ["a different tag", 'W/"deadbeef"'], + ["an empty header", ""], + ["no header at all", null], + ])("matchesETag rejects %s", (_label, header) => { + expect(matchesETag(header, weakETag("body"))).toBe(false); + }); +}); + +describe("the HTTP response (#9526)", () => { + it("answers 200 with the document, its ETag, and a revalidating cache policy", async () => { + const documents = buildDiscoveryDocuments(CONTEXT); + const response = respondWithDocument(documents["/.well-known/mcp.json"]!, null); + expect(response.status).toBe(200); + expect(response.headers.get("etag")).toBe(documents["/.well-known/mcp.json"]!.etag); + expect(response.headers.get("cache-control")).toContain("must-revalidate"); + expect(JSON.parse(await response.text()).name).toBe(SERVER_CARD_NAME); + }); + + it("answers 304 with NO body when the caller already has the entity", async () => { + const document = buildDiscoveryDocuments(CONTEXT)["/.well-known/mcp.json"]!; + const response = respondWithDocument(document, document.etag); + expect(response.status).toBe(304); + expect(await response.text()).toBe(""); + // The validators still travel so the cache can extend its entry. + expect(response.headers.get("etag")).toBe(document.etag); + }); + + it("answers 200 when the caller's tag is stale", () => { + const document = buildDiscoveryDocuments(CONTEXT)["/.well-known/mcp.json"]!; + expect(respondWithDocument(document, 'W/"stale"').status).toBe(200); + }); +}); + +describe("the per-origin memo (#9526)", () => { + it("returns the same object for a repeat request on one origin", () => { + expect(discoveryDocumentsFor(CONTEXT)).toBe(discoveryDocumentsFor(CONTEXT)); + }); + + it("keeps origins separate, so each advertises its OWN /mcp", () => { + const primary = discoveryDocumentsFor(CONTEXT); + const other = discoveryDocumentsFor({ ...CONTEXT, baseUrl: "https://self.example" }); + expect(other).not.toBe(primary); + expect(JSON.parse(other["/.well-known/mcp.json"]!.body).remotes[0].url).toBe("https://self.example/mcp"); + }); + + 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, + ); + expect(selfhost.deployment).toBe("selfhost"); + expect(selfhost.tools.length).not.toBe(cloud.tools.length); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 0a7f8d290..c14b5845e 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -27,6 +27,7 @@ export default defineConfig({ // first, it silently intercepted "@loopover/contract/tools" too and rewrote it to a bogus // "/tools" that resolved nowhere. Confirmed by reproducing the failure with a // throwaway probe test before reordering, not assumed from reading Vite's docs alone. + "@loopover/contract/discovery": new URL("./packages/loopover-contract/src/discovery.ts", import.meta.url).pathname, "@loopover/contract/enums": new URL("./packages/loopover-contract/src/enums.ts", import.meta.url).pathname, "@loopover/contract/tools": new URL("./packages/loopover-contract/src/tools/index.ts", import.meta.url).pathname, "@loopover/contract/cli-config": new URL("./packages/loopover-contract/src/cli-config.ts", import.meta.url).pathname, From 6e1026a2178ce5fd66ff4ef171316333c3e936a1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:44:48 -0700 Subject: [PATCH 2/6] feat(mcp): mount the remote tool set from the stdio server (#9526) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stdio server now discovers the remote server's tools/list with the login session and registers anything it does not already serve, so one config gets a contributor every tool their session entitles them to instead of one stdio config for local-git tools and a separate remote endpoint for the rest. The mount is best-effort by design. No session, no network, or a hostile response all leave a server that starts and lists its local tools plus one advisory resource explaining how to get the rest; --no-remote opts out to byte-identical pre-gateway behavior. Proxied tools re-use the contract registry's zod schemas rather than trusting a remote-supplied schema, so the proxy advertises exactly what the remote enforces, and carry _meta.transport="proxied" so telemetry can tell a proxied call from a local one. Name collisions cannot happen by construction — one registry, one entry per name, so a name is either local-git or remote — and validate:mcp now asserts that invariant. The mount still skips an already-registered name so a future violation degrades to "local wins" rather than crashing the server on a duplicate registration. --- packages/loopover-mcp/bin/loopover-mcp.ts | 119 ++++++++++++- packages/loopover-mcp/lib/gateway.ts | 129 ++++++++++++++ test/contract/validate-mcp.test.ts | 18 ++ test/unit/mcp-gateway-mount-inprocess.test.ts | 123 +++++++++++++ test/unit/mcp-gateway.test.ts | 168 ++++++++++++++++++ 5 files changed, 555 insertions(+), 2 deletions(-) create mode 100644 packages/loopover-mcp/lib/gateway.ts create mode 100644 test/unit/mcp-gateway-mount-inprocess.test.ts create mode 100644 test/unit/mcp-gateway.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 47e3c8dfa..1e0d47544 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -9,6 +9,15 @@ import { fileURLToPath } from "node:url"; import { CLI_RESPONSE_SCHEMAS, type ApiResponse, type ValidatedApiPath } from "@loopover/contract/api-schemas"; import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { + GATEWAY_DISABLED_ADVISORY, + discoverRemoteTools, + gatewayAdvisoryResource, + gatewayDisabled, + type GatewayFetch, + type GatewayMountResult, + type RemoteToolDescriptor, +} from "../lib/gateway.js"; import { buildFeasibilityVerdict, buildPrTextLint, buildGateDispositions, buildPublicPrBodyDraft } from "@loopover/engine"; // #9537: the plan-DAG state machine, formerly hand-copied into this file, untyped. import { applyStepResult, buildPlanDag, nextReadySteps, planProgress, validatePlanDag, type PlanDag } from "@loopover/engine"; @@ -832,6 +841,13 @@ export const server = new McpServer({ // Reads telemetryState() HERE on purpose: registerStdioTool's second parameter is the TOOL's config and // shadows the module-level `config`, so a read inside a nested function would silently see the wrong object. /* v8 ignore start -- thin registration glue; wrapStdioToolHandler covered by unit tests (#8690) */ +/** + * Every tool THIS server registers locally. Gateway mode reads it to skip a remote tool of the same name + * (#9526) -- the contract registry makes a collision impossible, but a duplicate registration would crash + * the server, so "local wins" is a cheaper failure than refusing to start. + */ +const locallyRegisteredToolNames = new Set(); + function registerStdioTool( name: string, handler: (input: TInput, extra?: unknown) => unknown, @@ -842,6 +858,7 @@ function registerStdioTool( ): void { const contract = getToolContract(name); if (!contract) throw new Error(`No @loopover/contract entry for stdio tool: ${name}`); + locallyRegisteredToolNames.add(name); server.registerTool( name, { @@ -2447,10 +2464,108 @@ server.registerPrompt( }), ); +/** The real transport for the discovery call; injected in tests so no test reaches the network. */ +const defaultGatewayFetch: GatewayFetch = async (url, init) => { + const response = await fetch(url, init as RequestInit); + return { ok: response.ok, status: response.status, json: () => response.json() }; +}; + +/** + * Re-register one remote tool as a passthrough proxy. + * + * Schemas come from the CONTRACT REGISTRY, not from the wire. The SDK's registerTool takes zod, while + * tools/list carries JSON Schema, so the wire shape cannot be handed over as-is -- and reaching for the + * registry is the better answer anyway: it is the same source the remote registered from, so the proxy + * advertises exactly what the remote enforces without this package trusting a remote-supplied schema. A + * tool absent from the registry (a remote running ahead of this package) still mounts, with the remote as + * the only validator -- which is where validation belongs regardless. + * + * `_meta.transport` marks the tool so telemetry can tell a proxied call from a local one (#9526 requirement + * 6) without having to know which names are remote. + */ +function registerProxiedTool(tool: RemoteToolDescriptor): void { + const contract = getToolContract(tool.name); + server.registerTool( + tool.name, + { + ...(tool.title ? { title: tool.title } : {}), + ...(tool.description ? { description: tool.description } : {}), + ...(contract ? { inputSchema: contract.input.shape, outputSchema: contract.output } : {}), + ...(tool.annotations ? { annotations: tool.annotations as never } : {}), + _meta: { ...(tool._meta ?? {}), transport: "proxied" }, + } as never, + (async (input: unknown) => { + // Forwarded verbatim to the remote's own tools/call: this layer routes, it does not interpret. + const payload = await apiPost("/mcp", { jsonrpc: "2.0", id: Date.now(), method: "tools/call", params: { name: tool.name, arguments: input } }); + const result = (payload as { result?: unknown }).result; + return (result ?? payload) as never; + }) as never, + ); +} + +/** Registered at most once: a re-mount replaces the advisory's CONTENT, not the registration. */ +let gatewayAdvisoryState: { status: string; advisory: string } | null = null; +let gatewayAdvisoryRegistered = false; + +/** The advisory a client sees instead of remote tools. A resource, never an error -- see gateway.ts. */ +function registerGatewayAdvisory(result: GatewayMountResult): void { + const advisory = gatewayAdvisoryResource(result); + if (!advisory) return; + gatewayAdvisoryState = advisory; + if (gatewayAdvisoryRegistered) return; + gatewayAdvisoryRegistered = true; + server.registerResource( + "loopover_gateway_status", + "loopover://gateway/status", + { title: "LoopOver gateway status", description: "Why remote tools are or are not mounted on this stdio server.", mimeType: "application/json" }, + async () => ({ contents: [{ uri: "loopover://gateway/status", mimeType: "application/json", text: JSON.stringify(gatewayAdvisoryState, null, 2) }] }), + ); +} + +/** + * Gateway mode (#9526): mount the REMOTE server's tools onto this stdio server. + * + * One stdio entry, every tool the caller's session entitles them to -- rather than a local config for the + * local-git tools and a second, separate remote entry for the rest. Each proxied tool re-registers with the + * schemas that came across the wire, so nothing is duplicated here and a remote schema change needs no + * release of this package. + * + * Every failure is non-fatal by design. No session, no network, a 500 from the API -- all of them leave a + * working local server plus one advisory resource. An enhancement that can break startup is a liability, and + * a contributor on a plane still needs their local tools. + */ +export async function mountRemoteTools(options: { argv?: readonly string[]; fetchImpl?: GatewayFetch } = {}): Promise { + const argv = options.argv ?? cliArgs; + if (gatewayDisabled(argv)) { + registerGatewayAdvisory({ status: "unavailable", advisory: GATEWAY_DISABLED_ADVISORY }); + return { status: "unavailable", advisory: GATEWAY_DISABLED_ADVISORY }; + } + + const result = await discoverRemoteTools({ + apiUrl, + token: getApiToken() ?? null, + // Local wins on a collision. The contract registry makes one impossible -- a name is local-git OR + // remote, never both -- but a duplicate registration would crash the server, so this degrades instead. + localToolNames: locallyRegisteredToolNames, + fetchImpl: options.fetchImpl ?? (defaultGatewayFetch as GatewayFetch), + }); + + if (result.status !== "mounted") { + registerGatewayAdvisory(result); + return result; + } + for (const tool of result.tools) registerProxiedTool(tool); + return result; +} + // #7764: only bind the shared stdin/stdout transport when actually launched as the CLI/stdio process. An // in-process unit-test importer holds the exported `server` and connects it to an in-memory transport instead. -/* v8 ignore next -- only the launched stdio process binds the real transport; unit tests connect in-memory. */ -if (runAsCliEntrypoint) await server.connect(new StdioServerTransport()); +/* v8 ignore next 4 -- only the launched stdio process mounts remote tools and binds the real transport; + unit tests call mountRemoteTools() directly and connect in-memory. */ +if (runAsCliEntrypoint) { + await mountRemoteTools(); + await server.connect(new StdioServerTransport()); +} async function withClientWorkspaceRoots(input: any) { return withWorkspaceRoots(input, await clientWorkspaceRoots()); diff --git a/packages/loopover-mcp/lib/gateway.ts b/packages/loopover-mcp/lib/gateway.ts new file mode 100644 index 000000000..9dd763c3a --- /dev/null +++ b/packages/loopover-mcp/lib/gateway.ts @@ -0,0 +1,129 @@ +// Gateway mode: the stdio server mounts the REMOTE server's tools (#9526). +// +// One stdio entry, every tool a session entitles you to. Without this a contributor configures the stdio +// server for local-git tools and separately points something at `https://api.loopover.ai/mcp` for the rest — +// two configs for what is one product. +// +// Name collisions cannot happen by construction, and that is a property of #9518's registry rather than a +// check here: post-migration a tool name is either locality `local-git` (served locally) or `remote` +// (proxied), never both, because there is ONE contract registry and one entry per name. `validate:mcp` +// asserts the invariant; this module relies on it and additionally skips anything already registered, so a +// future violation degrades to "local wins" instead of a duplicate-registration crash. +// +// Offline or unauthenticated is NOT an error. A contributor with no session, or on a plane, gets the local +// tools and one advisory — never a server that fails to start. That is the whole reason the mount is +// best-effort: the gateway is an enhancement, and an enhancement that can break startup is a liability. + +/** The subset of a remote `tools/list` entry this needs. Schemas come across the wire; nothing is duplicated. */ +export type RemoteToolDescriptor = { + name: string; + title?: string; + description?: string; + inputSchema?: unknown; + outputSchema?: unknown; + annotations?: Record; + _meta?: Record; +}; + +export type GatewayFetch = ( + url: string, + init: { method: string; headers: Record; body: string; signal?: AbortSignal }, +) => Promise<{ ok: boolean; status: number; json: () => Promise }>; + +export type GatewayMountInput = { + apiUrl: string; + /** The session token from `loopover-mcp login`. Absent ⇒ local tools only, no request attempted. */ + token: string | null; + /** Names the local server already serves. A remote tool with one of these names is skipped, never replaced. */ + localToolNames: ReadonlySet; + fetchImpl: GatewayFetch; + /** Bounds the discovery call so a slow or hanging remote cannot delay stdio startup. */ + timeoutMs?: number; +}; + +export type GatewayMountResult = + | { status: "mounted"; tools: RemoteToolDescriptor[]; skipped: string[] } + | { status: "unauthenticated"; advisory: string } + | { status: "unavailable"; advisory: string }; + +export const GATEWAY_UNAUTHENTICATED_ADVISORY = + "Remote tools are not mounted: no LoopOver session on this machine. Run `loopover-mcp login` and restart your client to use the full tool set. Local tools are available now."; + +export const GATEWAY_DISABLED_ADVISORY = + "Remote tools are not mounted: gateway mode was disabled with --no-remote. Local tools are available; drop the flag and restart your client to mount the remote tool set."; + +function unavailableAdvisory(reason: string): string { + return `Remote tools are not mounted: ${reason}. Local tools are available now; they will mount on the next start once the API is reachable.`; +} + +const DEFAULT_TIMEOUT_MS = 5_000; + +/** + * Discover the remote tool set for this session. + * + * Every failure path lands on `unavailable` with a readable reason rather than throwing: an offline + * contributor must still get a working stdio server. A non-2xx is reported by status because the body is + * remote-controlled and has no business in a local advisory string. + */ +export async function discoverRemoteTools(input: GatewayMountInput): Promise { + if (!input.token) return { status: "unauthenticated", advisory: GATEWAY_UNAUTHENTICATED_ADVISORY }; + + let response: Awaited>; + try { + response = await input.fetchImpl(`${input.apiUrl.replace(/\/+$/, "")}/mcp`, { + method: "POST", + headers: { + authorization: `Bearer ${input.token}`, + "content-type": "application/json", + accept: "application/json, text/event-stream", + }, + body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/list", params: {} }), + signal: AbortSignal.timeout(input.timeoutMs ?? DEFAULT_TIMEOUT_MS), + }); + } catch (error) { + return { status: "unavailable", advisory: unavailableAdvisory(`the API was unreachable (${error instanceof Error ? error.message : String(error)})`) }; + } + + if (!response.ok) { + return { status: "unavailable", advisory: unavailableAdvisory(`the API answered ${response.status}`) }; + } + + const payload = (await response.json().catch(() => null)) as { result?: { tools?: unknown } } | null; + const tools = payload?.result?.tools; + if (!Array.isArray(tools)) { + return { status: "unavailable", advisory: unavailableAdvisory("the API returned no tool list") }; + } + + const mounted: RemoteToolDescriptor[] = []; + const skipped: string[] = []; + for (const candidate of tools) { + if (!candidate || typeof candidate !== "object") continue; + const tool = candidate as RemoteToolDescriptor; + if (typeof tool.name !== "string" || tool.name === "") continue; + // Local wins. The registry makes this unreachable, but a duplicate registration would crash the server, + // and degrading to "the local one" is strictly better than refusing to start. + if (input.localToolNames.has(tool.name)) { + skipped.push(tool.name); + continue; + } + mounted.push(tool); + } + return { status: "mounted", tools: mounted, skipped }; +} + +/** `--no-remote` gives byte-identical pre-gateway behavior, for anyone who wants only local tools. */ +export function gatewayDisabled(argv: readonly string[]): boolean { + return argv.includes("--no-remote"); +} + +/** + * The advisory a client sees when remote tools are not mounted, as a resource rather than an error. + * + * A missing session is a normal state, not a failure, so it must never surface as a tool error or a + * non-zero exit — the client would render a broken server where the honest answer is "you have the local + * half, here is how to get the rest". + */ +export function gatewayAdvisoryResource(result: GatewayMountResult): { status: string; advisory: string } | null { + if (result.status === "mounted") return null; + return { status: result.status, advisory: result.advisory }; +} diff --git a/test/contract/validate-mcp.test.ts b/test/contract/validate-mcp.test.ts index 9b29ddb67..a23f8d7aa 100644 --- a/test/contract/validate-mcp.test.ts +++ b/test/contract/validate-mcp.test.ts @@ -240,6 +240,24 @@ describe("MCP contract validator (#9520)", () => { } }, 180_000); + it("REGRESSION: one tool name has ONE locality, which is what makes gateway collisions impossible", () => { + // #9526's gateway mounts every `remote` tool onto the stdio server, which serves the `local-git` ones. + // That is only safe because a NAME belongs to exactly one entry in the one registry — the same name + // registered at both localities would mean the gateway tries to register a tool the stdio server + // already has, and the SDK throws on a duplicate. The registry structurally prevents it (one entry per + // name); this asserts nothing has introduced a second. + const byName = new Map(); + for (const tool of listToolDefinitions()) { + byName.set(tool.name, [...(byName.get(tool.name) ?? []), tool.locality]); + } + const ambiguous = [...byName.entries()].filter(([, localities]) => new Set(localities).size > 1); + expect(ambiguous.map(([name]) => name)).toEqual([]); + + // And no name is declared twice at all, whatever its locality. + const duplicated = [...byName.entries()].filter(([, localities]) => localities.length > 1); + expect(duplicated.map(([name]) => name)).toEqual([]); + }); + it("locks the published MCP version across the three places it appears", () => { const packageVersion = (JSON.parse(readFileSync(join(process.cwd(), "packages/loopover-mcp/package.json"), "utf8")) as { version: string }).version; expect( diff --git a/test/unit/mcp-gateway-mount-inprocess.test.ts b/test/unit/mcp-gateway-mount-inprocess.test.ts new file mode 100644 index 000000000..09438f833 --- /dev/null +++ b/test/unit/mcp-gateway-mount-inprocess.test.ts @@ -0,0 +1,123 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { closeFixtureServer, startFixtureServer } from "./support/mcp-cli-harness"; +import type { GatewayFetch, GatewayMountResult } from "../../packages/loopover-mcp/lib/gateway"; + +// #9526: gateway mounting against the REAL stdio server, in-process. +// +// gateway.ts's discovery is unit-tested on its own; this covers the wiring the bin owns — that a mounted +// remote tool is actually registered and callable, that it is tagged as proxied so telemetry can tell it +// from a local call, and that the failure paths leave a server which still lists its local tools. + +const MODULE = "../../packages/loopover-mcp/bin/loopover-mcp.ts"; + +type BinModule = { + server: { connect: (transport: unknown) => Promise }; + mountRemoteTools: (options?: { argv?: readonly string[]; fetchImpl?: GatewayFetch }) => Promise; +}; + +let tempDir = ""; +let mod: BinModule; + +beforeAll(async () => { + tempDir = mkdtempSync(join(tmpdir(), "loopover-gateway-mount-")); + const apiUrl = await startFixtureServer(); + process.env.LOOPOVER_API_URL = apiUrl; + process.env.LOOPOVER_API_TOKEN = "in-process-token"; + process.env.LOOPOVER_API_TIMEOUT_MS = "2000"; + process.env.LOOPOVER_CONFIG_DIR = tempDir; + process.env.LOOPOVER_SKIP_NPM_VERSION_CHECK = "1"; + mod = (await import(MODULE)) as unknown as BinModule; +}, 120_000); + +afterAll(async () => { + await closeFixtureServer(); + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + for (const key of ["LOOPOVER_API_URL", "LOOPOVER_API_TOKEN", "LOOPOVER_API_TIMEOUT_MS", "LOOPOVER_CONFIG_DIR", "LOOPOVER_SKIP_NPM_VERSION_CHECK"]) { + delete process.env[key]; + } +}); + +async function connect(name: string) { + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await mod.server.connect(serverTransport); + const client = new Client({ name, version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +function remoteToolsFetch(tools: unknown[]): GatewayFetch { + return vi.fn(async () => ({ ok: true, status: 200, json: async () => ({ result: { tools } }) })); +} + +describe("mountRemoteTools against the real server (#9526)", () => { + it("--no-remote mounts nothing and never calls out", async () => { + const fetchImpl = remoteToolsFetch([]); + const result = await mod.mountRemoteTools({ argv: ["--stdio", "--no-remote"], fetchImpl }); + expect(result.status).toBe("unavailable"); + expect(fetchImpl, "the opt-out must be byte-identical to pre-gateway behavior").not.toHaveBeenCalled(); + }); + + it("mounts a remote tool and makes it callable and discoverable", async () => { + const result = await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([ + { + name: "loopover_gateway_probe", + title: "Gateway probe", + description: "A remote-only tool, mounted through the gateway.", + inputSchema: { type: "object", properties: {}, additionalProperties: false }, + }, + ]), + }); + expect(result.status).toBe("mounted"); + + const client = await connect("gateway-mount-test"); + try { + const { tools } = await client.listTools(); + const proxied = tools.find((tool) => tool.name === "loopover_gateway_probe"); + expect(proxied, "a mounted tool must appear in tools/list").toBeDefined(); + // Tagged so telemetry can distinguish a proxied call from a local one (#9526 requirement 6) without + // having to know which names are remote. + expect((proxied!._meta as { transport?: string } | undefined)?.transport).toBe("proxied"); + } finally { + await client.close(); + } + }); + + it("SKIPS a remote tool the local server already serves, leaving the local one in place", async () => { + const before = await connect("gateway-collision-before"); + const localNames = new Set((await before.listTools()).tools.map((tool) => tool.name)); + await before.close(); + const collidingName = [...localNames].find((name) => name.startsWith("loopover_"))!; + + const result = await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([{ name: collidingName, description: "remote impostor" }]), + }); + // The registry makes a real collision impossible; degrading beats crashing on duplicate registration. + expect(result.status).toBe("mounted"); + expect(result.status === "mounted" && result.skipped).toContain(collidingName); + }); + + it("an unreachable API leaves a server that still lists its LOCAL tools", async () => { + const fetchImpl = vi.fn(async () => { + throw new Error("ECONNREFUSED"); + }) as unknown as GatewayFetch; + const result = await mod.mountRemoteTools({ argv: ["--stdio"], fetchImpl }); + expect(result.status).toBe("unavailable"); + + const client = await connect("gateway-offline-test"); + try { + const { tools } = await client.listTools(); + // The whole posture: an enhancement that can break startup is a liability. + expect(tools.length).toBeGreaterThan(10); + } finally { + await client.close(); + } + }); +}); diff --git a/test/unit/mcp-gateway.test.ts b/test/unit/mcp-gateway.test.ts new file mode 100644 index 000000000..1022010ba --- /dev/null +++ b/test/unit/mcp-gateway.test.ts @@ -0,0 +1,168 @@ +import { describe, expect, it, vi } from "vitest"; +import { + GATEWAY_DISABLED_ADVISORY, + GATEWAY_UNAUTHENTICATED_ADVISORY, + discoverRemoteTools, + gatewayAdvisoryResource, + gatewayDisabled, + type GatewayFetch, + type RemoteToolDescriptor, +} from "../../packages/loopover-mcp/lib/gateway"; + +// #9526: gateway discovery. The governing rule is that NONE of this may break a stdio server — a missing +// session, a dead network, or a hostile response all leave the local tools working plus one advisory. An +// enhancement that can break startup is a liability, so every failure path here is asserted to be +// non-throwing and to say something a contributor can act on. + +const REMOTE_TOOLS: RemoteToolDescriptor[] = [ + { name: "loopover_get_decision_pack", title: "Decision pack", description: "…", inputSchema: { type: "object" } }, + { name: "loopover_preflight_pr", title: "Preflight", description: "…", inputSchema: { type: "object" } }, +]; + +function respondingWith(payload: unknown, init: { ok?: boolean; status?: number } = {}): GatewayFetch { + return vi.fn(async () => ({ ok: init.ok ?? true, status: init.status ?? 200, json: async () => payload })); +} + +function input(overrides: Partial[0]> = {}) { + return { + apiUrl: "https://api.loopover.ai", + token: "session-token", + localToolNames: new Set(), + fetchImpl: respondingWith({ result: { tools: REMOTE_TOOLS } }), + ...overrides, + }; +} + +describe("discovery requires a session (#9526)", () => { + it("reports unauthenticated WITHOUT making a request", async () => { + const fetchImpl = respondingWith({ result: { tools: [] } }); + const result = await discoverRemoteTools(input({ token: null, fetchImpl })); + expect(result.status).toBe("unauthenticated"); + expect(fetchImpl, "no session means nothing to authenticate with — do not call out").not.toHaveBeenCalled(); + }); + + it("the unauthenticated advisory tells a contributor exactly what to do", async () => { + const result = await discoverRemoteTools(input({ token: null })); + expect(result.status === "unauthenticated" && result.advisory).toBe(GATEWAY_UNAUTHENTICATED_ADVISORY); + expect(GATEWAY_UNAUTHENTICATED_ADVISORY).toContain("loopover-mcp login"); + expect(GATEWAY_UNAUTHENTICATED_ADVISORY, "and must say the local half still works").toContain("Local tools are available"); + }); +}); + +describe("a successful mount (#9526)", () => { + it("returns the remote tools and calls the remote's own tools/list with the session bearer", async () => { + const fetchImpl = respondingWith({ result: { tools: REMOTE_TOOLS } }); + const result = await discoverRemoteTools(input({ fetchImpl })); + expect(result.status).toBe("mounted"); + expect(result.status === "mounted" && result.tools.map((tool) => tool.name)).toEqual([ + "loopover_get_decision_pack", + "loopover_preflight_pr", + ]); + + const [url, init] = (fetchImpl as unknown as { mock: { calls: [string, { headers: Record; body: string }][] } }).mock.calls[0]!; + expect(url).toBe("https://api.loopover.ai/mcp"); + expect(init.headers.authorization).toBe("Bearer session-token"); + expect(JSON.parse(init.body).method).toBe("tools/list"); + }); + + it("trims a trailing slash rather than requesting a doubled path", async () => { + const fetchImpl = respondingWith({ result: { tools: [] } }); + await discoverRemoteTools(input({ apiUrl: "https://api.loopover.ai/", fetchImpl })); + expect((fetchImpl as unknown as { mock: { calls: [string][] } }).mock.calls[0]![0]).toBe("https://api.loopover.ai/mcp"); + }); + + it("SKIPS a remote tool whose name the local server already serves — local wins", async () => { + // The registry makes this unreachable, but a duplicate registration would crash the stdio server, and + // degrading to the local tool is strictly better than refusing to start. + const result = await discoverRemoteTools(input({ localToolNames: new Set(["loopover_preflight_pr"]) })); + expect(result.status === "mounted" && result.tools.map((tool) => tool.name)).toEqual(["loopover_get_decision_pack"]); + expect(result.status === "mounted" && result.skipped).toEqual(["loopover_preflight_pr"]); + }); + + it("ignores malformed entries instead of mounting a tool with no name", async () => { + const fetchImpl = respondingWith({ result: { tools: [...REMOTE_TOOLS, null, "nope", { title: "no name" }, { name: "" }] } }); + const result = await discoverRemoteTools(input({ fetchImpl })); + expect(result.status === "mounted" && result.tools).toHaveLength(2); + }); +}); + +describe("every failure degrades to a working local server (#9526)", () => { + it("a transport error reports unavailable, naming the cause", async () => { + const fetchImpl = vi.fn(async () => { + throw new Error("ECONNREFUSED"); + }) as unknown as GatewayFetch; + const result = await discoverRemoteTools(input({ fetchImpl })); + expect(result.status).toBe("unavailable"); + expect(result.status === "unavailable" && result.advisory).toContain("ECONNREFUSED"); + }); + + it("a non-2xx reports the STATUS and does not echo the remote body", async () => { + // The body is remote-controlled; it has no business in a local advisory string. + const fetchImpl = respondingWith({ error: "internal", secretish: "do-not-echo" }, { ok: false, status: 503 }); + const result = await discoverRemoteTools(input({ fetchImpl })); + expect(result.status === "unavailable" && result.advisory).toContain("503"); + expect(result.status === "unavailable" && result.advisory).not.toContain("do-not-echo"); + }); + + it.each([ + ["a body with no result", {}], + ["a result with no tools", { result: {} }], + ["tools that are not an array", { result: { tools: "everything" } }], + ["a null body", null], + ])("reports unavailable for %s rather than throwing", async (_label, payload) => { + const result = await discoverRemoteTools(input({ fetchImpl: respondingWith(payload) })); + expect(result.status).toBe("unavailable"); + }); + + it("a body that will not parse is unavailable, not a crash", async () => { + const fetchImpl = vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => { + throw new Error("not json"); + }, + })) as unknown as GatewayFetch; + expect((await discoverRemoteTools(input({ fetchImpl }))).status).toBe("unavailable"); + }); + + it("every unavailable advisory says the local tools still work", async () => { + const result = await discoverRemoteTools(input({ fetchImpl: respondingWith({}) })); + expect(result.status === "unavailable" && result.advisory).toContain("Local tools are available"); + }); + + it("bounds the call so a hanging remote cannot delay stdio startup", async () => { + const fetchImpl = vi.fn(async () => ({ ok: true, status: 200, json: async () => ({ result: { tools: [] } }) })); + await discoverRemoteTools(input({ fetchImpl: fetchImpl as unknown as GatewayFetch, timeoutMs: 25 })); + const [, init] = (fetchImpl as unknown as { mock: { calls: [string, { signal?: AbortSignal }][] } }).mock.calls[0]!; + expect(init.signal, "an unbounded discovery call would hang the server's start").toBeDefined(); + }); +}); + +describe("--no-remote (#9526)", () => { + it.each([ + [["--stdio", "--no-remote"], true], + [["--no-remote"], true], + [["--stdio"], false], + [[], false], + ])("gatewayDisabled(%j) is %s", (argv, expected) => { + expect(gatewayDisabled(argv)).toBe(expected); + }); + + it("its advisory explains how to turn the gateway back on", () => { + expect(GATEWAY_DISABLED_ADVISORY).toContain("--no-remote"); + expect(GATEWAY_DISABLED_ADVISORY).toContain("Local tools are available"); + }); +}); + +describe("the advisory resource (#9526)", () => { + it("is null when tools mounted — there is nothing to advise about", () => { + expect(gatewayAdvisoryResource({ status: "mounted", tools: [], skipped: [] })).toBeNull(); + }); + + it.each([ + ["unauthenticated" as const, GATEWAY_UNAUTHENTICATED_ADVISORY], + ["unavailable" as const, "some reason"], + ])("carries the %s status and its advisory", (status, advisory) => { + expect(gatewayAdvisoryResource({ status, advisory })).toEqual({ status, advisory }); + }); +}); From 2701ea6a5c52294a3be9b96b8aee8225d4f9ec59 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:25:54 -0700 Subject: [PATCH 3/6] feat(mcp): derive every client-config surface and tag proxied calls (#9526) The three ways to connect -- the stdio gateway, the remote streamable-http endpoint, and the miner's own stdio server -- were spelled out by hand in four places: clientSnippet() in the stdio bin, the mcp-clients docs page, and both package READMEs. They had already drifted; the docs page documented no remote auth at all, and nothing described the gateway because the prose predates it. The grid now lives in @loopover/contract as data, `init-client --print` renders it, and a generator writes the docs and README blocks from it under a --check drift guard, so a snippet copied from the docs and one printed by the CLI are byte-identical. `init-client` gains `--mode stdio|remote|miner`, defaulting to stdio so a pre-gateway invocation prints exactly what it always did. A remote entry names the token's environment variable in whichever dialect the host reads -- Codex's own key, the JSON hosts' header -- and never carries a value; a host whose remote dialect this repo cannot vouch for is refused rather than given a plausible snippet that fails on paste. Telemetry gains a `transport` dimension, so gateway adoption is measurable: `surface` says which server was asked, and only `transport` says whether a stdio call ran against the local checkout or was forwarded. Proxied tools now run through the same telemetry chokepoint local ones do, and re-register with the contract's schema OBJECTS rather than their `.shape`, which the SDK re-wraps in a way that drops the catchall and turns every extra field into a -32602. The discovery routes are specced through the #9519 seam with real response schemas: the four documents are now zod schemas with their TypeScript types inferred, so the published operation describes the object actually served instead of a second declaration of it. They also report the deployment that ANSWERED -- src/server.ts serves this same Hono app, so a self-host card was advertising the cloud's tool set, which is a list of calls that 404 there. Fix-what-you-find, all three the same rot: the vitest alias list for @loopover/contract, the published-tarball allowlist, and the mount's startup cost. The first two are now derived from the package's own exports map and its committed sources -- both had just failed by lagging a new file, with an error naming the symptom and not the cause. The third: the mount moved after connect() and is no longer awaited, so a network round-trip no longer sits between a client launching the process and it answering, and registering a tool on a connected server emits notifications/tools/list_changed -- the refresh path a stateless remote cannot push itself. --- apps/loopover-ui/content/docs/mcp-clients.mdx | 209 ++++++++++++++-- package.json | 2 + packages/loopover-contract/package.json | 4 + .../loopover-contract/src/client-config.ts | 225 ++++++++++++++++++ packages/loopover-contract/src/discovery.ts | 82 +++++-- packages/loopover-contract/src/telemetry.ts | 19 ++ packages/loopover-mcp/README.md | 54 ++++- packages/loopover-mcp/bin/loopover-mcp.ts | 140 ++++++----- packages/loopover-mcp/lib/telemetry.ts | 9 +- packages/loopover-miner/README.md | 10 +- scripts/gen-mcp-client-config.ts | 173 ++++++++++++++ scripts/mcp-package-allowlist.ts | 28 ++- src/api/routes.ts | 10 +- src/openapi/discovery-route-specs.ts | 68 ++++++ src/openapi/spec.ts | 10 + test/integration/mcp-discovery-routes.test.ts | 23 +- test/unit/contract-client-config.test.ts | 141 +++++++++++ .../unit/gen-mcp-client-config-script.test.ts | 98 ++++++++ test/unit/mcp-cli-basics.test.ts | 33 +++ test/unit/mcp-discovery-surfaces.test.ts | 34 +++ test/unit/mcp-dispatch-telemetry.test.ts | 11 +- test/unit/mcp-gateway-mount-inprocess.test.ts | 38 +++ test/unit/mcp-gateway.test.ts | 10 + .../mcp-local-telemetry-chokepoint.test.ts | 4 +- test/unit/mcp-local-telemetry.test.ts | 36 +++ vitest.config.ts | 44 ++-- 26 files changed, 1393 insertions(+), 122 deletions(-) create mode 100644 packages/loopover-contract/src/client-config.ts create mode 100644 scripts/gen-mcp-client-config.ts create mode 100644 src/openapi/discovery-route-specs.ts create mode 100644 test/unit/contract-client-config.test.ts create mode 100644 test/unit/gen-mcp-client-config-script.test.ts diff --git a/apps/loopover-ui/content/docs/mcp-clients.mdx b/apps/loopover-ui/content/docs/mcp-clients.mdx index 7a7259815..00540ac75 100644 --- a/apps/loopover-ui/content/docs/mcp-clients.mdx +++ b/apps/loopover-ui/content/docs/mcp-clients.mdx @@ -4,9 +4,11 @@ description: Configure your coding agent to talk to the LoopOver MCP. Pick stdio eyebrow: Get started --- +{/* GENERATED:MCP-CLIENT-CONFIG:BEGIN — edit packages/loopover-contract/src/client-config.ts, then `npm run mcp:client-config` */} + ## Generate config -These commands print config only. They do not mutate your local client files. +Every block on this page is what `init-client` prints. It prints config only — it never edits your client files. -`--print mcp` uses the same JSON snippet as Claude Desktop and Cursor for other stdio MCP hosts -that expect the `mcpServers` shape. Every generated snippet assumes `loopover-mcp` is on your -`PATH` (install it globally first, per [Quickstart](/docs/quickstart)) — pass -`--command /absolute/path/to/loopover-mcp` if your client doesn't inherit your shell PATH. +## Local stdio (gateway) + +The recommended default. Runs `loopover-mcp` on your machine, keeps auth and git analysis local, and — once you have run `loopover-mcp login` — mounts the remote tool set too, so one entry serves every tool your session entitles you to. -## Codex (OpenAI) +- Run `loopover-mcp login` before starting the client; without a session you get the local-git tools only, plus an advisory resource explaining how to get the rest. +- Pass `--no-remote` to keep the server purely local and skip the remote mount entirely. +- Assumes `loopover-mcp` is on your PATH; pass `--command /absolute/path/to/loopover-mcp` if your client does not inherit your shell PATH. + +### Codex (OpenAI) (stdio) -## Claude Desktop +### Claude Desktop / Claude Code (stdio) -## Cursor +### Cursor (stdio) -## VS Code +### Other `mcpServers` hosts (stdio) + + -VS Code's native MCP support uses a `servers` map with an explicit transport type instead of the -`mcpServers` shape the other JSON hosts use: +### VS Code (stdio) -## Remote MCP +## Remote streamable-http + +For agents that run in the cloud, or anywhere you do not want a local Node process. Connects straight to the hosted server; the local-git tools are not available over this transport because there is no local checkout to read. + +- Authenticates with a bearer token read from `LOOPOVER_API_TOKEN` — the same variable the CLI honors. Set it in the environment your client starts in; never paste the token into the config file. +- Tools whose work is a local git operation are absent here by design. Use the stdio mode if you need them. + +### Codex (OpenAI) (remote) + + + +Codex releases before its RMCP client became the default also need `experimental_use_rmcp_client = true` at the top level of config.toml. + +### Claude Desktop / Claude Code (remote) + + + +### Cursor (remote) + + + +### VS Code (remote) + + + +## Miner stdio + +AMS's own local state-visibility tools, as a separate stdio server. It stays separate on purpose: it reads this machine's SQLite state and shares no code or network path with the hosted server. + +- Takes no flags and needs no login — everything it reads is already on this machine. +- A dual-role operator runs this alongside the stdio gateway; the two entries coexist in one client config. + +### Codex (OpenAI) (miner) + + + +### Claude Desktop / Claude Code (miner) + + + +### Cursor (miner) + + + +### Other `mcpServers` hosts (miner) + + + +### VS Code (miner) + + -The Worker also exposes a remote MCP endpoint. Use this when your agent runs in the cloud or you -don't want a local Node process. +Every block above comes from the same grid the CLI prints from, so a snippet copied from here and one printed by `init-client` can never disagree. - +{/* GENERATED:MCP-CLIENT-CONFIG:END */} Local `--stdio` is the default recommendation. It keeps auth + analysis on your machine and is the - easiest path to log into with GitHub Device Flow. + easiest path to log into with GitHub Device Flow — and, since it mounts the remote tool set too, it + is the only mode that gives you every tool from one entry. diff --git a/package.json b/package.json index d4f2f8030..09965efc6 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,8 @@ "loadtest:worker": "node --experimental-strip-types scripts/load-test-worker.ts", "mcp:tool-reference": "tsx scripts/gen-mcp-tool-reference.ts", "mcp:tool-reference:check": "tsx scripts/gen-mcp-tool-reference.ts --check", + "mcp:client-config": "tsx scripts/gen-mcp-client-config.ts", + "mcp:client-config:check": "tsx scripts/gen-mcp-client-config.ts --check", "contract:api-schemas": "tsx scripts/gen-contract-api-schemas.ts", "contract:api-schemas:check": "tsx scripts/gen-contract-api-schemas.ts --check", "command-reference": "node --experimental-strip-types scripts/gen-command-reference.ts", diff --git a/packages/loopover-contract/package.json b/packages/loopover-contract/package.json index ae939ddc7..89e8766ca 100644 --- a/packages/loopover-contract/package.json +++ b/packages/loopover-contract/package.json @@ -63,6 +63,10 @@ "./discovery": { "types": "./dist/discovery.d.ts", "default": "./dist/discovery.js" + }, + "./client-config": { + "types": "./dist/client-config.d.ts", + "default": "./dist/client-config.js" } }, "files": [ diff --git a/packages/loopover-contract/src/client-config.ts b/packages/loopover-contract/src/client-config.ts new file mode 100644 index 000000000..fd2c93257 --- /dev/null +++ b/packages/loopover-contract/src/client-config.ts @@ -0,0 +1,225 @@ +// The one description of how a client connects to LoopOver (#9526). +// +// Three connection modes -- the stdio gateway, the remote streamable-http endpoint, and the miner's own +// stdio server -- times five client hosts, each with its own config file, language, and server-map shape. +// That is a 14-cell grid, and before this it was written out by hand in four places: `clientSnippet()` in +// the stdio bin, the mcp-clients docs page, and both package READMEs. Each copy drifted on its own +// schedule; the docs page never mentioned the remote endpoint's auth at all. +// +// So the grid lives here, as data, and every surface renders it: `init-client --print` reads this, and +// scripts/gen-mcp-client-config.ts writes the docs and README blocks from it under a --check drift guard. +// Adding a host or a mode is one entry in this file, and every surface that describes it moves together. +import { DEFAULT_LOOPOVER_API_URL } from "./cli-config.js"; + +/** How a client reaches LoopOver. Ordered as a reader should consider them: local default first. */ +export const CONNECTION_MODES = ["stdio", "remote", "miner"] as const; +export type ConnectionMode = (typeof CONNECTION_MODES)[number]; + +/** The MCP client hosts `init-client` can print for. */ +export const CLIENT_HOSTS = ["codex", "claude", "cursor", "mcp", "vscode"] as const; +export type ClientHost = (typeof CLIENT_HOSTS)[number]; + +/** + * The server-map shape a host expects. Three genuinely different shapes, which is the whole reason a single + * hand-written snippet could never serve every host: Codex reads TOML tables, most JSON hosts read + * `mcpServers`, and VS Code reads `servers` with an explicit transport `type`. + */ +type HostShape = "toml" | "mcpServers" | "servers"; + +export type ClientHostSpec = { + title: string; + /** Where the snippet goes. Rendered as the docs code block's filename. */ + file: string; + /** Where a REMOTE entry goes, when that is a different file from the stdio one. */ + remoteFile?: string; + lang: "toml" | "json"; + shape: HostShape; + /** Hosts that cannot talk streamable-http are stdio-only; the grid must not print config that will fail. */ + supportsRemote: boolean; + /** A caveat that applies only to this host's remote entry. */ + remoteNote?: string; +}; + +export const CLIENT_HOST_SPEC: Record = { + codex: { + title: "Codex (OpenAI)", + file: "~/.codex/config.toml", + lang: "toml", + shape: "toml", + supportsRemote: true, + remoteNote: "Codex releases before its RMCP client became the default also need `experimental_use_rmcp_client = true` at the top level of config.toml.", + }, + // Claude Desktop reaches remote servers through its connectors UI rather than a config file, so the + // remote snippet is Claude Code's project-scoped .mcp.json — the one place pasting it actually works. + claude: { title: "Claude Desktop / Claude Code", file: "claude_desktop_config.json", remoteFile: ".mcp.json", lang: "json", shape: "mcpServers", supportsRemote: true }, + cursor: { title: "Cursor", file: ".cursor/mcp.json", lang: "json", shape: "mcpServers", supportsRemote: true }, + // The generic `mcpServers` shape, for stdio hosts that expect it but are not one of the named three. + // STDIO-ONLY, deliberately: `mcpServers` is a de-facto stdio convention, and the remote dialects differ + // per host (Codex's `bearer_token_env_var` versus the JSON hosts' `headers`). Printing a remote block for + // an unnamed host would be guessing, and a snippet that fails on paste is worse than no snippet -- the + // stdio gateway serves the remote tools to those hosts anyway. + mcp: { title: "Other `mcpServers` hosts", file: "mcp.json", lang: "json", shape: "mcpServers", supportsRemote: false }, + vscode: { title: "VS Code", file: ".vscode/mcp.json", lang: "json", shape: "servers", supportsRemote: true }, +}; + +/** The config file a given pair's snippet belongs in. */ +export function clientConfigFile(host: ClientHost, mode: ConnectionMode): string { + const spec = CLIENT_HOST_SPEC[host]; + return CONNECTION_MODE_SPEC[mode].transport === "http" ? (spec.remoteFile ?? spec.file) : spec.file; +} + +export type ConnectionModeSpec = { + title: string; + /** One sentence a reader can choose by. */ + summary: string; + /** The key the server appears under in the client's server map. */ + serverKey: string; + transport: "stdio" | "http"; + /** stdio modes only: the executable. Absent for a remote mode, which starts no process. */ + command?: string; + /** Required, not optional: `args: []` is a real answer, and an absent one would make every consumer + * write the same `?? []` fallback -- a branch with nothing on the other side of it. */ + args: readonly string[]; + /** Anything a reader must do besides paste the snippet. */ + notes: readonly string[]; +}; + +/** The remote endpoint. Same origin the server card advertises, so the two can never disagree. */ +export const REMOTE_MCP_URL = `${DEFAULT_LOOPOVER_API_URL}/mcp`; + +/** + * The env var the remote endpoint's bearer token comes from. + * + * Deliberately an env-var REFERENCE, never a literal: a config file with a pasted token in it is a token + * in your shell history, your backups, and eventually a screenshot. `loopover-mcp login` stores a session + * for the stdio modes; a remote client reads this variable instead. + */ +export const REMOTE_TOKEN_ENV_VAR = "LOOPOVER_API_TOKEN"; + +export const CONNECTION_MODE_SPEC: Record = { + stdio: { + title: "Local stdio (gateway)", + summary: + "The recommended default. Runs `loopover-mcp` on your machine, keeps auth and git analysis local, and — once you have run `loopover-mcp login` — mounts the remote tool set too, so one entry serves every tool your session entitles you to.", + serverKey: "loopover", + transport: "stdio", + command: "loopover-mcp", + args: ["--stdio"], + notes: [ + "Run `loopover-mcp login` before starting the client; without a session you get the local-git tools only, plus an advisory resource explaining how to get the rest.", + "Pass `--no-remote` to keep the server purely local and skip the remote mount entirely.", + "Assumes `loopover-mcp` is on your PATH; pass `--command /absolute/path/to/loopover-mcp` if your client does not inherit your shell PATH.", + ], + }, + remote: { + title: "Remote streamable-http", + summary: + "For agents that run in the cloud, or anywhere you do not want a local Node process. Connects straight to the hosted server; the local-git tools are not available over this transport because there is no local checkout to read.", + serverKey: "loopover", + transport: "http", + args: [], + notes: [ + `Authenticates with a bearer token read from \`${REMOTE_TOKEN_ENV_VAR}\` — the same variable the CLI honors. Set it in the environment your client starts in; never paste the token into the config file.`, + "Tools whose work is a local git operation are absent here by design. Use the stdio mode if you need them.", + ], + }, + miner: { + title: "Miner stdio", + summary: + "AMS's own local state-visibility tools, as a separate stdio server. It stays separate on purpose: it reads this machine's SQLite state and shares no code or network path with the hosted server.", + serverKey: "loopover-miner", + transport: "stdio", + command: "loopover-miner-mcp", + args: [], + notes: [ + "Takes no flags and needs no login — everything it reads is already on this machine.", + "A dual-role operator runs this alongside the stdio gateway; the two entries coexist in one client config.", + ], + }, +}; + +/** A mode a host cannot actually speak is not printed — a snippet that fails on paste is worse than none. */ +export function supportsConnectionMode(host: ClientHost, mode: ConnectionMode): boolean { + return CONNECTION_MODE_SPEC[mode].transport === "stdio" || CLIENT_HOST_SPEC[host].supportsRemote; +} + +/** Every (host, mode) pair that has a snippet, in a stable order for generated output. */ +export function clientConfigMatrix(): Array<{ host: ClientHost; mode: ConnectionMode }> { + return CLIENT_HOSTS.flatMap((host) => CONNECTION_MODES.filter((mode) => supportsConnectionMode(host, mode)).map((mode) => ({ host, mode }))); +} + +/** A TOML server entry is FLAT by construction -- strings and string arrays only, which is what lets + * tomlTable stay a dozen lines with no escape hatch for a shape it cannot render. */ +type TomlEntry = Record; + +/** The stdio entry both dialects share. */ +function stdioEntry(mode: ConnectionMode, command: string): { command: string; args: readonly string[] } { + return { command, args: CONNECTION_MODE_SPEC[mode].args }; +} + +export type ClientConfigOptions = { + /** Overrides the mode's default executable, for a client that does not inherit your shell PATH. */ + command?: string; +}; + +/** + * The config snippet for one (host, mode) pair. + * + * Throws on a pair with no snippet rather than emitting a plausible-looking one — printing config that + * cannot work is the failure this whole module exists to prevent. + */ +export function clientConfigSnippet(host: ClientHost, mode: ConnectionMode, options: ClientConfigOptions = {}): string { + if (!supportsConnectionMode(host, mode)) { + throw new Error(`${CLIENT_HOST_SPEC[host].title} cannot connect over the ${CONNECTION_MODE_SPEC[mode].title} mode.`); + } + const spec = CONNECTION_MODE_SPEC[mode]; + const hostSpec = CLIENT_HOST_SPEC[host]; + const remote = spec.transport === "http"; + const command = options.command ?? spec.command ?? ""; + + if (hostSpec.shape === "toml") { + // Codex names the token's environment variable in its own key and reads it itself. + const entry: TomlEntry = remote ? { url: REMOTE_MCP_URL, bearer_token_env_var: REMOTE_TOKEN_ENV_VAR } : stdioEntry(mode, command); + return tomlTable(`mcp_servers.${spec.serverKey}`, entry); + } + + // The JSON hosts want the header spelled out instead. Same secret, same never-in-the-file rule. + // `servers` hosts require the transport type on stdio entries too; `mcpServers` hosts infer it from + // `command`, and adding it there would only be noise in a config people read. + const entry = remote + ? { type: "http", url: REMOTE_MCP_URL, headers: { Authorization: `Bearer \${${REMOTE_TOKEN_ENV_VAR}}` } } + : { ...(hostSpec.shape === "servers" ? { type: "stdio" } : {}), ...stdioEntry(mode, command) }; + return compactStringArrays(JSON.stringify({ [hostSpec.shape === "servers" ? "servers" : "mcpServers"]: { [spec.serverKey]: entry } }, null, 2)); +} + +/** + * Collapse a short array of strings back onto one line. + * + * JSON.stringify's pretty printer gives every element its own row, which turns `"args": ["--stdio"]` into + * four lines of a block people read at least as often as they paste it. Done here rather than in the docs + * generator so the CLI's output and the docs' blocks stay byte-identical -- a formatting difference between + * the two is exactly the kind of drift this module exists to remove. + */ +export function compactStringArrays(json: string): string { + return json.replace(/\[\n(\s+)("(?:[^"\\]|\\.)*"(?:,\n\1"(?:[^"\\]|\\.)*")*)\n\s+\]/g, (whole, _indent: string, body: string) => { + const inline = `[${body.split(/,\n\s+/).join(", ")}]`; + return inline.length <= 72 ? inline : whole; + }); +} + +/** + * A TOML table for one server entry. + * + * Hand-rolled rather than pulled from a TOML library on purpose: @loopover/contract is a zod-only leaf + * package that both a Cloudflare Worker and a published CLI depend on, and it is not worth a dependency to + * serialize strings and string arrays. TomlEntry is what keeps that honest -- a nested table would need a + * second pass to place it after its parent's scalars, so the type forbids one rather than a runtime guard + * apologizing for one that can never arrive. + */ +function tomlTable(tableName: string, entry: TomlEntry): string { + const lines = [`[${tableName}]`]; + for (const [key, value] of Object.entries(entry)) { + lines.push(typeof value === "string" ? `${key} = ${JSON.stringify(value)}` : `${key} = [${value.map((item) => JSON.stringify(item)).join(", ")}]`); + } + return lines.join("\n"); +} diff --git a/packages/loopover-contract/src/discovery.ts b/packages/loopover-contract/src/discovery.ts index 32bc855f4..c690f7740 100644 --- a/packages/loopover-contract/src/discovery.ts +++ b/packages/loopover-contract/src/discovery.ts @@ -9,10 +9,71 @@ // the self-host app, and any test can call them identically. Availability filtering is the CALLER's job — // a self-host deployment passes its own filtered list, which is what makes the same route serve a truthful // answer on both deployments. +import { z } from "zod"; import type { McpToolDefinition } from "./tool-definition.js"; /** Which deployment is answering; the card names it so a reader can tell the two apart. */ -export type DiscoveryDeployment = "cloud" | "selfhost"; +export const DISCOVERY_DEPLOYMENTS = ["cloud", "selfhost"] as const; +export type DiscoveryDeployment = (typeof DISCOVERY_DEPLOYMENTS)[number]; + +/** + * The documents are ZOD SCHEMAS with their TypeScript types inferred, not types with a hand-written schema + * beside them (#9526). + * + * They are served by specced routes, so the published OpenAPI document needs a schema for each -- and a + * second, hand-kept declaration of the same shape is the drift this whole epic exists to remove. Declaring + * the schema and inferring the type means the builders below are type-checked against exactly what the API + * document promises. + */ +const JsonSchemaLikeSchema = z.looseObject({}).describe("JSON Schema (draft 2020-12) for a tool's arguments or result."); +const ToolAnnotationsSchema = z.object({ readOnlyHint: z.boolean(), destructiveHint: z.boolean() }); + +export const ServerCardSchema = z.object({ + name: z.string(), + version: z.string(), + description: z.string(), + deployment: z.enum(DISCOVERY_DEPLOYMENTS), + generated_at: z.string(), + capabilities: z.object({ tools: z.object({ listChanged: z.boolean() }) }), + remotes: z.array(z.object({ type: z.literal("streamable-http"), url: z.string() })), + tools: z.array(z.object({ name: z.string(), title: z.string(), description: z.string(), category: z.string(), annotations: ToolAnnotationsSchema })), +}); + +export const ToolExecutorSchema = z.object({ transport: z.literal("streamable-http"), url: z.string(), method: z.literal("tools/call") }); + +export const AgentToolsIndexSchema = z.object({ + generated_at: z.string(), + executor: ToolExecutorSchema, + tools: z.array( + z.object({ + name: z.string(), + title: z.string(), + description: z.string(), + category: z.string(), + annotations: ToolAnnotationsSchema, + input_schema: JsonSchemaLikeSchema, + output_schema: JsonSchemaLikeSchema, + }), + ), +}); + +export const OpenAiToolsSchema = z.object({ + generated_at: z.string(), + executor: ToolExecutorSchema, + tools: z.array(z.object({ type: z.literal("function"), function: z.object({ name: z.string(), description: z.string(), parameters: JsonSchemaLikeSchema }) })), +}); + +export const AnthropicToolsSchema = z.object({ + generated_at: z.string(), + executor: ToolExecutorSchema, + tools: z.array(z.object({ name: z.string(), description: z.string(), input_schema: JsonSchemaLikeSchema })), +}); + +export type ServerCard = z.infer; +export type ToolExecutor = z.infer; +export type AgentToolsIndex = z.infer; +export type OpenAiTools = z.infer; +export type AnthropicTools = z.infer; export type ServerCardInput = { /** The server's semantic version. Sourced from @loopover/mcp's package.json — never hand-bumped here. */ @@ -29,17 +90,6 @@ export type ServerCardInput = { generatedAt: string; }; -export type ServerCard = { - name: string; - version: string; - description: string; - deployment: DiscoveryDeployment; - generated_at: string; - capabilities: { tools: { listChanged: boolean } }; - remotes: Array<{ type: "streamable-http"; url: string }>; - tools: Array<{ name: string; title: string; description: string; category: string; annotations: McpToolDefinition["annotations"] }>; -}; - export const SERVER_CARD_NAME = "io.github.JSONbored/loopover"; export const SERVER_CARD_DESCRIPTION = "LoopOver's contribution-intelligence tools: issue ranking, branch and PR preflight, gate prediction, review explanation, and maintainer/operator management."; @@ -72,8 +122,6 @@ function trimTrailingSlash(value: string): string { * How a caller actually invokes one of these tools. Every agent-tools document carries it, because a tool * catalog with no executor is a list a reader cannot act on. */ -export type ToolExecutor = { transport: "streamable-http"; url: string; method: "tools/call" }; - function executorFor(baseUrl: string): ToolExecutor { return { transport: "streamable-http", url: `${trimTrailingSlash(baseUrl)}/mcp`, method: "tools/call" }; } @@ -81,7 +129,7 @@ function executorFor(baseUrl: string): ToolExecutor { export type AgentToolsInput = { baseUrl: string; tools: readonly McpToolDefinition[]; generatedAt: string }; /** The neutral index: every tool with both schemas, plus the executor. */ -export function buildAgentToolsIndex(input: AgentToolsInput) { +export function buildAgentToolsIndex(input: AgentToolsInput): AgentToolsIndex { return { generated_at: input.generatedAt, executor: executorFor(input.baseUrl), @@ -98,7 +146,7 @@ export function buildAgentToolsIndex(input: AgentToolsInput) { } /** OpenAI's function-tool shape. `parameters` is the input schema verbatim — no second translation. */ -export function buildOpenAiTools(input: AgentToolsInput) { +export function buildOpenAiTools(input: AgentToolsInput): OpenAiTools { return { generated_at: input.generatedAt, executor: executorFor(input.baseUrl), @@ -110,7 +158,7 @@ export function buildOpenAiTools(input: AgentToolsInput) { } /** Anthropic's tool shape. Same schemas, different key names; still one source. */ -export function buildAnthropicTools(input: AgentToolsInput) { +export function buildAnthropicTools(input: AgentToolsInput): AnthropicTools { return { generated_at: input.generatedAt, executor: executorFor(input.baseUrl), diff --git a/packages/loopover-contract/src/telemetry.ts b/packages/loopover-contract/src/telemetry.ts index 597e656a6..62ebbc6c3 100644 --- a/packages/loopover-contract/src/telemetry.ts +++ b/packages/loopover-contract/src/telemetry.ts @@ -43,6 +43,18 @@ export type McpTelemetryErrorCode = (typeof MCP_TELEMETRY_ERROR_CODES)[number]; export const MCP_TELEMETRY_SURFACES = ["remote", "stdio", "miner"] as const; export type McpTelemetrySurface = (typeof MCP_TELEMETRY_SURFACES)[number]; +/** + * HOW the answering server executed the call (#9526). + * + * Orthogonal to `surface`, which says which server was asked. Since the stdio gateway mounts the remote + * tool set, one `surface: "stdio"` call may have run against the local checkout or been forwarded to the + * hosted server, and those are different products from an adoption standpoint: gateway uptake is exactly + * the count of `stdio` + `proxied`, and it is unmeasurable without this dimension because the tool name + * alone does not say which path a given release took. + */ +export const MCP_TELEMETRY_TRANSPORTS = ["local", "proxied"] as const; +export type McpTelemetryTransport = (typeof MCP_TELEMETRY_TRANSPORTS)[number]; + /** * The COMPLETE set of property keys any MCP telemetry event may carry. * @@ -54,6 +66,7 @@ export const MCP_TELEMETRY_PROPERTY_KEYS = [ "tool", "category", "surface", + "transport", "ok", "duration_ms", "error_code", @@ -68,6 +81,10 @@ export const McpToolCallTelemetry = z.object({ tool: z.string().min(1), category: z.string().min(1), surface: z.enum(MCP_TELEMETRY_SURFACES), + // Optional at the seam, never optional on the wire: a sink that has no notion of proxying (the Worker + // and the miner both execute everything themselves) should not have to say so, but a breakdown by + // transport still needs both buckets populated, so buildUsageEventProperties defaults it. + transport: z.enum(MCP_TELEMETRY_TRANSPORTS).optional(), ok: z.boolean(), durationMs: z.number().int().min(0), errorCode: z.enum(MCP_TELEMETRY_ERROR_CODES).optional(), @@ -85,6 +102,7 @@ export function buildUsageEventProperties(call: McpToolCallTelemetry): Record [--json] loopover-mcp doctor [--profile name] [--cwd path] [--exit-code] [--json] loopover-mcp telemetry enable|disable|status [--json] -loopover-mcp init-client --print codex|claude|cursor|mcp|vscode [--agent-profile miner-planner|maintainer-triage|repo-owner-intake] [--json] +loopover-mcp init-client --print codex|claude|cursor|mcp|vscode [--mode stdio|remote|miner] [--agent-profile miner-planner|maintainer-triage|repo-owner-intake] [--json] loopover-mcp decision-pack --login [--json] loopover-mcp repo-decision --login --repo owner/repo [--json] loopover-mcp contributor-profile [--login ] [--json] @@ -334,7 +334,57 @@ The same capabilities are exposed to MCP clients as: ### Client config -`init-client --print ` prints the stdio MCP config for a host: `codex` (TOML), `claude`, `cursor`, and `mcp` (the shared `mcpServers` JSON shape), and `vscode` (VS Code's native `servers` map with `"type": "stdio"`, for `.vscode/mcp.json`). It prints config only; it never edits client files. + + +`init-client --print [--mode ]` prints the MCP config for a host: `codex`, `claude`, `cursor`, `mcp`, `vscode`. Modes are `stdio`, `remote`, `miner`, defaulting to `stdio`. It prints config only; it never edits client files. + +#### Local stdio (gateway) + +The recommended default. Runs `loopover-mcp` on your machine, keeps auth and git analysis local, and — once you have run `loopover-mcp login` — mounts the remote tool set too, so one entry serves every tool your session entitles you to. + +Claude Desktop / Claude Code, in `claude_desktop_config.json`: + +```json +{ + "mcpServers": { + "loopover": { + "command": "loopover-mcp", + "args": ["--stdio"] + } + } +} +``` + +- Run `loopover-mcp login` before starting the client; without a session you get the local-git tools only, plus an advisory resource explaining how to get the rest. +- Pass `--no-remote` to keep the server purely local and skip the remote mount entirely. +- Assumes `loopover-mcp` is on your PATH; pass `--command /absolute/path/to/loopover-mcp` if your client does not inherit your shell PATH. + +#### Remote streamable-http + +For agents that run in the cloud, or anywhere you do not want a local Node process. Connects straight to the hosted server; the local-git tools are not available over this transport because there is no local checkout to read. + +Claude Desktop / Claude Code, in `.mcp.json`: + +```json +{ + "mcpServers": { + "loopover": { + "type": "http", + "url": "https://api.loopover.ai/mcp", + "headers": { + "Authorization": "Bearer ${LOOPOVER_API_TOKEN}" + } + } + } +} +``` + +- Authenticates with a bearer token read from `LOOPOVER_API_TOKEN` — the same variable the CLI honors. Set it in the environment your client starts in; never paste the token into the config file. +- Tools whose work is a local git operation are absent here by design. Use the stdio mode if you need them. + +The remote endpoint is `https://api.loopover.ai/mcp` — the same URL `/.well-known/mcp.json` advertises. + + ### Agent profiles diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 1e0d47544..0455d2820 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -7,6 +7,17 @@ import { homedir } from "node:os"; import { delimiter, dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { CLI_RESPONSE_SCHEMAS, type ApiResponse, type ValidatedApiPath } from "@loopover/contract/api-schemas"; +import { + CLIENT_HOSTS, + CLIENT_HOST_SPEC, + CONNECTION_MODES, + CONNECTION_MODE_SPEC, + clientConfigFile, + clientConfigSnippet, + supportsConnectionMode, + type ClientHost, + type ConnectionMode, +} from "@loopover/contract/client-config"; import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { @@ -246,7 +257,14 @@ const CLI_COMMAND_SPEC = { tools: { subcommands: ["search"], usage: ["tools [--json]", "tools search [--json]"] }, doctor: { subcommands: [], usage: ["doctor [--profile name] [--cwd path] [--exit-code] [--json]"] }, telemetry: { subcommands: ["enable", "disable", "status"], usage: ["telemetry enable|disable|status [--json]"] }, - "init-client": { subcommands: [], usage: ["init-client --print codex|claude|cursor|mcp|vscode [--agent-profile miner-planner|maintainer-triage|repo-owner-intake] [--json]"] }, + "init-client": { + subcommands: [], + // Hosts and modes come from the @loopover/contract grid that also renders the snippets and the docs, so + // adding either can never leave the usage line describing a smaller surface than the command accepts. + usage: [ + `init-client --print ${CLIENT_HOSTS.join("|")} [--mode ${CONNECTION_MODES.join("|")}] [--agent-profile miner-planner|maintainer-triage|repo-owner-intake] [--json]`, + ], + }, "decision-pack": { subcommands: [], usage: ["decision-pack --login [--json]"] }, "repo-decision": { subcommands: [], usage: ["repo-decision --login --repo owner/repo [--json]"] }, "contributor-profile": { subcommands: [], usage: ["contributor-profile [--login ] [--json]"] }, @@ -2490,16 +2508,25 @@ function registerProxiedTool(tool: RemoteToolDescriptor): void { { ...(tool.title ? { title: tool.title } : {}), ...(tool.description ? { description: tool.description } : {}), - ...(contract ? { inputSchema: contract.input.shape, outputSchema: contract.output } : {}), + // The schema OBJECTS, not their `.shape` -- a raw shape is re-wrapped in a plain `z.object` that + // silently drops the catchall, turning every extra field the remote allows into a -32602 here. + ...(contract ? { inputSchema: contract.input, outputSchema: contract.output } : {}), ...(tool.annotations ? { annotations: tool.annotations as never } : {}), _meta: { ...(tool._meta ?? {}), transport: "proxied" }, } as never, - (async (input: unknown) => { - // Forwarded verbatim to the remote's own tools/call: this layer routes, it does not interpret. - const payload = await apiPost("/mcp", { jsonrpc: "2.0", id: Date.now(), method: "tools/call", params: { name: tool.name, arguments: input } }); - const result = (payload as { result?: unknown }).result; - return (result ?? payload) as never; - }) as never, + // Wrapped by the same telemetry chokepoint local tools go through, tagged `proxied` -- gateway + // adoption is exactly the count of stdio calls that took this path (#9526 requirement 6). + wrapStdioToolHandler( + tool.name, + () => telemetryState().enabled, + (async (input: unknown) => { + // Forwarded verbatim to the remote's own tools/call: this layer routes, it does not interpret. + const payload = await apiPost("/mcp", { jsonrpc: "2.0", id: Date.now(), method: "tools/call", params: { name: tool.name, arguments: input } }); + const result = (payload as { result?: unknown }).result; + return result ?? payload; + }) as (...args: unknown[]) => Promise, + "proxied", + ) as never, ); } @@ -2554,18 +2581,45 @@ export async function mountRemoteTools(options: { argv?: readonly string[]; fetc registerGatewayAdvisory(result); return result; } - for (const tool of result.tools) registerProxiedTool(tool); - return result; + + const mounted: RemoteToolDescriptor[] = []; + const skipped = [...result.skipped]; + for (const tool of result.tools) { + try { + registerProxiedTool(tool); + mounted.push(tool); + } catch { + // One descriptor the SDK refuses -- a repeated name in the same response, say -- must cost that tool + // and nothing else. Mounting is all-or-nothing only if you let it be. + skipped.push(tool.name); + } + } + return { status: "mounted", tools: mounted, skipped }; } // #7764: only bind the shared stdin/stdout transport when actually launched as the CLI/stdio process. An // in-process unit-test importer holds the exported `server` and connects it to an in-memory transport instead. -/* v8 ignore next 4 -- only the launched stdio process mounts remote tools and binds the real transport; +/* v8 ignore start -- only the launched stdio process binds the real transport and mounts remote tools; unit tests call mountRemoteTools() directly and connect in-memory. */ if (runAsCliEntrypoint) { - await mountRemoteTools(); await server.connect(new StdioServerTransport()); + // Mount AFTER connecting, and without awaiting it (#9526): a network round-trip in front of the transport + // bind would put seconds of dead air between a client launching this process and it answering anything. + // Registering a tool on a connected server emits notifications/tools/list_changed, so a client that + // listed early is told to re-list the moment the remote set lands -- which is the refresh path the + // stateless remote cannot push on its own. + // + // The .catch is the boundary belt: mountRemoteTools is total by construction (discoverRemoteTools guards + // its own I/O, and each registration is caught individually), but nothing here awaits it, and an unhandled + // rejection ends a Node process. A dead stdio server is not an acceptable cost for an enhancement. + void mountRemoteTools().catch((error: unknown) => { + registerGatewayAdvisory({ + status: "unavailable", + advisory: `Remote tools are not mounted: the gateway failed to start (${error instanceof Error ? error.message : String(error)}). Local tools are available now.`, + }); + }); } +/* v8 ignore stop */ async function withClientWorkspaceRoots(input: any) { return withWorkspaceRoots(input, await clientWorkspaceRoots()); @@ -4851,20 +4905,30 @@ function shellArg(value: any) { } function initClient(options: any) { - const client = String(options.print ?? options.client ?? "").toLowerCase(); - if (!client) throw new Error("Pass --print codex, --print claude, --print cursor, --print mcp, or --print vscode."); - const command = options.command ?? "loopover-mcp"; - const snippet = clientSnippet(client, command); + const client = String(options.print ?? options.client ?? "").toLowerCase() as ClientHost; + if (!client) throw new Error(`Pass --print with one of: ${CLIENT_HOSTS.join(", ")}.`); + if (!CLIENT_HOSTS.includes(client)) throw new Error(`Unsupported client: ${client}. Use ${CLIENT_HOSTS.join(", ")}.`); + // #9526: the same host can be wired three ways, so the mode is explicit rather than assumed. `stdio` stays + // the default, which is why every pre-gateway invocation keeps printing exactly what it printed before. + const mode = String(options.mode ?? "stdio").toLowerCase() as ConnectionMode; + if (!CONNECTION_MODES.includes(mode)) throw new Error(`Unsupported mode: ${mode}. Use ${CONNECTION_MODES.join(", ")}.`); + if (!supportsConnectionMode(client, mode)) throw new Error(`${CLIENT_HOST_SPEC[client].title} cannot connect over the ${CONNECTION_MODE_SPEC[mode].title} mode.`); + const modeSpec = CONNECTION_MODE_SPEC[mode]; + const command = options.command ?? modeSpec.command ?? "loopover-mcp"; + const snippet = clientConfigSnippet(client, mode, { command }); const agentProfile = resolveAgentProfile(options.agentProfile); + const remoteNote = modeSpec.transport === "http" ? CLIENT_HOST_SPEC[client].remoteNote : undefined; const payload = { client, - command, - args: ["--stdio"], + mode, + file: clientConfigFile(client, mode), + command: modeSpec.transport === "http" ? null : command, + args: [...modeSpec.args], snippet, agentProfile, notes: [ - "Run `loopover-mcp login` before starting the MCP client.", - "Use an absolute command path if the client does not inherit your shell PATH.", + ...modeSpec.notes, + ...(remoteNote ? [remoteNote] : []), "This command prints config only; it does not edit client files.", ...(agentProfile ? [ @@ -5265,42 +5329,6 @@ function redactPrivateValidationMetrics(text: any) { ); } -function clientSnippet(client: any, command: any) { - if (client === "codex") return `[mcp_servers.loopover]\ncommand = ${JSON.stringify(command)}\nargs = ["--stdio"]`; - if (client === "claude" || client === "cursor" || client === "mcp") { - return JSON.stringify( - { - mcpServers: { - loopover: { - command, - args: ["--stdio"], - }, - }, - }, - null, - 2, - ); - } - // VS Code's native MCP support uses a `servers` map with an explicit transport type, not the - // `mcpServers` shape the other JSON hosts use, so it needs its own snippet (see .vscode/mcp.json). - if (client === "vscode") { - return JSON.stringify( - { - servers: { - loopover: { - type: "stdio", - command, - args: ["--stdio"], - }, - }, - }, - null, - 2, - ); - } - throw new Error(`Unsupported client: ${client}. Use codex, claude, cursor, mcp, or vscode.`); -} - async function getDecisionPackWithCache(login: any) { try { const payload = await apiGet(`/v1/contributors/${encodeURIComponent(login)}/decision-pack`); diff --git a/packages/loopover-mcp/lib/telemetry.ts b/packages/loopover-mcp/lib/telemetry.ts index 09ac91f2c..2a56445bc 100644 --- a/packages/loopover-mcp/lib/telemetry.ts +++ b/packages/loopover-mcp/lib/telemetry.ts @@ -9,6 +9,7 @@ import { resolveErrorCode, toolExcludesPayloads, UNKNOWN_TOOL_CATEGORY, + type McpTelemetryTransport, type McpToolCallTelemetry, } from "@loopover/contract"; @@ -111,6 +112,9 @@ export function wrapStdioToolHandler( name: string, getTelemetryEnabled: () => boolean, handler: StdioToolHandler, + // #9526: "local" means this process did the work; "proxied" means it forwarded the call to the hosted + // server through gateway mode. Defaulted so every pre-gateway registration keeps reporting what it did. + transport: McpTelemetryTransport = "local", ): StdioToolHandler { return async (...args) => { const startedAt = Date.now(); @@ -124,6 +128,7 @@ export function wrapStdioToolHandler( tool: name, ok, durationMs: Date.now() - startedAt, + transport, args: args[0], result: result?.structuredContent, }); @@ -134,6 +139,7 @@ export function wrapStdioToolHandler( tool: name, ok: false, durationMs: Date.now() - startedAt, + transport, args: args[0], error, }); @@ -155,7 +161,7 @@ export function wrapStdioToolHandler( */ export async function recordStdioDispatchTelemetry( telemetryEnabled: boolean, - call: { tool: string; ok: boolean; durationMs: number; args?: unknown; result?: unknown; error?: unknown }, + call: { tool: string; ok: boolean; durationMs: number; transport?: McpTelemetryTransport; args?: unknown; result?: unknown; error?: unknown }, ): Promise { if (telemetryEnabled !== true) return; const apiKey = trimmedOrUndefined(process.env.LOOPOVER_MCP_POSTHOG_API_KEY); @@ -166,6 +172,7 @@ export async function recordStdioDispatchTelemetry( tool: call.tool, category: contract?.category ?? UNKNOWN_TOOL_CATEGORY, surface: "stdio", + transport: call.transport ?? "local", ok: call.ok, durationMs: call.durationMs, ...(call.ok ? {} : { errorCode: resolveErrorCode(call.error) }), diff --git a/packages/loopover-miner/README.md b/packages/loopover-miner/README.md index 9210a4ced..d8cdd4772 100644 --- a/packages/loopover-miner/README.md +++ b/packages/loopover-miner/README.md @@ -287,7 +287,9 @@ It exposes these read-only tools, generated from the `@loopover/contract` regist ### Client config -`loopover-mcp` (ORB's hosted contributor-workflow tools) and `loopover-miner-mcp` (AMS's own local state-visibility tools above) can run as two separate stdio servers in the same MCP client session — useful for a dual-role operator running both ORB and AMS on the same box. Generate ORB's half with `loopover-mcp init-client --print claude` (see the [`@loopover/mcp` README](../loopover-mcp/README.md#client-config)); `loopover-miner-mcp` takes no flags, so its entry is just the bin name. Combined, a Claude Desktop / Claude Code style config looks like: + + +`loopover-mcp` (ORB's hosted contributor-workflow tools) and `loopover-miner-mcp` (AMS's own local state-visibility tools above) run as two separate stdio servers in the same MCP client session — the dual-role case for an operator running both ORB and AMS on one box. Generate ORB's half with `loopover-mcp init-client --print claude` (see the [`@loopover/mcp` README](../loopover-mcp/README.md#client-config)); `loopover-miner-mcp` takes no flags, so its entry is just the bin name. Combined: ```json { @@ -304,7 +306,11 @@ It exposes these read-only tools, generated from the `@loopover/contract` regist } ``` -`loopover` exposes ORB's hosted contributor-workflow tools (issue ranking, PR packet prep, decision packs). `loopover-miner` exposes AMS's own local state-visibility tools listed above (portfolio dashboard, claims, audit feed, run state, plans, calibration) — a fully separate, 100% local tool surface with no shared code or network calls between the two. Both follow the same `loopover_*` tool-naming convention (`loopover_...` vs. `loopover_miner_...`), but back onto different stores: ORB's tools read the hosted loopover backend, AMS's tools read this machine's own local SQLite files (see [Local storage](#local-storage)) — a handful of AMS tools even name the ORB tool they mirror (e.g. `loopover_miner_get_run_state` is the read-only analog of `loopover_get_automation_state`) so the relationship is explicit at the point of use, not just here. +`loopover` exposes ORB's hosted contributor-workflow tools (issue ranking, PR packet prep, decision packs) and, once you have run `loopover-mcp login`, the remote tool set it mounts. `loopover-miner` exposes AMS's own local state-visibility tools listed above (portfolio dashboard, claims, audit feed, run state, plans, calibration) — a fully separate, 100% local tool surface with no shared code or network calls between the two. + +Both follow the same `loopover_*` naming convention (`loopover_...` vs. `loopover_miner_...`), but back onto different stores: ORB's tools read the hosted loopover backend, AMS's tools read this machine's own local SQLite files (see [Local storage](#local-storage)) — a handful of AMS tools even name the ORB tool they mirror (e.g. `loopover_miner_get_run_state` is the read-only analog of `loopover_get_automation_state`) so the relationship is explicit at the point of use, not just here. + + ## Version check diff --git a/scripts/gen-mcp-client-config.ts b/scripts/gen-mcp-client-config.ts new file mode 100644 index 000000000..202e93eab --- /dev/null +++ b/scripts/gen-mcp-client-config.ts @@ -0,0 +1,173 @@ +#!/usr/bin/env node +// Generate every client-config surface from the @loopover/contract grid (#9526). +// +// Same one-source pattern as gen-mcp-tool-reference.ts, applied to the other half of the docs: how you +// CONNECT, rather than what you get once connected. Four places used to spell out the same server-map +// snippets by hand -- `clientSnippet()` in the stdio bin, apps/loopover-ui/content/docs/mcp-clients.mdx, and +// both package READMEs -- and they had already drifted: the docs page documented no remote auth at all, and +// nothing described the gateway's remote mount because the gateway did not exist when the prose was written. +// +// Surfaces: +// 1. apps/loopover-ui/content/docs/mcp-clients.mdx -- a section per host, one code block per mode +// 2. packages/loopover-mcp/README.md -- the stdio + remote config blocks +// 3. packages/loopover-miner/README.md -- the dual-role (gateway + miner) combined block +// +// `--check` regenerates in memory and diffs, exiting 1 on drift; test:ci runs it. +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { + CLIENT_HOSTS, + CLIENT_HOST_SPEC, + CONNECTION_MODES, + CONNECTION_MODE_SPEC, + REMOTE_MCP_URL, + clientConfigFile, + clientConfigSnippet, + compactStringArrays, + supportsConnectionMode, + type ClientHost, + type ConnectionMode, +} from "@loopover/contract/client-config"; + +const CHECK = process.argv.includes("--check"); +const ROOT = process.cwd(); + +// MDX is not HTML: an `` comment is parsed as JSX and blows up the page, so the docs markers use the +// expression form. The READMEs are plain markdown and keep the HTML form the other generators use. +const MDX_BEGIN = "{/* GENERATED:MCP-CLIENT-CONFIG:BEGIN — edit packages/loopover-contract/src/client-config.ts, then `npm run mcp:client-config` */}"; +const MDX_END = "{/* GENERATED:MCP-CLIENT-CONFIG:END */}"; +const MD_BEGIN = ""; +const MD_END = ""; + +/** Backticks and `${` would both terminate or interpolate inside an MDX `code={\`...\`}` template literal. */ +function escapeForMdxTemplate(text: string): string { + return text.replace(/\\/g, "\\\\").replace(/`/g, "\\`").replace(/\$\{/g, "\\${"); +} + +function mdxCodeBlock(filename: string, lang: string, code: string): string { + const attributes = [...(filename ? [` filename=${JSON.stringify(filename)}`] : []), ` lang=${JSON.stringify(lang)}`]; + return ``; +} + +/** The docs page body: choose a mode, then a host, with each host's caveats where the reader will hit them. */ +export function docsSection(): string { + const lines: string[] = [ + "## Generate config", + "", + "Every block on this page is what `init-client` prints. It prints config only — it never edits your client files.", + "", + mdxCodeBlock( + "", + "bash", + CONNECTION_MODES.flatMap((mode) => + CLIENT_HOSTS.filter((host) => supportsConnectionMode(host, mode)).map((host) => `loopover-mcp init-client --print ${host}${mode === "stdio" ? "" : ` --mode ${mode}`}`), + ).join("\n"), + ), + "", + ]; + for (const mode of CONNECTION_MODES) { + const spec = CONNECTION_MODE_SPEC[mode]; + lines.push(`## ${spec.title}`, "", spec.summary, ""); + for (const note of spec.notes) lines.push(`- ${note}`); + lines.push(""); + for (const host of CLIENT_HOSTS.filter((candidate) => supportsConnectionMode(candidate, mode))) { + const hostSpec = CLIENT_HOST_SPEC[host]; + // The mode goes in the heading, not just the section, so the anchors stay unique across the page. + lines.push(`### ${hostSpec.title} (${mode})`, ""); + lines.push(mdxCodeBlock(clientConfigFile(host, mode), hostSpec.lang, clientConfigSnippet(host, mode)), ""); + const remoteNote = spec.transport === "http" ? hostSpec.remoteNote : undefined; + if (remoteNote) lines.push(remoteNote, ""); + } + } + lines.push("Every block above comes from the same grid the CLI prints from, so a snippet copied from here and one printed by `init-client` can never disagree."); + return lines.join("\n"); +} + +function fence(lang: string, code: string): string { + return "```" + lang + "\n" + code + "\n```"; +} + +/** The stdio README: how to wire the gateway, and the remote endpoint for anyone who cannot run a process. */ +export function stdioReadmeSection(): string { + const lines: string[] = []; + lines.push( + `\`init-client --print [--mode ]\` prints the MCP config for a host: ${CLIENT_HOSTS.map((host) => `\`${host}\``).join(", ")}. Modes are ${CONNECTION_MODES.map((mode) => `\`${mode}\``).join(", ")}, defaulting to \`stdio\`. It prints config only; it never edits client files.`, + "", + ); + for (const mode of ["stdio", "remote"] as const) { + const spec = CONNECTION_MODE_SPEC[mode]; + lines.push(`#### ${spec.title}`, "", spec.summary, ""); + lines.push(`${CLIENT_HOST_SPEC.claude.title}, in \`${clientConfigFile("claude", mode)}\`:`, ""); + lines.push(fence("json", clientConfigSnippet("claude", mode)), ""); + for (const note of spec.notes) lines.push(`- ${note}`); + lines.push(""); + } + lines.push(`The remote endpoint is \`${REMOTE_MCP_URL}\` — the same URL \`/.well-known/mcp.json\` advertises.`); + return lines.join("\n"); +} + +/** + * The miner README's dual-role block: the gateway and the miner server in ONE client config. + * + * Built by merging the two modes' entries rather than by writing a third snippet, because "run both" is + * exactly the case a hand-written third copy gets wrong first. + */ +export function minerReadmeSection(): string { + const merged = { mcpServers: {} as Record }; + for (const mode of ["stdio", "miner"] as const) { + Object.assign(merged.mcpServers, (JSON.parse(clientConfigSnippet("claude", mode)) as { mcpServers: Record }).mcpServers); + } + return [ + `\`${CONNECTION_MODE_SPEC.stdio.command}\` (ORB's hosted contributor-workflow tools) and \`${CONNECTION_MODE_SPEC.miner.command}\` (AMS's own local state-visibility tools above) run as two separate stdio servers in the same MCP client session — the dual-role case for an operator running both ORB and AMS on one box. Generate ORB's half with \`loopover-mcp init-client --print claude\` (see the [\`@loopover/mcp\` README](../loopover-mcp/README.md#client-config)); \`${CONNECTION_MODE_SPEC.miner.command}\` takes no flags, so its entry is just the bin name. Combined:`, + "", + fence("json", compactStringArrays(JSON.stringify(merged, null, 2))), + "", + `\`${CONNECTION_MODE_SPEC.stdio.serverKey}\` exposes ORB's hosted contributor-workflow tools (issue ranking, PR packet prep, decision packs) and, once you have run \`loopover-mcp login\`, the remote tool set it mounts. \`${CONNECTION_MODE_SPEC.miner.serverKey}\` exposes AMS's own local state-visibility tools listed above (portfolio dashboard, claims, audit feed, run state, plans, calibration) — a fully separate, 100% local tool surface with no shared code or network calls between the two.`, + "", + `Both follow the same \`loopover_*\` naming convention (\`loopover_...\` vs. \`loopover_miner_...\`), but back onto different stores: ORB's tools read the hosted loopover backend, AMS's tools read this machine's own local SQLite files (see [Local storage](#local-storage)) — a handful of AMS tools even name the ORB tool they mirror (e.g. \`loopover_miner_get_run_state\` is the read-only analog of \`loopover_get_automation_state\`) so the relationship is explicit at the point of use, not just here.`, + ].join("\n"); +} + +export function replaceBetweenMarkers(source: string, replacement: string, begin: string, end: string, file: string): string { + const beginAt = source.indexOf(begin); + const endAt = source.indexOf(end); + if (beginAt === -1 || endAt === -1) throw new Error(`${file} is missing the GENERATED:MCP-CLIENT-CONFIG markers.`); + return source.slice(0, beginAt + begin.length) + "\n\n" + replacement + "\n\n" + source.slice(endAt); +} + +const TARGETS: Array<{ file: string; next: (current: string) => string }> = [ + { + file: "apps/loopover-ui/content/docs/mcp-clients.mdx", + next: (current) => replaceBetweenMarkers(current, docsSection(), MDX_BEGIN, MDX_END, "apps/loopover-ui/content/docs/mcp-clients.mdx"), + }, + { + file: "packages/loopover-mcp/README.md", + next: (current) => replaceBetweenMarkers(current, stdioReadmeSection(), MD_BEGIN, MD_END, "packages/loopover-mcp/README.md"), + }, + { + file: "packages/loopover-miner/README.md", + next: (current) => replaceBetweenMarkers(current, minerReadmeSection(), MD_BEGIN, MD_END, "packages/loopover-miner/README.md"), + }, +]; + +function main(): void { + const drifted: string[] = []; + for (const target of TARGETS) { + const path = join(ROOT, target.file); + const current = readFileSync(path, "utf8"); + const next = target.next(current); + if (next === current) continue; + if (CHECK) drifted.push(target.file); + else writeFileSync(path, next); + } + if (CHECK && drifted.length > 0) { + process.stderr.write(`gen-mcp-client-config: stale generated output in ${drifted.join(", ")} -- run \`npm run mcp:client-config\`.\n`); + process.exit(1); + } + const pairs = CLIENT_HOSTS.flatMap((host) => CONNECTION_MODES.filter((mode) => supportsConnectionMode(host as ClientHost, mode as ConnectionMode))); + process.stdout.write(`gen-mcp-client-config: ${CHECK ? "checked" : "wrote"} ${TARGETS.length} surface(s) from ${pairs.length} host/mode pairs.\n`); +} + +// Importable for tests without running the file's I/O (the same guard gen-command-reference.ts uses). +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main(); diff --git a/scripts/mcp-package-allowlist.ts b/scripts/mcp-package-allowlist.ts index da2fbedd3..5752ef87e 100644 --- a/scripts/mcp-package-allowlist.ts +++ b/scripts/mcp-package-allowlist.ts @@ -1,14 +1,30 @@ // Canonical MCP published-tarball allowlist (#6291). Shared by check-mcp-package.ts and // mcp-release-candidate-core.ts so the dry-run gate and the release-candidate tarball check // cannot drift (the previous duplicated lists already missed shipped lib/*.js files). +// +// The dist/ half is DERIVED from the committed TypeScript sources rather than typed out (#9526). Every +// version of this list has failed the same way -- by UNDER-listing, so a legitimately shipped lib/*.js +// tripped the gate that exists to catch strays -- and a list you must remember to extend is a list that +// eventually lags the thing it describes. Reading the source tree keeps the property that matters: the +// tarball may contain the compiled form of committed source and nothing else, so a stray file, a copied +// node_modules artifact, or a build that emits somewhere unexpected still fails loudly. +import { readdirSync } from "node:fs"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const PACKAGE_ROOT = join(fileURLToPath(new URL(".", import.meta.url).href), "..", "packages", "loopover-mcp"); + +/** `dist//.js` for every committed `/.ts`, as anchored exact-match patterns. */ +function compiledOutputPatterns(dir: "bin" | "lib"): RegExp[] { + return readdirSync(join(PACKAGE_ROOT, dir)) + .filter((file) => file.endsWith(".ts") && !file.endsWith(".d.ts")) + .sort() + .map((file) => new RegExp(`^dist/${dir}/${file.replace(/\.ts$/, "").replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\.js$`)); +} export const MCP_PACKAGE_ALLOWED_FILE_PATTERNS: RegExp[] = [ - /^dist\/bin\/loopover-mcp\.js$/, - /^dist\/lib\/cli-error\.js$/, - /^dist\/lib\/local-branch\.js$/, - /^dist\/lib\/format-table\.js$/, - /^dist\/lib\/redact-local-path\.js$/, - /^dist\/lib\/telemetry\.js$/, + ...compiledOutputPatterns("bin"), + ...compiledOutputPatterns("lib"), /^scripts\/gittensor-score-preview\.(mjs|py)$/, /^package\.json$/, /^README\.md$/, diff --git a/src/api/routes.ts b/src/api/routes.ts index fce4c6744..54a1be3db 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -180,6 +180,7 @@ import { handleMcpRequest } from "../mcp/server"; import { simulateOpenPrPressureShape } from "../mcp/server"; import { simulateOpenPrPressure, type OpenPrPressureInput } from "../services/open-pr-pressure-scenarios"; import { DISCOVERY_PATHS, discoveryDocumentsFor, respondWithDocument, toolsForDeployment } from "../mcp/discovery-routes"; +import { isSelfHostedReviewRuntime } from "../selfhost/review-runtime"; import { buildOpenApiSpec } from "../openapi/spec"; import { COMMAND_RATE_LIMIT_EVENT_TYPE, generateSignalSnapshots } from "../queue/processors"; import { generateChatQaAnswer } from "../services/ai-chat-qa"; @@ -1258,11 +1259,16 @@ export function createApp() { // excluded from requiresApiToken alongside the other unauthenticated document routes. for (const path of DISCOVERY_PATHS) { app.get(path, (c) => { + // The SAME routes on both deployments, scoped to what each actually serves. A self-host card that + // advertised the cloud's tool set would be a list of calls that 404 -- and this app IS the self-host + // 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"; const documents = discoveryDocumentsFor({ version: LATEST_RECOMMENDED_MCP_VERSION, - deployment: "cloud", + deployment, baseUrl: c.env.PUBLIC_API_ORIGIN ?? new URL(c.req.url).origin, - tools: toolsForDeployment("cloud"), + tools: toolsForDeployment(deployment), }); return respondWithDocument(documents[path]!, c.req.header("if-none-match") ?? null); }); diff --git a/src/openapi/discovery-route-specs.ts b/src/openapi/discovery-route-specs.ts new file mode 100644 index 000000000..934c43553 --- /dev/null +++ b/src/openapi/discovery-route-specs.ts @@ -0,0 +1,68 @@ +// Spec entries for the `.well-known` discovery surfaces (#9526). +// +// SPEC-ONLY, the same split `orb-and-control-route-specs.ts` records: `registerRouteSpec` contributes the +// operation while the handler stays where it is registered, because the app and the OpenAPI registry are +// built by two different functions (`createApp` and `buildOpenApiSpec`) and `defineRoute` needs both at +// once. Nothing is lost here -- these routes take no request body and no query, so the validating wrapper +// would have nothing to validate. +// +// The response schemas come from @loopover/contract, which is also what the handlers build their bodies +// with, so the published document describes exactly the object served rather than a second description of +// it that can drift. +import type { OpenAPIRegistry } from "@asteasolutions/zod-to-openapi"; +import { AgentToolsIndexSchema, AnthropicToolsSchema, OpenAiToolsSchema, ServerCardSchema } from "@loopover/contract/discovery"; +import { DISCOVERY_PATHS } from "../mcp/discovery-routes"; +import { registerRouteSpec } from "./define-route"; +import type { z } from "zod"; + +/** + * One entry per discovery path, keyed BY the path constant. + * + * Keyed rather than listed so the type system enforces exhaustiveness: adding a document to + * DISCOVERY_PATHS without describing it here is a compile error, not a ratchet failure discovered in CI. + */ +const DISCOVERY_SPECS: Record<(typeof DISCOVERY_PATHS)[number], { operationId: string; summary: string; schema: z.ZodTypeAny }> = { + "/.well-known/mcp.json": { + operationId: "getMcpServerCard", + summary: "MCP server card for this deployment", + schema: ServerCardSchema, + }, + "/.well-known/agent-tools/index.json": { + operationId: "getAgentToolsIndex", + summary: "Neutral agent-tools catalog with both schemas per tool", + schema: AgentToolsIndexSchema, + }, + "/.well-known/agent-tools/openai.json": { + operationId: "getAgentToolsOpenAi", + summary: "Agent-tools catalog in OpenAI's function-tool shape", + schema: OpenAiToolsSchema, + }, + "/.well-known/agent-tools/anthropic.json": { + operationId: "getAgentToolsAnthropic", + summary: "Agent-tools catalog in Anthropic's tool shape", + schema: AnthropicToolsSchema, + }, +}; + +export function registerDiscoveryRouteSpecs(registry: OpenAPIRegistry): void { + for (const path of DISCOVERY_PATHS) { + const spec = DISCOVERY_SPECS[path]; + registerRouteSpec(registry, { + method: "get", + path, + operationId: spec.operationId, + tags: ["Discovery"], + summary: spec.summary, + // Unauthenticated on purpose: a registry crawler has no token, and everything here -- tool names, + // descriptions, schemas -- is already public through /mcp's own tools/list. + auth: "public", + description: + "Computed at request time from the tool contract registry, filtered to what this deployment serves. " + + "Carries a weak ETag; send `if-none-match` to get a 304 instead of the body.", + responses: { + 200: { description: spec.summary, schema: spec.schema }, + 304: { description: "The caller's `if-none-match` already names this document" }, + }, + }); + } +} diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 698a8b662..90f438f21 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1,5 +1,6 @@ import { OpenApiGeneratorV3, OpenAPIRegistry } from "@asteasolutions/zod-to-openapi"; import { requiresApiToken } from "../auth/route-auth"; +import { registerDiscoveryRouteSpecs } from "./discovery-route-specs"; import { registerOrbAndControlRouteSpecs } from "./orb-and-control-route-specs"; import { registerInternalAndPublicRouteSpecs } from "./internal-and-public-route-specs"; import { z } from "zod"; @@ -2248,7 +2249,16 @@ export function buildOpenApiSpec() { // Registered from their own module rather than inline here because each entry declares an auth // level that DERIVES its security stanza, instead of having one bolted on afterwards by // applySecurityMetadata's path-prefix guesswork. +<<<<<<< HEAD for (const register of SPEC_REGISTRARS) register(registry); +||||||| parent of 42b72c8f5 (feat(mcp): derive every client-config surface and tag proxied calls (#9526)) + registerOrbAndControlRouteSpecs(registry); + registerInternalAndPublicRouteSpecs(registry); +======= + registerOrbAndControlRouteSpecs(registry); + registerDiscoveryRouteSpecs(registry); + registerInternalAndPublicRouteSpecs(registry); +>>>>>>> 42b72c8f5 (feat(mcp): derive every client-config surface and tag proxied calls (#9526)) const generator = new OpenApiGeneratorV3(registry.definitions); const document = generator.generateDocument({ diff --git a/test/integration/mcp-discovery-routes.test.ts b/test/integration/mcp-discovery-routes.test.ts index 9ee3c02fb..4f98cfc22 100644 --- a/test/integration/mcp-discovery-routes.test.ts +++ b/test/integration/mcp-discovery-routes.test.ts @@ -34,11 +34,32 @@ describe("discovery routes (#9526)", () => { tools: unknown[]; }; expect(card.name).toBe(SERVER_CARD_NAME); - expect(card.deployment).toBe("cloud"); expect(card.remotes[0]!.url).toBe("https://api.loopover.ai/mcp"); expect(card.tools.length).toBeGreaterThan(100); }); + it("names the deployment that ANSWERED, and scopes the catalog to what it serves", async () => { + // src/server.ts serves this very Hono app, so the same route answers on both deployments. A self-host + // card advertising the cloud's tool set would be a list of calls that 404 there. + const selfhostEnv = createTestEnv(); + expect(selfhostEnv.SELFHOST_TRANSIENT_CACHE, "the test env is the self-host runtime").toBeTruthy(); + const selfhost = (await (await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), selfhostEnv)).json()) as { + deployment: string; + tools: Array<{ name: string }>; + }; + expect(selfhost.deployment).toBe("selfhost"); + + resetDiscoveryCacheForTesting(); + const cloud = (await ( + await app.fetch(new Request("https://api.loopover.ai/.well-known/mcp.json"), { ...selfhostEnv, SELFHOST_TRANSIENT_CACHE: undefined } as unknown as Env) + ).json()) as { deployment: string; tools: Array<{ name: string }> }; + expect(cloud.deployment).toBe("cloud"); + + // Not merely a different label: the registry's cloud-only entries are absent from the self-host card. + const cloudOnly = cloud.tools.filter((tool) => !selfhost.tools.some((entry) => entry.name === tool.name)); + expect(cloudOnly.length, "a cloud-only tool must not appear on a self-host card").toBeGreaterThan(0); + }); + it("does not hardcode a version — the card reports what the shipped package says", async () => { const card = (await (await request("/.well-known/mcp.json")).json()) as { version: string }; // The old serverInfo said "0.1.0" forever; this must track the real release instead. diff --git a/test/unit/contract-client-config.test.ts b/test/unit/contract-client-config.test.ts new file mode 100644 index 000000000..37bdce77a --- /dev/null +++ b/test/unit/contract-client-config.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it } from "vitest"; +import { + CLIENT_HOSTS, + CLIENT_HOST_SPEC, + CONNECTION_MODES, + CONNECTION_MODE_SPEC, + REMOTE_MCP_URL, + REMOTE_TOKEN_ENV_VAR, + clientConfigFile, + clientConfigMatrix, + clientConfigSnippet, + compactStringArrays, + supportsConnectionMode, +} from "@loopover/contract/client-config"; + +// #9526: the client-config grid. Its whole job is that a snippet WORKS when pasted, so these assertions are +// about the shapes real clients read -- not about the strings matching some other copy of themselves. + +describe("the grid covers three connection modes across every host (#9526)", () => { + it("names exactly the three modes the issue records, stdio first", () => { + expect(CONNECTION_MODES).toEqual(["stdio", "remote", "miner"]); + }); + + it("every (host, mode) pair either has a snippet or is refused — never a plausible-looking wrong one", () => { + for (const host of CLIENT_HOSTS) { + for (const mode of CONNECTION_MODES) { + if (supportsConnectionMode(host, mode)) expect(clientConfigSnippet(host, mode)).toBeTruthy(); + else expect(() => clientConfigSnippet(host, mode)).toThrow(/cannot connect/i); + } + } + }); + + it("the matrix is every supported pair, and only those", () => { + const matrix = clientConfigMatrix(); + expect(matrix.every((pair) => supportsConnectionMode(pair.host, pair.mode))).toBe(true); + // Every stdio pair, plus remote for the hosts whose remote dialect this repo can actually vouch for. + const remoteHosts = CLIENT_HOSTS.filter((host) => CLIENT_HOST_SPEC[host].supportsRemote); + expect(matrix).toHaveLength(CLIENT_HOSTS.length * 2 + remoteHosts.length); + expect(remoteHosts.length).toBeLessThan(CLIENT_HOSTS.length); + }); + + it("REFUSES a pair it cannot vouch for rather than printing a snippet that fails on paste", () => { + // The generic `mcpServers` bucket is an unnamed host; its remote dialect is a guess, and the stdio + // gateway already serves it the remote tools. + expect(supportsConnectionMode("mcp", "remote")).toBe(false); + expect(() => clientConfigSnippet("mcp", "remote")).toThrow(/cannot connect over/); + }); +}); + +describe("each host gets the shape it actually reads (#9526)", () => { + it("Codex gets a TOML table, not JSON", () => { + const snippet = clientConfigSnippet("codex", "stdio"); + expect(snippet).toContain("[mcp_servers.loopover]"); + expect(snippet).toContain('args = ["--stdio"]'); + expect(snippet).not.toContain("{"); + }); + + it.each(["claude", "cursor", "mcp"] as const)("%s gets the mcpServers shape with no transport type", (host) => { + const config = JSON.parse(clientConfigSnippet(host, "stdio")) as { mcpServers: Record }; + expect(config.mcpServers.loopover).toEqual({ command: "loopover-mcp", args: ["--stdio"] }); + // `mcpServers` hosts infer stdio from `command`; spelling it out would be noise in a file people read. + expect(config.mcpServers.loopover!.type).toBeUndefined(); + }); + + it("VS Code gets a `servers` map WITH the explicit transport type it requires", () => { + const config = JSON.parse(clientConfigSnippet("vscode", "stdio")) as { servers: Record; mcpServers?: unknown }; + expect(config.servers.loopover!.type).toBe("stdio"); + expect(config.mcpServers).toBeUndefined(); + }); + + it("an overridden command lands in the snippet, for a client that does not inherit PATH", () => { + expect(clientConfigSnippet("claude", "stdio", { command: "/opt/bin/loopover-mcp" })).toContain("/opt/bin/loopover-mcp"); + }); +}); + +describe("the remote mode never puts a secret in a config file (#9526)", () => { + it.each(CLIENT_HOSTS.filter((host) => CLIENT_HOST_SPEC[host].supportsRemote))("%s references the env var rather than a token value", (host) => { + const snippet = clientConfigSnippet(host, "remote"); + expect(snippet).toContain(REMOTE_TOKEN_ENV_VAR); + expect(snippet).toContain(REMOTE_MCP_URL); + // A literal token would be in shell history, backups, and eventually a screenshot. + expect(snippet).not.toMatch(/github_pat_|gh[pousr]_|Bearer [A-Za-z0-9]{8}/); + }); + + it("Codex names the variable in its own key; the JSON hosts spell out the header", () => { + expect(clientConfigSnippet("codex", "remote")).toContain(`bearer_token_env_var = "${REMOTE_TOKEN_ENV_VAR}"`); + const config = JSON.parse(clientConfigSnippet("vscode", "remote")) as { servers: Record }> }; + expect(config.servers.loopover).toEqual({ type: "http", url: REMOTE_MCP_URL, headers: { Authorization: `Bearer \${${REMOTE_TOKEN_ENV_VAR}}` } }); + }); + + it("points a remote Claude entry at .mcp.json — Claude Desktop has no config file for this", () => { + expect(clientConfigFile("claude", "remote")).toBe(".mcp.json"); + expect(clientConfigFile("claude", "stdio")).toBe("claude_desktop_config.json"); + }); + + it("falls back to the host's one file when it has no separate remote location", () => { + expect(clientConfigFile("cursor", "remote")).toBe(CLIENT_HOST_SPEC.cursor.file); + }); +}); + +describe("the miner mode stays its own server (#9526)", () => { + it("registers under a different key, with no flags and no login", () => { + const config = JSON.parse(clientConfigSnippet("claude", "miner")) as { mcpServers: Record }; + expect(config.mcpServers["loopover-miner"]).toEqual({ command: "loopover-miner-mcp", args: [] }); + expect(config.mcpServers.loopover).toBeUndefined(); + }); + + it("its keys never collide with the gateway's, so both fit in one client config", () => { + expect(CONNECTION_MODE_SPEC.miner.serverKey).not.toBe(CONNECTION_MODE_SPEC.stdio.serverKey); + }); +}); + +describe("every mode explains itself (#9526)", () => { + it.each(CONNECTION_MODES)("%s carries a summary and at least one actionable note", (mode) => { + const spec = CONNECTION_MODE_SPEC[mode]; + expect(spec.summary.length).toBeGreaterThan(40); + expect(spec.notes.length).toBeGreaterThan(0); + }); + + it("the stdio mode tells a reader how to log in AND how to opt out of the remote mount", () => { + const notes = CONNECTION_MODE_SPEC.stdio.notes.join("\n"); + expect(notes).toContain("loopover-mcp login"); + expect(notes).toContain("--no-remote"); + }); +}); + +describe("snippet formatting (#9526)", () => { + it("keeps a short string array on one line so the block reads as config, not as a list", () => { + expect(compactStringArrays('{\n "args": [\n "--stdio"\n ]\n}')).toBe('{\n "args": ["--stdio"]\n}'); + }); + + it("leaves a long array expanded rather than producing an unreadable line", () => { + const long = JSON.stringify({ args: Array.from({ length: 12 }, (_, index) => `--flag-number-${index}`) }, null, 2); + expect(compactStringArrays(long)).toBe(long); + }); + + it("leaves a non-string array alone", () => { + const numeric = JSON.stringify({ ports: [1, 2, 3] }, null, 2); + expect(compactStringArrays(numeric)).toBe(numeric); + }); +}); diff --git a/test/unit/gen-mcp-client-config-script.test.ts b/test/unit/gen-mcp-client-config-script.test.ts new file mode 100644 index 000000000..9f235e699 --- /dev/null +++ b/test/unit/gen-mcp-client-config-script.test.ts @@ -0,0 +1,98 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { CLIENT_HOSTS, CONNECTION_MODES, CONNECTION_MODE_SPEC, clientConfigSnippet, supportsConnectionMode } from "@loopover/contract/client-config"; +import { docsSection, minerReadmeSection, replaceBetweenMarkers, stdioReadmeSection } from "../../scripts/gen-mcp-client-config"; + +// #9526: the client-config generator. The point of generating these blocks is that the docs and the CLI can +// no longer disagree, so the assertions here are about that property -- every snippet the docs show is +// byte-identical to one the grid produces -- rather than about the prose around them. + +function read(file: string): string { + return readFileSync(join(process.cwd(), file), "utf8"); +} + +describe("the generated docs page (#9526)", () => { + const section = docsSection(); + + it.each(CONNECTION_MODES)("gives %s its own section with its summary and notes", (mode) => { + const spec = CONNECTION_MODE_SPEC[mode]; + expect(section).toContain(`## ${spec.title}`); + expect(section).toContain(spec.summary); + for (const note of spec.notes) expect(section).toContain(note); + }); + + it("shows every supported host/mode pair's snippet VERBATIM", () => { + for (const mode of CONNECTION_MODES) { + for (const host of CLIENT_HOSTS.filter((candidate) => supportsConnectionMode(candidate, mode))) { + // Escaped exactly as the MDX template literal needs; anything else renders as broken JSX. + const escaped = clientConfigSnippet(host, mode).replace(/\\/g, "\\\\").replace(/`/g, "\\`").replace(/\$\{/g, "\\${"); + expect(section, `${host}/${mode} must appear as the grid produces it`).toContain(escaped); + } + } + }); + + it("escapes the `${VAR}` in a remote snippet so MDX does not interpolate it away", () => { + // Unescaped, MDX evaluates ${LOOPOVER_API_TOKEN} as JS and the published page shows a broken header. + expect(section).toContain("Bearer \\${LOOPOVER_API_TOKEN}"); + expect(section).not.toContain('"Bearer ${LOOPOVER_API_TOKEN}"'); + }); + + it("prints one init-client command per pair, so the page and the CLI enumerate the same surface", () => { + for (const mode of CONNECTION_MODES) { + for (const host of CLIENT_HOSTS.filter((candidate) => supportsConnectionMode(candidate, mode))) { + expect(section).toContain(`loopover-mcp init-client --print ${host}${mode === "stdio" ? "" : ` --mode ${mode}`}`); + } + } + }); +}); + +describe("the generated README sections (#9526)", () => { + it("the stdio README documents both the gateway and the remote endpoint", () => { + const section = stdioReadmeSection(); + expect(section).toContain(CONNECTION_MODE_SPEC.stdio.title); + expect(section).toContain(CONNECTION_MODE_SPEC.remote.title); + expect(section).toContain("https://api.loopover.ai/mcp"); + }); + + it("the miner README's combined block registers BOTH servers and stays valid JSON", () => { + const block = minerReadmeSection().match(/```json\r?\n([\s\S]*?)\r?\n```/)![1]!; + const config = JSON.parse(block) as { mcpServers: Record }; + expect(config.mcpServers).toEqual({ + loopover: { command: "loopover-mcp", args: ["--stdio"] }, + "loopover-miner": { command: "loopover-miner-mcp", args: [] }, + }); + }); +}); + +describe("the marker contract (#9526)", () => { + it("replaces only what is between the markers, keeping both in place", () => { + const replaced = replaceBetweenMarkers("head\nBEGIN\nold\nEND\ntail", "new", "BEGIN", "END", "probe"); + expect(replaced).toBe("head\nBEGIN\n\nnew\n\nEND\ntail"); + }); + + it.each([ + ["a missing BEGIN", "END only"], + ["a missing END", "BEGIN only"], + ])("FAILS on %s rather than silently generating nothing", (_label, source) => { + // A generator that quietly writes nowhere is the exact rot this whole family of checks exists to catch. + expect(() => replaceBetweenMarkers(source, "new", "BEGIN", "END", "probe")).toThrow(/missing the GENERATED:MCP-CLIENT-CONFIG markers/); + }); +}); + +describe("the committed surfaces are up to date (#9526)", () => { + it.each([ + ["apps/loopover-ui/content/docs/mcp-clients.mdx", docsSection], + ["packages/loopover-mcp/README.md", stdioReadmeSection], + ["packages/loopover-miner/README.md", minerReadmeSection], + ])("%s contains the current generated block", (file, build) => { + expect(read(file)).toContain(build()); + }); + + it("the docs page has no hand-written config block left outside the markers", () => { + const source = read("apps/loopover-ui/content/docs/mcp-clients.mdx"); + const outside = source.slice(0, source.indexOf("{/* GENERATED:MCP-CLIENT-CONFIG:BEGIN")) + source.slice(source.indexOf("{/* GENERATED:MCP-CLIENT-CONFIG:END */}")); + expect(outside).not.toContain("mcpServers"); + expect(outside).not.toContain("mcp_servers"); + }); +}); diff --git a/test/unit/mcp-cli-basics.test.ts b/test/unit/mcp-cli-basics.test.ts index 3e236e781..0ccec1706 100644 --- a/test/unit/mcp-cli-basics.test.ts +++ b/test/unit/mcp-cli-basics.test.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { closeFixtureServer, createPacketRepo, run, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; import mcpPackageJson from "../../packages/loopover-mcp/package.json"; +import { clientConfigSnippet } from "@loopover/contract/client-config"; // #8587: JSON/plain business-payload cases call the exported runCli in-process (the same dispatcher the // spawned bin runs; pattern from mcp-cli-contributor-profile-inprocess.test.ts). Cases that assert process @@ -92,6 +93,38 @@ describe("loopover-mcp CLI — basics", () => { expect(vscode.snippet).not.toContain('"mcpServers"'); }); + it("prints the remote and miner modes, byte-identical to the docs' own blocks (#9526)", async () => { + const remote = JSON.parse(await runInProcess(["init-client", "--print", "vscode", "--mode", "remote", "--json"])) as { + mode: string; + file: string; + command: string | null; + snippet: string; + notes: string[]; + }; + expect(remote.mode).toBe("remote"); + expect(remote.snippet).toBe(clientConfigSnippet("vscode", "remote")); + // A remote entry has no local process, so reporting a command would be a lie a reader could act on. + expect(remote.command).toBeNull(); + // The token is named, never carried. + expect(remote.snippet).toContain("${LOOPOVER_API_TOKEN}"); + expect(remote.notes.join("\n")).toContain("never paste the token into the config file"); + + const miner = JSON.parse(await runInProcess(["init-client", "--print", "claude", "--mode", "miner", "--json"])) as { snippet: string; args: string[] }; + expect(miner.snippet).toBe(clientConfigSnippet("claude", "miner")); + expect(miner.args).toEqual([]); + }); + + it("defaults to the stdio mode, so a pre-gateway invocation prints what it always did (#9526)", async () => { + const explicit = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--mode", "stdio", "--json"])) as { snippet: string }; + const implicit = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--json"])) as { snippet: string; mode: string }; + expect(implicit.mode).toBe("stdio"); + expect(implicit.snippet).toBe(explicit.snippet); + }); + + it("rejects an unsupported mode by naming the ones that exist (#9526)", async () => { + await expect(runInProcess(["init-client", "--print", "claude", "--mode", "carrier-pigeon"])).rejects.toThrow(/Unsupported mode.*stdio, remote, miner/); + }); + it("prints human-approved agent profile instructions for supported MCP clients", async () => { const payload = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--agent-profile", "miner-planner", "--json"])) as { agentProfile: { diff --git a/test/unit/mcp-discovery-surfaces.test.ts b/test/unit/mcp-discovery-surfaces.test.ts index 8d6e69fc9..b31555086 100644 --- a/test/unit/mcp-discovery-surfaces.test.ts +++ b/test/unit/mcp-discovery-surfaces.test.ts @@ -1,6 +1,10 @@ import { beforeEach, describe, expect, it } from "vitest"; import { + AgentToolsIndexSchema, + AnthropicToolsSchema, + OpenAiToolsSchema, SERVER_CARD_NAME, + ServerCardSchema, buildAgentToolsIndex, buildAnthropicTools, buildOpenAiTools, @@ -18,6 +22,7 @@ import { respondWithDocument, toolsForDeployment, } from "../../src/mcp/discovery-routes"; +import { buildOpenApiSpec } from "../../src/openapi/spec"; // #9526: the discovery surfaces are COMPUTED, never committed — metagraphed's committed server card made // every concurrent tool PR conflict on one generated file. So there is no golden fixture to compare here; @@ -216,3 +221,32 @@ describe("the per-origin memo (#9526)", () => { expect(selfhost.tools.length).not.toBe(cloud.tools.length); }); }); + +describe("the documents are what the API document PROMISES (#9526)", () => { + const tools = listToolDefinitions(); + const context = { version: "1.2.3", deployment: "cloud" as const, baseUrl: "https://api.loopover.ai", tools }; + const agentToolsInput = { baseUrl: context.baseUrl, tools, generatedAt: deterministicGeneratedAt(context.version) }; + + it.each([ + ["/.well-known/mcp.json", ServerCardSchema, () => buildServerCard({ ...context, generatedAt: agentToolsInput.generatedAt })], + ["/.well-known/agent-tools/index.json", AgentToolsIndexSchema, () => buildAgentToolsIndex(agentToolsInput)], + ["/.well-known/agent-tools/openai.json", OpenAiToolsSchema, () => buildOpenAiTools(agentToolsInput)], + ["/.well-known/agent-tools/anthropic.json", AnthropicToolsSchema, () => buildAnthropicTools(agentToolsInput)], + ])("%s validates against the schema the spec publishes", (_path, schema, build) => { + // The builders' return types are INFERRED from these schemas, so a mismatch is normally a compile + // error -- but the JSON Schema in the document is generated from the same object, and this is what + // proves the runtime value satisfies it rather than merely type-checking against it. + expect(schema.safeParse(build()).success).toBe(true); + }); + + it("every discovery path has a specced GET operation with a 200 schema and a 304", () => { + const paths = buildOpenApiSpec().paths as Record } }>; + for (const path of DISCOVERY_PATHS) { + const operation = paths[path]?.get; + expect(operation?.operationId, `${path} must be described in the published document`).toBeTruthy(); + expect(operation!.responses!["200"]!.content, `${path}'s 200 must carry a schema, not just a description`).toBeTruthy(); + // Without the 304 in the document, a generated client has no reason to send if-none-match at all. + expect(operation!.responses!["304"]).toBeTruthy(); + } + }); +}); diff --git a/test/unit/mcp-dispatch-telemetry.test.ts b/test/unit/mcp-dispatch-telemetry.test.ts index 4efe12c81..dcd965039 100644 --- a/test/unit/mcp-dispatch-telemetry.test.ts +++ b/test/unit/mcp-dispatch-telemetry.test.ts @@ -29,10 +29,17 @@ const call: McpToolCallTelemetry = { tool: "loopover_get_repo_context", category describe("MCP telemetry event shapes (#9525)", () => { it("omits error_code on success rather than sending it as null", () => { const properties = buildUsageEventProperties(call); - expect(properties).toEqual({ tool: call.tool, category: "maintainer", surface: "remote", ok: true, duration_ms: 12 }); + expect(properties).toEqual({ tool: call.tool, category: "maintainer", surface: "remote", transport: "local", ok: true, duration_ms: 12 }); expect("error_code" in properties).toBe(false); }); + it("defaults transport to local for a sink with no notion of proxying, and reports it when there is one (#9526)", () => { + // Always emitted, never conditional: a breakdown by transport with an empty `local` bucket would read + // as "nothing runs locally" rather than "most sinks do not set this". + expect(buildUsageEventProperties(call).transport).toBe("local"); + expect(buildUsageEventProperties({ ...call, surface: "stdio", transport: "proxied" }).transport).toBe("proxied"); + }); + it("carries the closed error code on failure", () => { expect(buildUsageEventProperties({ ...call, ok: false, errorCode: "not_found" })).toMatchObject({ ok: false, error_code: "not_found" }); }); @@ -66,7 +73,7 @@ describe("MCP telemetry event shapes (#9525)", () => { it("keeps span attributes a strict subset -- never arguments or the excluded marker", () => { const attributes = buildMcpToolSpanAttributes({ ...call, ok: false, errorCode: "timeout" }); - expect(Object.keys(attributes).sort()).toEqual(["category", "duration_ms", "error_code", "ok", "surface", "tool"]); + expect(Object.keys(attributes).sort()).toEqual(["category", "duration_ms", "error_code", "ok", "surface", "tool", "transport"]); expect(mcpToolSpanName("loopover_x")).toBe("mcp.tool/loopover_x"); }); }); diff --git a/test/unit/mcp-gateway-mount-inprocess.test.ts b/test/unit/mcp-gateway-mount-inprocess.test.ts index 09438f833..205754d3e 100644 --- a/test/unit/mcp-gateway-mount-inprocess.test.ts +++ b/test/unit/mcp-gateway-mount-inprocess.test.ts @@ -104,6 +104,44 @@ describe("mountRemoteTools against the real server (#9526)", () => { expect(result.status === "mounted" && result.skipped).toContain(collidingName); }); + it("mounts a REGISTRY-KNOWN tool with the contract's schemas rather than the wire's", async () => { + // The remote registered from the same contract registry, so the proxy can advertise exactly what the + // remote enforces without this package trusting a schema a remote handed it. + const contractName = "loopover_refresh_repo_focus_manifest"; + const result = await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([{ name: contractName, description: "remote-only in a later release" }]), + }); + expect(result.status === "mounted" && result.tools).toHaveLength(1); + + const client = await connect("gateway-contract-schema"); + try { + const proxied = (await client.listTools()).tools.find((tool) => tool.name === contractName)!; + // The wire descriptor carried NO inputSchema; the registry supplied one. + expect(Object.keys(proxied.inputSchema.properties ?? {}), "the contract's shape, not the wire's silence").toEqual(["owner", "repo"]); + } finally { + await client.close(); + } + }); + + it("a descriptor the SDK refuses costs that tool and nothing else", async () => { + // A remote that repeats a name in one response would otherwise crash the whole mount on the duplicate + // registration — mounting is all-or-nothing only if you let it be. + const result = await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([ + { name: "loopover_gateway_twice", description: "first" }, + { name: "loopover_gateway_twice", description: "a repeat of the same name" }, + { name: "loopover_gateway_survivor", description: "must still mount" }, + ]), + }); + expect(result.status === "mounted" && result.tools.map((tool) => tool.name)).toEqual([ + "loopover_gateway_twice", + "loopover_gateway_survivor", + ]); + expect(result.status === "mounted" && result.skipped).toEqual(["loopover_gateway_twice"]); + }); + it("an unreachable API leaves a server that still lists its LOCAL tools", async () => { const fetchImpl = vi.fn(async () => { throw new Error("ECONNREFUSED"); diff --git a/test/unit/mcp-gateway.test.ts b/test/unit/mcp-gateway.test.ts index 1022010ba..7981114a2 100644 --- a/test/unit/mcp-gateway.test.ts +++ b/test/unit/mcp-gateway.test.ts @@ -96,6 +96,16 @@ describe("every failure degrades to a working local server (#9526)", () => { expect(result.status === "unavailable" && result.advisory).toContain("ECONNREFUSED"); }); + it("names a non-Error rejection too, rather than rendering it as [object Object]", async () => { + // fetch implementations reject with plenty of things that are not Errors; an advisory reading + // "the API was unreachable ([object Object])" tells a contributor nothing. + const fetchImpl = vi.fn(async () => { + throw "ETIMEDOUT from a bare string"; + }) as unknown as GatewayFetch; + const result = await discoverRemoteTools(input({ fetchImpl })); + expect(result.status === "unavailable" && result.advisory).toContain("ETIMEDOUT from a bare string"); + }); + it("a non-2xx reports the STATUS and does not echo the remote body", async () => { // The body is remote-controlled; it has no business in a local advisory string. const fetchImpl = respondingWith({ error: "internal", secretish: "do-not-echo" }, { ok: false, status: 503 }); diff --git a/test/unit/mcp-local-telemetry-chokepoint.test.ts b/test/unit/mcp-local-telemetry-chokepoint.test.ts index 98ab0090b..ed0e2953c 100644 --- a/test/unit/mcp-local-telemetry-chokepoint.test.ts +++ b/test/unit/mcp-local-telemetry-chokepoint.test.ts @@ -146,11 +146,11 @@ describe("loopover-mcp local telemetry chokepoint (#6238)", () => { // two separate sets so a future field of OURS can never hide among the vendor's. const allowedByEvent: Record = { mcp_tool_call: ["caller_type", "duration_ms", "ok", "tool"], - usage_event: ["category", "duration_ms", "ok", "surface", "tool"], + usage_event: ["category", "duration_ms", "ok", "surface", "tool", "transport"], // No `arguments`/`result`: payloads are excluded for every tool by default (#9525). This test // is what established that -- the first design included them, and this assertion found a real // commit message on the wire. - $mcp_tool_call: ["category", "duration_ms", "ok", "payloads_excluded", "surface", "tool"], + $mcp_tool_call: ["category", "duration_ms", "ok", "payloads_excluded", "surface", "tool", "transport"], }; for (const event of received) { const properties = Object.keys(event.properties ?? {}); diff --git a/test/unit/mcp-local-telemetry.test.ts b/test/unit/mcp-local-telemetry.test.ts index 4d27f4fb9..7e8fa12a6 100644 --- a/test/unit/mcp-local-telemetry.test.ts +++ b/test/unit/mcp-local-telemetry.test.ts @@ -361,6 +361,42 @@ describe("recordStdioToolTelemetry / wrapStdioToolHandler (#8690)", () => { expect(h.captureExceptionSpy).toHaveBeenCalledOnce(); }); + // #9526: gateway adoption is measurable only if a proxied call is distinguishable from a local one on the + // wire. `surface` says which server was asked; only `transport` says which path the call actually took. + it("wrapStdioToolHandler tags a PROXIED call so gateway adoption is measurable", async () => { + vi.stubEnv("LOOPOVER_MCP_POSTHOG_API_KEY", "phc_test"); + const wrapped = wrapStdioToolHandler("loopover_lint_pr_text", () => true, async () => ({ structuredContent: { ok: true } }), "proxied"); + await wrapped({}); + + const usage = h.captureSpy.mock.calls.map((entry) => entry[0] as CapturedMessage).find((message) => message.event === "usage_event")!; + expect(usage.properties).toMatchObject({ surface: "stdio", transport: "proxied" }); + }); + + it("wrapStdioToolHandler tags a local call `local` without the caller saying so", async () => { + vi.stubEnv("LOOPOVER_MCP_POSTHOG_API_KEY", "phc_test"); + const wrapped = wrapStdioToolHandler("loopover_lint_pr_text", () => true, async () => ({ structuredContent: { ok: true } })); + await wrapped({}); + + const usage = h.captureSpy.mock.calls.map((entry) => entry[0] as CapturedMessage).find((message) => message.event === "usage_event")!; + expect(usage.properties).toMatchObject({ transport: "local" }); + }); + + it("carries the transport through the THROW path too — a failed proxy is still a proxied call", async () => { + vi.stubEnv("LOOPOVER_MCP_POSTHOG_API_KEY", "phc_test"); + const wrapped = wrapStdioToolHandler( + "loopover_lint_pr_text", + () => true, + async () => { + throw new Error("the remote refused"); + }, + "proxied", + ); + await expect(wrapped({})).rejects.toThrow("the remote refused"); + + const usage = h.captureSpy.mock.calls.map((entry) => entry[0] as CapturedMessage).find((message) => message.event === "usage_event")!; + expect(usage.properties).toMatchObject({ ok: false, transport: "proxied" }); + }); + it("wrapStdioToolHandler is a no-op for PostHog when telemetry is disabled", async () => { vi.stubEnv("LOOPOVER_MCP_POSTHOG_API_KEY", "phc_test"); const wrapped = wrapStdioToolHandler("loopover_demo", () => false, async () => ({ ok: true })); diff --git a/vitest.config.ts b/vitest.config.ts index c14b5845e..fa983b972 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,7 +1,35 @@ +import { readFileSync } from "node:fs"; import { defineConfig } from "vitest/config"; const junitPath = process.env.VITEST_JUNIT_PATH; +/** + * The @loopover/contract source aliases, DERIVED from that package's own "exports" map (#9526). + * + * This list used to be typed out by hand, one line per subpath, and adding an export without also adding + * its alias produced a "Cannot find package" that names the subpath but not the reason -- a trap that costs + * whoever hits it a confused half hour. The exports map already enumerates every subpath, so read it. + * + * Longest specifier FIRST. Vite's string-`find` alias matcher treats a plain string as matching both the + * exact specifier and anything starting with `find + "/"`, first-match-wins in declaration order -- so with + * the bare "@loopover/contract" entry first it silently intercepted "@loopover/contract/tools" too and + * rewrote it to a "/tools" that resolved nowhere. + */ +function contractSourceAliases(): Record { + const manifest = JSON.parse(readFileSync(new URL("./packages/loopover-contract/package.json", import.meta.url).pathname, "utf8")) as { + exports: Record; + }; + const entries: Array<[string, string]> = []; + for (const [subpath, target] of Object.entries(manifest.exports)) { + const dist = typeof target === "string" ? target : target.default; + // "./package.json" maps to a literal file, not a module; nothing to point at source. + if (!dist?.endsWith(".js")) continue; + const source = dist.replace(/^\.\/dist\//, "./packages/loopover-contract/src/").replace(/\.js$/, ".ts"); + entries.push([`@loopover/contract${subpath.replace(/^\./, "")}`, new URL(source, import.meta.url).pathname]); + } + return Object.fromEntries(entries.sort(([a], [b]) => b.length - a.length)); +} + export default defineConfig({ ssr: { noExternal: ["agents", "partyserver"], @@ -21,20 +49,8 @@ export default defineConfig({ // for free from their relative-import resolution; this closes the same gap for a package // that's actually installed as a workspace dependency. // - // "/tools" MUST be listed before the bare "@loopover/contract" entry: Vite's string-`find` - // alias matcher treats a plain string as matching BOTH the exact specifier and anything - // starting with `find + "/"`, first-match-wins in declaration order -- so with the bare entry - // first, it silently intercepted "@loopover/contract/tools" too and rewrote it to a bogus - // "/tools" that resolved nowhere. Confirmed by reproducing the failure with a - // throwaway probe test before reordering, not assumed from reading Vite's docs alone. - "@loopover/contract/discovery": new URL("./packages/loopover-contract/src/discovery.ts", import.meta.url).pathname, - "@loopover/contract/enums": new URL("./packages/loopover-contract/src/enums.ts", import.meta.url).pathname, - "@loopover/contract/tools": new URL("./packages/loopover-contract/src/tools/index.ts", import.meta.url).pathname, - "@loopover/contract/cli-config": new URL("./packages/loopover-contract/src/cli-config.ts", import.meta.url).pathname, - "@loopover/contract/orb-broker": new URL("./packages/loopover-contract/src/orb-broker.ts", import.meta.url).pathname, - "@loopover/contract/api-schemas": new URL("./packages/loopover-contract/src/api-schemas.ts", import.meta.url).pathname, - "@loopover/contract/public-api": new URL("./packages/loopover-contract/src/public-api.ts", import.meta.url).pathname, - "@loopover/contract": new URL("./packages/loopover-contract/src/index.ts", import.meta.url).pathname, + // Ordering and derivation both live in contractSourceAliases() above. + ...contractSourceAliases(), }, }, test: { From fbb402369fbe8cefdd7679c8c6d2e938f3d94baf Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:53:07 -0700 Subject: [PATCH 4/6] fix(mcp): pin the publisher binary by checksum and gate the publish job (#9526) Three defects in the registry-publish workflow, two of them raised by the security scan and all three real. The install step's comment said "SHA-pinned" while the code floated on a release tag, which a maintainer can move to a different commit after review -- for a binary that authenticates as this repository and writes to a public registry. It now verifies a sha256 of the download before extracting it, so a moved tag fails the job rather than publishing under bytes nobody looked at. The job holds `id-token: write` and had no deployment environment, so protection rules had nothing to attach to and anyone able to dispatch a workflow could publish. It now declares one. And the download would have 404'd regardless: the asset was requested as `mcp-publisher_${VERSION}_linux_amd64.tar.gz`, but the release publishes it with no version in the name. A publish workflow that cannot fetch its own tool is a workflow nobody has run, which is exactly the class the anti-rot guard exists for -- so the guard now covers the workflow itself: dispatch-only, main-only, environment present, permissions minimal, actions sha-pinned, the checksum verified before extraction, and the manifest check ordered before login and publish. Also fixes a genuine flake this PR's CI hit twice, unrelated to the change: selfhost-pg-queue's recentDeadCount assertion compared the bound cutoff only against a timestamp captured BEFORE the call, while the implementation reads the clock after it -- so one millisecond ticking between the two failed the test (1785302335173 <= 1785302335172). Bracketed on both sides now. --- .github/workflows/publish-mcp-registry.yml | 14 +- apps/loopover-ui/src/routeTree.gen.ts | 562 +++++++++--------- .../publish-mcp-registry-workflow.test.ts | 95 +++ test/unit/selfhost-pg-queue.test.ts | 11 +- 4 files changed, 395 insertions(+), 287 deletions(-) create mode 100644 test/unit/publish-mcp-registry-workflow.test.ts diff --git a/.github/workflows/publish-mcp-registry.yml b/.github/workflows/publish-mcp-registry.yml index 59ec2df71..8525e65c1 100644 --- a/.github/workflows/publish-mcp-registry.yml +++ b/.github/workflows/publish-mcp-registry.yml @@ -35,6 +35,10 @@ jobs: publish: name: Publish server.json runs-on: ubuntu-latest + # A deployment environment, so the one job in this repo that holds `id-token: write` and writes to a + # public registry can carry protection rules (required reviewers, a wait timer) configured outside the + # workflow file. Without it, anyone who can dispatch a workflow can publish. + environment: mcp-registry # Belt and braces alongside the dispatch-only trigger: a dispatch can name any ref, and a registry # publish must only ever describe what is on main. if: github.ref == 'refs/heads/main' @@ -54,12 +58,16 @@ jobs: - name: Install mcp-publisher env: - # SHA-pinned: this binary authenticates as this repo and writes to a public registry, so it is not - # something to float on a moving tag. - MCP_PUBLISHER_VERSION: v1.2.3 + MCP_PUBLISHER_VERSION: v1.8.0 + # The tag is mutable; this is not. A release tag can be moved to point at a different commit, and + # this binary authenticates as this repository and writes to a public registry -- so the download + # is checked against the exact bytes reviewed here, and a mismatch fails the job rather than + # publishing under a binary nobody looked at. + MCP_PUBLISHER_SHA256: 1370446bbe74d562608e8005a6ccce02d146a661fbd78674e11cc70b9618d6cf run: | set -euo pipefail curl -fsSL "https://github.com/modelcontextprotocol/registry/releases/download/${MCP_PUBLISHER_VERSION}/mcp-publisher_linux_amd64.tar.gz" -o mcp-publisher.tar.gz + echo "${MCP_PUBLISHER_SHA256} mcp-publisher.tar.gz" | sha256sum --check --strict - tar -xzf mcp-publisher.tar.gz mcp-publisher chmod +x mcp-publisher diff --git a/apps/loopover-ui/src/routeTree.gen.ts b/apps/loopover-ui/src/routeTree.gen.ts index 1409d48c9..7ac38456e 100644 --- a/apps/loopover-ui/src/routeTree.gen.ts +++ b/apps/loopover-ui/src/routeTree.gen.ts @@ -9,63 +9,63 @@ // Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified. import { Route as rootRouteImport } from './routes/__root' -import { Route as RoadmapRouteImport } from './routes/roadmap' -import { Route as MinersRouteImport } from './routes/miners' -import { Route as MaintainersRouteImport } from './routes/maintainers' -import { Route as InstallRouteImport } from './routes/install' -import { Route as FairnessRouteImport } from './routes/fairness' -import { Route as DocsRouteImport } from './routes/docs' -import { Route as ChangelogRouteImport } from './routes/changelog' -import { Route as AppRouteImport } from './routes/app' -import { Route as ApiRouteImport } from './routes/api' -import { Route as AgentsRouteImport } from './routes/agents' import { Route as IndexRouteImport } from './routes/index' -import { Route as InstallIndexRouteImport } from './routes/install.index' -import { Route as DocsIndexRouteImport } from './routes/docs.index' -import { Route as AppIndexRouteImport } from './routes/app.index' +import { Route as AgentsRouteImport } from './routes/agents' +import { Route as ApiRouteImport } from './routes/api' +import { Route as AppRouteImport } from './routes/app' +import { Route as ChangelogRouteImport } from './routes/changelog' +import { Route as DocsRouteImport } from './routes/docs' +import { Route as FairnessRouteImport } from './routes/fairness' +import { Route as InstallRouteImport } from './routes/install' +import { Route as MaintainersRouteImport } from './routes/maintainers' +import { Route as MinersRouteImport } from './routes/miners' +import { Route as RoadmapRouteImport } from './routes/roadmap' import { Route as ApiIndexRouteImport } from './routes/api.index' -import { Route as InstallPermissionsRouteImport } from './routes/install.permissions' -import { Route as DocsFumadocsSpikeApiReferenceRouteImport } from './routes/docs.fumadocs-spike-api-reference' -import { Route as DocsSlugRouteImport } from './routes/docs.$slug' -import { Route as AppWorkbenchRouteImport } from './routes/app.workbench' -import { Route as AppRunsRouteImport } from './routes/app.runs' -import { Route as AppReposRouteImport } from './routes/app.repos' -import { Route as AppPlaygroundRouteImport } from './routes/app.playground' -import { Route as AppOwnerRouteImport } from './routes/app.owner' -import { Route as AppOperatorRouteImport } from './routes/app.operator' -import { Route as AppMinerRouteImport } from './routes/app.miner' -import { Route as AppMaintainerRouteImport } from './routes/app.maintainer' -import { Route as AppDigestRouteImport } from './routes/app.digest' -import { Route as AppConfigGeneratorRouteImport } from './routes/app.config-generator' -import { Route as AppCommandsRouteImport } from './routes/app.commands' -import { Route as AppAuditRouteImport } from './routes/app.audit' -import { Route as AppAnalyticsRouteImport } from './routes/app.analytics' import { Route as ApiOpRouteImport } from './routes/api.$op' +import { Route as AppIndexRouteImport } from './routes/app.index' +import { Route as AppAnalyticsRouteImport } from './routes/app.analytics' +import { Route as AppAuditRouteImport } from './routes/app.audit' +import { Route as AppCommandsRouteImport } from './routes/app.commands' +import { Route as AppConfigGeneratorRouteImport } from './routes/app.config-generator' +import { Route as AppDigestRouteImport } from './routes/app.digest' +import { Route as AppMaintainerRouteImport } from './routes/app.maintainer' +import { Route as AppMinerRouteImport } from './routes/app.miner' +import { Route as AppOperatorRouteImport } from './routes/app.operator' +import { Route as AppOwnerRouteImport } from './routes/app.owner' +import { Route as AppPlaygroundRouteImport } from './routes/app.playground' +import { Route as AppReposRouteImport } from './routes/app.repos' +import { Route as AppRunsRouteImport } from './routes/app.runs' +import { Route as AppWorkbenchRouteImport } from './routes/app.workbench' +import { Route as DocsIndexRouteImport } from './routes/docs.index' +import { Route as DocsSlugRouteImport } from './routes/docs.$slug' +import { Route as DocsFumadocsSpikeApiReferenceRouteImport } from './routes/docs.fumadocs-spike-api-reference' +import { Route as InstallIndexRouteImport } from './routes/install.index' +import { Route as InstallPermissionsRouteImport } from './routes/install.permissions' import { Route as ReposOwnerRepoQualityRouteImport } from './routes/repos.$owner.$repo.quality' -const RoadmapRoute = RoadmapRouteImport.update({ - id: '/roadmap', - path: '/roadmap', +const IndexRoute = IndexRouteImport.update({ + id: '/', + path: '/', getParentRoute: () => rootRouteImport, } as any) -const MinersRoute = MinersRouteImport.update({ - id: '/miners', - path: '/miners', +const AgentsRoute = AgentsRouteImport.update({ + id: '/agents', + path: '/agents', getParentRoute: () => rootRouteImport, } as any) -const MaintainersRoute = MaintainersRouteImport.update({ - id: '/maintainers', - path: '/maintainers', +const ApiRoute = ApiRouteImport.update({ + id: '/api', + path: '/api', getParentRoute: () => rootRouteImport, } as any) -const InstallRoute = InstallRouteImport.update({ - id: '/install', - path: '/install', +const AppRoute = AppRouteImport.update({ + id: '/app', + path: '/app', getParentRoute: () => rootRouteImport, } as any) -const FairnessRoute = FairnessRouteImport.update({ - id: '/fairness', - path: '/fairness', +const ChangelogRoute = ChangelogRouteImport.update({ + id: '/changelog', + path: '/changelog', getParentRoute: () => rootRouteImport, } as any) const DocsRoute = DocsRouteImport.update({ @@ -73,95 +73,74 @@ const DocsRoute = DocsRouteImport.update({ path: '/docs', getParentRoute: () => rootRouteImport, } as any) -const ChangelogRoute = ChangelogRouteImport.update({ - id: '/changelog', - path: '/changelog', +const FairnessRoute = FairnessRouteImport.update({ + id: '/fairness', + path: '/fairness', getParentRoute: () => rootRouteImport, } as any) -const AppRoute = AppRouteImport.update({ - id: '/app', - path: '/app', +const InstallRoute = InstallRouteImport.update({ + id: '/install', + path: '/install', getParentRoute: () => rootRouteImport, } as any) -const ApiRoute = ApiRouteImport.update({ - id: '/api', - path: '/api', +const MaintainersRoute = MaintainersRouteImport.update({ + id: '/maintainers', + path: '/maintainers', getParentRoute: () => rootRouteImport, } as any) -const AgentsRoute = AgentsRouteImport.update({ - id: '/agents', - path: '/agents', +const MinersRoute = MinersRouteImport.update({ + id: '/miners', + path: '/miners', getParentRoute: () => rootRouteImport, } as any) -const IndexRoute = IndexRouteImport.update({ - id: '/', - path: '/', +const RoadmapRoute = RoadmapRouteImport.update({ + id: '/roadmap', + path: '/roadmap', getParentRoute: () => rootRouteImport, } as any) -const InstallIndexRoute = InstallIndexRouteImport.update({ +const ApiIndexRoute = ApiIndexRouteImport.update({ id: '/', path: '/', - getParentRoute: () => InstallRoute, + getParentRoute: () => ApiRoute, } as any) -const DocsIndexRoute = DocsIndexRouteImport.update({ - id: '/', - path: '/', - getParentRoute: () => DocsRoute, +const ApiOpRoute = ApiOpRouteImport.update({ + id: '/$op', + path: '/$op', + getParentRoute: () => ApiRoute, } as any) const AppIndexRoute = AppIndexRouteImport.update({ id: '/', path: '/', getParentRoute: () => AppRoute, } as any) -const ApiIndexRoute = ApiIndexRouteImport.update({ - id: '/', - path: '/', - getParentRoute: () => ApiRoute, -} as any) -const InstallPermissionsRoute = InstallPermissionsRouteImport.update({ - id: '/permissions', - path: '/permissions', - getParentRoute: () => InstallRoute, -} as any) -const DocsFumadocsSpikeApiReferenceRoute = - DocsFumadocsSpikeApiReferenceRouteImport.update({ - id: '/fumadocs-spike-api-reference', - path: '/fumadocs-spike-api-reference', - getParentRoute: () => DocsRoute, - } as any) -const DocsSlugRoute = DocsSlugRouteImport.update({ - id: '/$slug', - path: '/$slug', - getParentRoute: () => DocsRoute, -} as any) -const AppWorkbenchRoute = AppWorkbenchRouteImport.update({ - id: '/workbench', - path: '/workbench', +const AppAnalyticsRoute = AppAnalyticsRouteImport.update({ + id: '/analytics', + path: '/analytics', getParentRoute: () => AppRoute, } as any) -const AppRunsRoute = AppRunsRouteImport.update({ - id: '/runs', - path: '/runs', +const AppAuditRoute = AppAuditRouteImport.update({ + id: '/audit', + path: '/audit', getParentRoute: () => AppRoute, } as any) -const AppReposRoute = AppReposRouteImport.update({ - id: '/repos', - path: '/repos', +const AppCommandsRoute = AppCommandsRouteImport.update({ + id: '/commands', + path: '/commands', getParentRoute: () => AppRoute, } as any) -const AppPlaygroundRoute = AppPlaygroundRouteImport.update({ - id: '/playground', - path: '/playground', +const AppConfigGeneratorRoute = AppConfigGeneratorRouteImport.update({ + id: '/config-generator', + path: '/config-generator', getParentRoute: () => AppRoute, } as any) -const AppOwnerRoute = AppOwnerRouteImport.update({ - id: '/owner', - path: '/owner', +const AppDigestRoute = AppDigestRouteImport.update({ + id: '/digest', + path: '/digest', getParentRoute: () => AppRoute, } as any) -const AppOperatorRoute = AppOperatorRouteImport.update({ - id: '/operator', - path: '/operator', +const AppMaintainerRoute = AppMaintainerRouteImport.update({ + id: '/maintainer', + path: '/maintainer', getParentRoute: () => AppRoute, } as any) const AppMinerRoute = AppMinerRouteImport.update({ @@ -169,40 +148,61 @@ const AppMinerRoute = AppMinerRouteImport.update({ path: '/miner', getParentRoute: () => AppRoute, } as any) -const AppMaintainerRoute = AppMaintainerRouteImport.update({ - id: '/maintainer', - path: '/maintainer', +const AppOperatorRoute = AppOperatorRouteImport.update({ + id: '/operator', + path: '/operator', getParentRoute: () => AppRoute, } as any) -const AppDigestRoute = AppDigestRouteImport.update({ - id: '/digest', - path: '/digest', +const AppOwnerRoute = AppOwnerRouteImport.update({ + id: '/owner', + path: '/owner', getParentRoute: () => AppRoute, } as any) -const AppConfigGeneratorRoute = AppConfigGeneratorRouteImport.update({ - id: '/config-generator', - path: '/config-generator', +const AppPlaygroundRoute = AppPlaygroundRouteImport.update({ + id: '/playground', + path: '/playground', getParentRoute: () => AppRoute, } as any) -const AppCommandsRoute = AppCommandsRouteImport.update({ - id: '/commands', - path: '/commands', +const AppReposRoute = AppReposRouteImport.update({ + id: '/repos', + path: '/repos', getParentRoute: () => AppRoute, } as any) -const AppAuditRoute = AppAuditRouteImport.update({ - id: '/audit', - path: '/audit', +const AppRunsRoute = AppRunsRouteImport.update({ + id: '/runs', + path: '/runs', getParentRoute: () => AppRoute, } as any) -const AppAnalyticsRoute = AppAnalyticsRouteImport.update({ - id: '/analytics', - path: '/analytics', +const AppWorkbenchRoute = AppWorkbenchRouteImport.update({ + id: '/workbench', + path: '/workbench', getParentRoute: () => AppRoute, } as any) -const ApiOpRoute = ApiOpRouteImport.update({ - id: '/$op', - path: '/$op', - getParentRoute: () => ApiRoute, +const DocsIndexRoute = DocsIndexRouteImport.update({ + id: '/', + path: '/', + getParentRoute: () => DocsRoute, +} as any) +const DocsSlugRoute = DocsSlugRouteImport.update({ + id: '/$slug', + path: '/$slug', + getParentRoute: () => DocsRoute, +} as any) +const DocsFumadocsSpikeApiReferenceRoute = + DocsFumadocsSpikeApiReferenceRouteImport.update({ + id: '/fumadocs-spike-api-reference', + path: '/fumadocs-spike-api-reference', + getParentRoute: () => DocsRoute, + } as any) +const InstallIndexRoute = InstallIndexRouteImport.update({ + id: '/', + path: '/', + getParentRoute: () => InstallRoute, +} as any) +const InstallPermissionsRoute = InstallPermissionsRouteImport.update({ + id: '/permissions', + path: '/permissions', + getParentRoute: () => InstallRoute, } as any) const ReposOwnerRepoQualityRoute = ReposOwnerRepoQualityRouteImport.update({ id: '/repos/$owner/$repo/quality', @@ -433,39 +433,39 @@ export interface RootRouteChildren { declare module '@tanstack/react-router' { interface FileRoutesByPath { - '/roadmap': { - id: '/roadmap' - path: '/roadmap' - fullPath: '/roadmap' - preLoaderRoute: typeof RoadmapRouteImport + '/': { + id: '/' + path: '/' + fullPath: '/' + preLoaderRoute: typeof IndexRouteImport parentRoute: typeof rootRouteImport } - '/miners': { - id: '/miners' - path: '/miners' - fullPath: '/miners' - preLoaderRoute: typeof MinersRouteImport + '/agents': { + id: '/agents' + path: '/agents' + fullPath: '/agents' + preLoaderRoute: typeof AgentsRouteImport parentRoute: typeof rootRouteImport } - '/maintainers': { - id: '/maintainers' - path: '/maintainers' - fullPath: '/maintainers' - preLoaderRoute: typeof MaintainersRouteImport + '/api': { + id: '/api' + path: '/api' + fullPath: '/api' + preLoaderRoute: typeof ApiRouteImport parentRoute: typeof rootRouteImport } - '/install': { - id: '/install' - path: '/install' - fullPath: '/install' - preLoaderRoute: typeof InstallRouteImport + '/app': { + id: '/app' + path: '/app' + fullPath: '/app' + preLoaderRoute: typeof AppRouteImport parentRoute: typeof rootRouteImport } - '/fairness': { - id: '/fairness' - path: '/fairness' - fullPath: '/fairness' - preLoaderRoute: typeof FairnessRouteImport + '/changelog': { + id: '/changelog' + path: '/changelog' + fullPath: '/changelog' + preLoaderRoute: typeof ChangelogRouteImport parentRoute: typeof rootRouteImport } '/docs': { @@ -475,54 +475,54 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof DocsRouteImport parentRoute: typeof rootRouteImport } - '/changelog': { - id: '/changelog' - path: '/changelog' - fullPath: '/changelog' - preLoaderRoute: typeof ChangelogRouteImport + '/fairness': { + id: '/fairness' + path: '/fairness' + fullPath: '/fairness' + preLoaderRoute: typeof FairnessRouteImport parentRoute: typeof rootRouteImport } - '/app': { - id: '/app' - path: '/app' - fullPath: '/app' - preLoaderRoute: typeof AppRouteImport + '/install': { + id: '/install' + path: '/install' + fullPath: '/install' + preLoaderRoute: typeof InstallRouteImport parentRoute: typeof rootRouteImport } - '/api': { - id: '/api' - path: '/api' - fullPath: '/api' - preLoaderRoute: typeof ApiRouteImport + '/maintainers': { + id: '/maintainers' + path: '/maintainers' + fullPath: '/maintainers' + preLoaderRoute: typeof MaintainersRouteImport parentRoute: typeof rootRouteImport } - '/agents': { - id: '/agents' - path: '/agents' - fullPath: '/agents' - preLoaderRoute: typeof AgentsRouteImport + '/miners': { + id: '/miners' + path: '/miners' + fullPath: '/miners' + preLoaderRoute: typeof MinersRouteImport parentRoute: typeof rootRouteImport } - '/': { - id: '/' - path: '/' - fullPath: '/' - preLoaderRoute: typeof IndexRouteImport + '/roadmap': { + id: '/roadmap' + path: '/roadmap' + fullPath: '/roadmap' + preLoaderRoute: typeof RoadmapRouteImport parentRoute: typeof rootRouteImport } - '/install/': { - id: '/install/' + '/api/': { + id: '/api/' path: '/' - fullPath: '/install/' - preLoaderRoute: typeof InstallIndexRouteImport - parentRoute: typeof InstallRoute + fullPath: '/api/' + preLoaderRoute: typeof ApiIndexRouteImport + parentRoute: typeof ApiRoute } - '/docs/': { - id: '/docs/' - path: '/' - fullPath: '/docs/' - preLoaderRoute: typeof DocsIndexRouteImport - parentRoute: typeof DocsRoute + '/api/$op': { + id: '/api/$op' + path: '/$op' + fullPath: '/api/$op' + preLoaderRoute: typeof ApiOpRouteImport + parentRoute: typeof ApiRoute } '/app/': { id: '/app/' @@ -531,74 +531,46 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AppIndexRouteImport parentRoute: typeof AppRoute } - '/api/': { - id: '/api/' - path: '/' - fullPath: '/api/' - preLoaderRoute: typeof ApiIndexRouteImport - parentRoute: typeof ApiRoute - } - '/install/permissions': { - id: '/install/permissions' - path: '/permissions' - fullPath: '/install/permissions' - preLoaderRoute: typeof InstallPermissionsRouteImport - parentRoute: typeof InstallRoute - } - '/docs/fumadocs-spike-api-reference': { - id: '/docs/fumadocs-spike-api-reference' - path: '/fumadocs-spike-api-reference' - fullPath: '/docs/fumadocs-spike-api-reference' - preLoaderRoute: typeof DocsFumadocsSpikeApiReferenceRouteImport - parentRoute: typeof DocsRoute - } - '/docs/$slug': { - id: '/docs/$slug' - path: '/$slug' - fullPath: '/docs/$slug' - preLoaderRoute: typeof DocsSlugRouteImport - parentRoute: typeof DocsRoute - } - '/app/workbench': { - id: '/app/workbench' - path: '/workbench' - fullPath: '/app/workbench' - preLoaderRoute: typeof AppWorkbenchRouteImport + '/app/analytics': { + id: '/app/analytics' + path: '/analytics' + fullPath: '/app/analytics' + preLoaderRoute: typeof AppAnalyticsRouteImport parentRoute: typeof AppRoute } - '/app/runs': { - id: '/app/runs' - path: '/runs' - fullPath: '/app/runs' - preLoaderRoute: typeof AppRunsRouteImport + '/app/audit': { + id: '/app/audit' + path: '/audit' + fullPath: '/app/audit' + preLoaderRoute: typeof AppAuditRouteImport parentRoute: typeof AppRoute } - '/app/repos': { - id: '/app/repos' - path: '/repos' - fullPath: '/app/repos' - preLoaderRoute: typeof AppReposRouteImport + '/app/commands': { + id: '/app/commands' + path: '/commands' + fullPath: '/app/commands' + preLoaderRoute: typeof AppCommandsRouteImport parentRoute: typeof AppRoute } - '/app/playground': { - id: '/app/playground' - path: '/playground' - fullPath: '/app/playground' - preLoaderRoute: typeof AppPlaygroundRouteImport + '/app/config-generator': { + id: '/app/config-generator' + path: '/config-generator' + fullPath: '/app/config-generator' + preLoaderRoute: typeof AppConfigGeneratorRouteImport parentRoute: typeof AppRoute } - '/app/owner': { - id: '/app/owner' - path: '/owner' - fullPath: '/app/owner' - preLoaderRoute: typeof AppOwnerRouteImport + '/app/digest': { + id: '/app/digest' + path: '/digest' + fullPath: '/app/digest' + preLoaderRoute: typeof AppDigestRouteImport parentRoute: typeof AppRoute } - '/app/operator': { - id: '/app/operator' - path: '/operator' - fullPath: '/app/operator' - preLoaderRoute: typeof AppOperatorRouteImport + '/app/maintainer': { + id: '/app/maintainer' + path: '/maintainer' + fullPath: '/app/maintainer' + preLoaderRoute: typeof AppMaintainerRouteImport parentRoute: typeof AppRoute } '/app/miner': { @@ -608,54 +580,82 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AppMinerRouteImport parentRoute: typeof AppRoute } - '/app/maintainer': { - id: '/app/maintainer' - path: '/maintainer' - fullPath: '/app/maintainer' - preLoaderRoute: typeof AppMaintainerRouteImport + '/app/operator': { + id: '/app/operator' + path: '/operator' + fullPath: '/app/operator' + preLoaderRoute: typeof AppOperatorRouteImport parentRoute: typeof AppRoute } - '/app/digest': { - id: '/app/digest' - path: '/digest' - fullPath: '/app/digest' - preLoaderRoute: typeof AppDigestRouteImport + '/app/owner': { + id: '/app/owner' + path: '/owner' + fullPath: '/app/owner' + preLoaderRoute: typeof AppOwnerRouteImport parentRoute: typeof AppRoute } - '/app/config-generator': { - id: '/app/config-generator' - path: '/config-generator' - fullPath: '/app/config-generator' - preLoaderRoute: typeof AppConfigGeneratorRouteImport + '/app/playground': { + id: '/app/playground' + path: '/playground' + fullPath: '/app/playground' + preLoaderRoute: typeof AppPlaygroundRouteImport parentRoute: typeof AppRoute } - '/app/commands': { - id: '/app/commands' - path: '/commands' - fullPath: '/app/commands' - preLoaderRoute: typeof AppCommandsRouteImport + '/app/repos': { + id: '/app/repos' + path: '/repos' + fullPath: '/app/repos' + preLoaderRoute: typeof AppReposRouteImport parentRoute: typeof AppRoute } - '/app/audit': { - id: '/app/audit' - path: '/audit' - fullPath: '/app/audit' - preLoaderRoute: typeof AppAuditRouteImport + '/app/runs': { + id: '/app/runs' + path: '/runs' + fullPath: '/app/runs' + preLoaderRoute: typeof AppRunsRouteImport parentRoute: typeof AppRoute } - '/app/analytics': { - id: '/app/analytics' - path: '/analytics' - fullPath: '/app/analytics' - preLoaderRoute: typeof AppAnalyticsRouteImport + '/app/workbench': { + id: '/app/workbench' + path: '/workbench' + fullPath: '/app/workbench' + preLoaderRoute: typeof AppWorkbenchRouteImport parentRoute: typeof AppRoute } - '/api/$op': { - id: '/api/$op' - path: '/$op' - fullPath: '/api/$op' - preLoaderRoute: typeof ApiOpRouteImport - parentRoute: typeof ApiRoute + '/docs/': { + id: '/docs/' + path: '/' + fullPath: '/docs/' + preLoaderRoute: typeof DocsIndexRouteImport + parentRoute: typeof DocsRoute + } + '/docs/$slug': { + id: '/docs/$slug' + path: '/$slug' + fullPath: '/docs/$slug' + preLoaderRoute: typeof DocsSlugRouteImport + parentRoute: typeof DocsRoute + } + '/docs/fumadocs-spike-api-reference': { + id: '/docs/fumadocs-spike-api-reference' + path: '/fumadocs-spike-api-reference' + fullPath: '/docs/fumadocs-spike-api-reference' + preLoaderRoute: typeof DocsFumadocsSpikeApiReferenceRouteImport + parentRoute: typeof DocsRoute + } + '/install/': { + id: '/install/' + path: '/' + fullPath: '/install/' + preLoaderRoute: typeof InstallIndexRouteImport + parentRoute: typeof InstallRoute + } + '/install/permissions': { + id: '/install/permissions' + path: '/permissions' + fullPath: '/install/permissions' + preLoaderRoute: typeof InstallPermissionsRouteImport + parentRoute: typeof InstallRoute } '/repos/$owner/$repo/quality': { id: '/repos/$owner/$repo/quality' diff --git a/test/unit/publish-mcp-registry-workflow.test.ts b/test/unit/publish-mcp-registry-workflow.test.ts new file mode 100644 index 000000000..08e134ab6 --- /dev/null +++ b/test/unit/publish-mcp-registry-workflow.test.ts @@ -0,0 +1,95 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; +import { parse } from "yaml"; + +// #9526: the registry-publish workflow's own safety properties. +// +// This is the one job in the repo that holds `id-token: write` and writes to a PUBLIC registry under this +// repository's namespace. Everything asserted here is a property whose absence is invisible until it +// matters -- a workflow that publishes from a branch, or under a binary nobody reviewed, looks exactly like +// one that does not, right up until it does. + +const WORKFLOW_PATH = ".github/workflows/publish-mcp-registry.yml"; + +type Step = { name?: string; run?: string; uses?: string; env?: Record }; +type Workflow = { + on?: Record; + permissions?: Record; + jobs?: Record; +}; + +const raw = readFileSync(WORKFLOW_PATH, "utf8"); +const workflow = parse(raw) as Workflow; +const publish = workflow.jobs?.publish; +const steps = publish?.steps ?? []; + +describe("the registry publish workflow cannot fire on its own (#9526)", () => { + it("is dispatch-only — no push, schedule, or pull_request trigger", () => { + // Publishing announces a version to a public registry. A merge must not do that as a side effect. + expect(Object.keys(workflow.on ?? {})).toEqual(["workflow_dispatch"]); + }); + + it("refuses any ref but main, since a dispatch can name a branch", () => { + expect(publish?.if).toContain("refs/heads/main"); + }); + + it("runs in a deployment environment, so the OIDC-holding job can carry protection rules", () => { + // Without one, anyone who can dispatch a workflow can publish. The rules themselves live in repo + // settings; what the workflow owes is the hook they attach to. + expect(publish?.environment).toBeTruthy(); + }); + + it("takes id-token write and nothing else beyond read", () => { + expect(workflow.permissions).toEqual({ contents: "read", "id-token": "write" }); + }); +}); + +describe("the publisher binary is pinned to bytes, not to a tag (#9526)", () => { + const install = steps.find((step) => step.run?.includes("mcp-publisher.tar.gz")); + + it("verifies a sha256 checksum before extracting anything", () => { + // A release tag is mutable -- it can be moved to a different commit after review. The checksum is what + // makes "pinned" true rather than merely stated, and #9526 called for exactly this. + expect(install?.run).toContain("sha256sum --check --strict"); + expect(install?.env?.MCP_PUBLISHER_SHA256).toMatch(/^[0-9a-f]{64}$/); + }); + + it("checks the download BEFORE running it, not after", () => { + const run = install!.run!; + expect(run.indexOf("sha256sum")).toBeLessThan(run.indexOf("tar -xzf")); + expect(run).toContain("set -euo pipefail"); + }); + + it("downloads the asset name the release actually publishes", () => { + // The first cut asked for `mcp-publisher_${VERSION}_linux_amd64.tar.gz`, which 404s -- the release + // asset carries no version in its name. A publish workflow that cannot even fetch its tool is a + // workflow nobody has run. + expect(install?.run).toContain("mcp-publisher_linux_amd64.tar.gz"); + expect(install?.run).not.toMatch(/mcp-publisher_\$\{[^}]+\}_linux/); + expect(install?.env?.MCP_PUBLISHER_VERSION).toMatch(/^v\d+\.\d+\.\d+$/); + }); + + it("pins every action it uses to a commit sha", () => { + for (const step of steps.filter((entry) => entry.uses)) { + expect(step.uses, `${step.uses} must be sha-pinned`).toMatch(/@[0-9a-f]{40}(\s|$)/); + } + }); +}); + +describe("the anti-rot guard runs before anything is announced (#9526)", () => { + it("validates server.json and its watched paths before login or publish", () => { + const validateAt = steps.findIndex((step) => step.run?.includes("check-server-manifest")); + const loginAt = steps.findIndex((step) => step.run?.includes("mcp-publisher login")); + const publishAt = steps.findIndex((step) => step.run?.includes("mcp-publisher publish")); + expect(validateAt).toBeGreaterThanOrEqual(0); + // metagraphed's version-sync workflow watched a renamed-away path and kept passing while doing nothing + // for months. The guard is only worth having if it runs before the irreversible step. + expect(validateAt).toBeLessThan(loginAt); + expect(validateAt).toBeLessThan(publishAt); + }); + + it("offers a dry run that stops short of publishing", () => { + const publishStep = steps.find((step) => step.run?.includes("mcp-publisher publish")); + expect(JSON.stringify(publishStep)).toContain("dry_run"); + }); +}); diff --git a/test/unit/selfhost-pg-queue.test.ts b/test/unit/selfhost-pg-queue.test.ts index aa7a7d8ff..c92d1d704 100644 --- a/test/unit/selfhost-pg-queue.test.ts +++ b/test/unit/selfhost-pg-queue.test.ts @@ -3853,15 +3853,20 @@ describe("createPgQueue (durable #977)", () => { const windowMs = 15 * 60 * 1000; const before = Date.now(); expect(await q.recentDeadCount(windowMs)).toBe(3); + const after = Date.now(); const calls = (m.fn as unknown as ReturnType).mock.calls; const call = calls.find((c: unknown[]) => String(c[0]).includes("status='dead' AND dead_at IS NOT NULL")); expect(call).toBeDefined(); const bound = call![1] as number[]; expect(bound).toEqual([expect.any(Number)]); - // The bound cutoff is "now - windowMs", not a fixed constant -- sanity-check it's in the right ballpark. - expect(bound[0]).toBeLessThanOrEqual(before - windowMs); - expect(bound[0]).toBeGreaterThan(before - windowMs - 5_000); + // The bound cutoff is "now - windowMs", not a fixed constant -- so it must fall inside the window the + // call itself spanned. Bracketing on BOTH sides is what makes that assertion honest: the previous + // version compared only against `before`, and since the implementation reads the clock strictly after + // that line, a single millisecond ticking between the two failed the test. That is not hypothetical -- + // it flaked CI on both attempts (1785302335173 <= 1785302335172). + expect(bound[0]).toBeGreaterThanOrEqual(before - windowMs); + expect(bound[0]).toBeLessThanOrEqual(after - windowMs); }); it("stats() returns persisted queue metric counts", async () => { From 8ff3b1b82a43f6536d70c149b492bae2b57b9f43 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:22:35 -0700 Subject: [PATCH 5/6] fix(mcp): forward the caller's arguments through a proxied tool (#9526) Raising the gateway's patch coverage found a real defect in it. registerProxiedTool declared an inputSchema only for a tool the contract registry knows, and the SDK invokes a handler as `(args, extra)` ONLY when an inputSchema is declared -- as `(extra)` alone when it is not. So a proxied tool absent from the registry, which is precisely the case the fallback exists for (a remote running ahead of this package), forwarded the SDK's own `{ signal, requestId }` to the remote AS THE ARGUMENTS and silently dropped everything the caller passed. An unmodelled tool now gets a fully open input schema instead of none, which keeps the remote the only validator -- where validation belongs for a tool this package does not model -- while making the SDK route arguments the way it does for every other tool. The test that caught it was previously asserting only that the call resolved. callTool resolves for a FAILED call too, so it passed while nothing was being proxied at all: the fixture had no /mcp route, the proxy 404'd, and the SDK turned the throw into an isError result nobody checked. The fixture now answers tools/call with a real JSON-RPC envelope and the assertion is on the payload -- the tool name and the arguments as the remote received them. Also: the advisory resource is registered on every outcome rather than only on failure, so a successful mount no longer leaves an earlier failure's advisory standing as the answer to "why don't I have the remote tools". Plus coverage for the paths that had none -- calling a proxied tool, reading the advisory, a descriptor carrying annotations, one carrying only a name, an envelope with no `result`, the real fetch transport against the loopback fixture, and the no-session path that makes no outbound request at all. --- packages/loopover-mcp/bin/loopover-mcp.ts | 28 +++-- test/unit/mcp-gateway-mount-inprocess.test.ts | 105 ++++++++++++++++++ test/unit/support/mcp-cli-harness.ts | 29 +++++ 3 files changed, 153 insertions(+), 9 deletions(-) diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 0455d2820..f76b753fa 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -2510,7 +2510,15 @@ function registerProxiedTool(tool: RemoteToolDescriptor): void { ...(tool.description ? { description: tool.description } : {}), // The schema OBJECTS, not their `.shape` -- a raw shape is re-wrapped in a plain `z.object` that // silently drops the catchall, turning every extra field the remote allows into a -32602 here. - ...(contract ? { inputSchema: contract.input, outputSchema: contract.output } : {}), + // + // A tool absent from the registry (a remote running ahead of this package) still gets an inputSchema, + // just a fully open one. That is NOT cosmetic: the SDK invokes a handler as `(args, extra)` only when + // an inputSchema is declared, and as `(extra)` alone when it is not -- so omitting it made the proxy + // forward the SDK's own `{ signal, requestId }` to the remote AS THE ARGUMENTS, silently dropping + // everything the caller passed. An open object keeps the remote the only validator, which is where + // validation belongs for a tool this package does not model. + inputSchema: contract?.input ?? z.looseObject({}), + ...(contract ? { outputSchema: contract.output } : {}), ...(tool.annotations ? { annotations: tool.annotations as never } : {}), _meta: { ...(tool._meta ?? {}), transport: "proxied" }, } as never, @@ -2534,11 +2542,15 @@ function registerProxiedTool(tool: RemoteToolDescriptor): void { let gatewayAdvisoryState: { status: string; advisory: string } | null = null; let gatewayAdvisoryRegistered = false; -/** The advisory a client sees instead of remote tools. A resource, never an error -- see gateway.ts. */ +/** + * The resource a client reads to learn why it does or does not have the remote tools. + * + * A resource, never an error -- see gateway.ts. Registered on EVERY outcome, including success: a mount + * that succeeded must not leave an earlier failure's advisory standing, and "why don't I have the remote + * tools" deserves an answer when the answer is "you do". + */ function registerGatewayAdvisory(result: GatewayMountResult): void { - const advisory = gatewayAdvisoryResource(result); - if (!advisory) return; - gatewayAdvisoryState = advisory; + gatewayAdvisoryState = gatewayAdvisoryResource(result) ?? { status: "mounted", advisory: "Remote tools are mounted on this server." }; if (gatewayAdvisoryRegistered) return; gatewayAdvisoryRegistered = true; server.registerResource( @@ -2577,10 +2589,8 @@ export async function mountRemoteTools(options: { argv?: readonly string[]; fetc fetchImpl: options.fetchImpl ?? (defaultGatewayFetch as GatewayFetch), }); - if (result.status !== "mounted") { - registerGatewayAdvisory(result); - return result; - } + registerGatewayAdvisory(result); + if (result.status !== "mounted") return result; const mounted: RemoteToolDescriptor[] = []; const skipped = [...result.skipped]; diff --git a/test/unit/mcp-gateway-mount-inprocess.test.ts b/test/unit/mcp-gateway-mount-inprocess.test.ts index 205754d3e..20a7ce7a8 100644 --- a/test/unit/mcp-gateway-mount-inprocess.test.ts +++ b/test/unit/mcp-gateway-mount-inprocess.test.ts @@ -142,6 +142,111 @@ describe("mountRemoteTools against the real server (#9526)", () => { expect(result.status === "mounted" && result.skipped).toEqual(["loopover_gateway_twice"]); }); + it("CALLS a proxied tool and forwards it to the remote's own tools/call", async () => { + // Listing a proxied tool proves registration; calling one proves the proxy actually routes. The fixture + // server answers /mcp, so this exercises the whole path the way a client would. + await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([{ name: "loopover_gateway_call_probe", description: "callable through the gateway" }]), + }); + + const client = await connect("gateway-call-test"); + try { + const result = (await client.callTool({ name: "loopover_gateway_call_probe", arguments: { owner: "acme" } })) as { + isError?: boolean; + structuredContent?: { calledTool?: string; echoedArguments?: unknown }; + }; + // The remote's `result` is handed back VERBATIM -- this layer routes, it does not interpret -- and the + // arguments reached the remote untouched. Asserting the payload rather than merely "it resolved": + // callTool resolves for a failed call too, so a weaker assertion passes even when nothing was proxied. + expect(result.isError).toBeFalsy(); + expect(result.structuredContent?.calledTool).toBe("loopover_gateway_call_probe"); + expect(result.structuredContent?.echoedArguments).toEqual({ owner: "acme" }); + } finally { + await client.close(); + } + }); + + it("hands back the WHOLE payload when the remote's envelope carries no `result`", async () => { + // The `??` fallback, and a real posture rather than a defensive shrug: a remote that answers a shape + // this package does not model must still reach the caller intact, so the caller can see what came back + // instead of an empty success. + await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([{ name: "loopover_gateway_resultless" }]), + }); + const client = await connect("gateway-resultless"); + try { + const raw = (await client.callTool({ name: "loopover_gateway_resultless", arguments: {} })) as { note?: string; isError?: boolean }; + expect(raw.isError).toBeFalsy(); + expect(raw.note, "the envelope itself reaches the caller when it carries no result").toBe("no result member"); + } finally { + await client.close(); + } + }); + + it("mounts a tool carrying annotations, and one carrying neither description nor annotations", async () => { + const result = await mod.mountRemoteTools({ + argv: ["--stdio"], + fetchImpl: remoteToolsFetch([ + { name: "loopover_gateway_annotated", description: "has hints", annotations: { readOnlyHint: true, destructiveHint: false } }, + // No description, no annotations: the remote is free to send a minimal descriptor and it must mount. + { name: "loopover_gateway_bare" }, + ]), + }); + expect(result.status === "mounted" && result.tools).toHaveLength(2); + + const client = await connect("gateway-annotations-test"); + try { + const tools = (await client.listTools()).tools; + expect(tools.find((tool) => tool.name === "loopover_gateway_annotated")?.annotations?.readOnlyHint).toBe(true); + const bare = tools.find((tool) => tool.name === "loopover_gateway_bare"); + expect(bare, "a descriptor with only a name must still mount").toBeDefined(); + expect(bare!.description).toBeUndefined(); + } finally { + await client.close(); + } + }); + + it("publishes an advisory resource a client can READ, and updates it on a later mount", async () => { + await mod.mountRemoteTools({ argv: ["--stdio", "--no-remote"] }); + const client = await connect("gateway-advisory-read"); + try { + const disabled = await client.readResource({ uri: "loopover://gateway/status" }); + expect(String((disabled.contents[0] as { text?: string }).text)).toContain("--no-remote"); + + // A successful mount must not leave the earlier failure's advisory standing. + await mod.mountRemoteTools({ argv: ["--stdio"], fetchImpl: remoteToolsFetch([]) }); + const after = await client.readResource({ uri: "loopover://gateway/status" }); + expect(JSON.parse(String((after.contents[0] as { text?: string }).text)).status).toBe("mounted"); + } finally { + await client.close(); + } + }); + + it("uses a REAL fetch when the caller injects none", async () => { + // The default transport, exercised against the loopback fixture server rather than stubbed away -- so + // the one line that actually talks to the network is covered by talking to a network. + const result = await mod.mountRemoteTools({ argv: ["--stdio"] }); + // The fixture answers tools/call, not tools/list, so discovery finds no tool array and degrades -- + // which is the point: the real transport ran, and a shape it did not expect did not break the server. + expect(["mounted", "unavailable"]).toContain(result.status); + }); + + it("defaults argv to the process's own, and the transport to a real fetch, without touching the network", async () => { + // Both defaults on one call. No session means discoverRemoteTools returns before it ever calls the + // transport, so the real fetch default is selected and never invoked -- which is the point: an + // unauthenticated contributor makes no outbound request at all. + const token = process.env.LOOPOVER_API_TOKEN; + delete process.env.LOOPOVER_API_TOKEN; + try { + const result = await mod.mountRemoteTools(); + expect(result.status).toBe("unauthenticated"); + } finally { + process.env.LOOPOVER_API_TOKEN = token; + } + }); + it("an unreachable API leaves a server that still lists its LOCAL tools", async () => { const fetchImpl = vi.fn(async () => { throw new Error("ECONNREFUSED"); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index bd9a41722..d91f6cd0d 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -257,6 +257,35 @@ export async function startFixtureServer( response.end(JSON.stringify({ version: options.latestVersion ?? "0.4.0" })); return; } + // #9526: the remote MCP endpoint the stdio gateway proxies tools/call to. Answers the JSON-RPC envelope + // a real server does, so a proxied call can be asserted end to end rather than only up to the request. + if (new URL(request.url ?? "/", "http://localhost").pathname === "/mcp") { + let raw = ""; + request.on("data", (chunk) => { + raw += chunk; + }); + request.on("end", () => { + const parsed = JSON.parse(raw || "{}") as { id?: unknown; params?: { name?: string; arguments?: unknown } }; + // A tool named `*_resultless` gets an envelope with NO `result` member, so the gateway's + // "hand back the whole payload" fallback can be exercised against a real response rather than a + // hand-built object. Keyed on the name because one fixture serves every gateway test. + if (parsed.params?.name?.endsWith("_resultless")) { + response.end(JSON.stringify({ jsonrpc: "2.0", id: parsed.id ?? 1, note: "no result member" })); + return; + } + response.end( + JSON.stringify({ + jsonrpc: "2.0", + id: parsed.id ?? 1, + result: { + content: [{ type: "text", text: `remote answered ${parsed.params?.name ?? "unknown"}` }], + structuredContent: { calledTool: parsed.params?.name, echoedArguments: parsed.params?.arguments ?? null }, + }, + }), + ); + }); + return; + } if (request.url === "/v1/mcp/compatibility") { if (options.compatibilityStatus && options.compatibilityStatus >= 400) { response.statusCode = options.compatibilityStatus; From 12e2b8e7e44ab137193cc3b075d55b8504d07acc Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:46:29 -0700 Subject: [PATCH 6/6] test(mcp): cover init-client's host and mode guards (#9526) The five branch partials codecov/patch was holding the PR on, all in init-client's argument handling: no host at all, an unknown host, the --client spelling as well as --print, a host/mode pair the grid refuses, and the per-host remote caveat's absent side. Each asserts the message rather than merely the throw -- "unsupported client" without the list leaves a reader guessing at a five-value set they cannot see from outside the process. --- apps/loopover-ui/public/openapi.json | 451 +++++++++++++++++++++++++++ src/openapi/spec.ts | 16 +- test/unit/mcp-cli-basics.test.ts | 27 ++ 3 files changed, 484 insertions(+), 10 deletions(-) diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 281e2017d..bb41b1537 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -26440,6 +26440,457 @@ } } } + }, + "/.well-known/mcp.json": { + "get": { + "operationId": "getMcpServerCard", + "tags": [ + "Discovery" + ], + "summary": "MCP server card for this deployment", + "description": "Computed at request time from the tool contract registry, filtered to what this deployment serves. Carries a weak ETag; send `if-none-match` to get a 304 instead of the body.", + "security": [], + "responses": { + "200": { + "description": "MCP server card for this deployment", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "version": { + "type": "string" + }, + "description": { + "type": "string" + }, + "deployment": { + "type": "string", + "enum": [ + "cloud", + "selfhost" + ] + }, + "generated_at": { + "type": "string" + }, + "capabilities": { + "type": "object", + "properties": { + "tools": { + "type": "object", + "properties": { + "listChanged": { + "type": "boolean" + } + }, + "required": [ + "listChanged" + ] + } + }, + "required": [ + "tools" + ] + }, + "remotes": { + "type": "array", + "items": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "streamable-http" + ] + }, + "url": { + "type": "string" + } + }, + "required": [ + "type", + "url" + ] + } + }, + "tools": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "title": { + "type": "string" + }, + "description": { + "type": "string" + }, + "category": { + "type": "string" + }, + "annotations": { + "type": "object", + "properties": { + "readOnlyHint": { + "type": "boolean" + }, + "destructiveHint": { + "type": "boolean" + } + }, + "required": [ + "readOnlyHint", + "destructiveHint" + ] + } + }, + "required": [ + "name", + "title", + "description", + "category", + "annotations" + ] + } + } + }, + "required": [ + "name", + "version", + "description", + "deployment", + "generated_at", + "capabilities", + "remotes", + "tools" + ] + } + } + } + }, + "304": { + "description": "The caller's `if-none-match` already names this document" + } + } + } + }, + "/.well-known/agent-tools/index.json": { + "get": { + "operationId": "getAgentToolsIndex", + "tags": [ + "Discovery" + ], + "summary": "Neutral agent-tools catalog with both schemas per tool", + "description": "Computed at request time from the tool contract registry, filtered to what this deployment serves. Carries a weak ETag; send `if-none-match` to get a 304 instead of the body.", + "security": [], + "responses": { + "200": { + "description": "Neutral agent-tools catalog with both schemas per tool", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "generated_at": { + "type": "string" + }, + "executor": { + "type": "object", + "properties": { + "transport": { + "type": "string", + "enum": [ + "streamable-http" + ] + }, + "url": { + "type": "string" + }, + "method": { + "type": "string", + "enum": [ + "tools/call" + ] + } + }, + "required": [ + "transport", + "url", + "method" + ] + }, + "tools": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "title": { + "type": "string" + }, + "description": { + "type": "string" + }, + "category": { + "type": "string" + }, + "annotations": { + "type": "object", + "properties": { + "readOnlyHint": { + "type": "boolean" + }, + "destructiveHint": { + "type": "boolean" + } + }, + "required": [ + "readOnlyHint", + "destructiveHint" + ] + }, + "input_schema": { + "type": "object", + "properties": {}, + "additionalProperties": { + "nullable": true + }, + "description": "JSON Schema (draft 2020-12) for a tool's arguments or result." + }, + "output_schema": { + "type": "object", + "properties": {}, + "additionalProperties": { + "nullable": true + }, + "description": "JSON Schema (draft 2020-12) for a tool's arguments or result." + } + }, + "required": [ + "name", + "title", + "description", + "category", + "annotations", + "input_schema", + "output_schema" + ] + } + } + }, + "required": [ + "generated_at", + "executor", + "tools" + ] + } + } + } + }, + "304": { + "description": "The caller's `if-none-match` already names this document" + } + } + } + }, + "/.well-known/agent-tools/openai.json": { + "get": { + "operationId": "getAgentToolsOpenAi", + "tags": [ + "Discovery" + ], + "summary": "Agent-tools catalog in OpenAI's function-tool shape", + "description": "Computed at request time from the tool contract registry, filtered to what this deployment serves. Carries a weak ETag; send `if-none-match` to get a 304 instead of the body.", + "security": [], + "responses": { + "200": { + "description": "Agent-tools catalog in OpenAI's function-tool shape", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "generated_at": { + "type": "string" + }, + "executor": { + "type": "object", + "properties": { + "transport": { + "type": "string", + "enum": [ + "streamable-http" + ] + }, + "url": { + "type": "string" + }, + "method": { + "type": "string", + "enum": [ + "tools/call" + ] + } + }, + "required": [ + "transport", + "url", + "method" + ] + }, + "tools": { + "type": "array", + "items": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": [ + "function" + ] + }, + "function": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "description": { + "type": "string" + }, + "parameters": { + "type": "object", + "properties": {}, + "additionalProperties": { + "nullable": true + }, + "description": "JSON Schema (draft 2020-12) for a tool's arguments or result." + } + }, + "required": [ + "name", + "description", + "parameters" + ] + } + }, + "required": [ + "type", + "function" + ] + } + } + }, + "required": [ + "generated_at", + "executor", + "tools" + ] + } + } + } + }, + "304": { + "description": "The caller's `if-none-match` already names this document" + } + } + } + }, + "/.well-known/agent-tools/anthropic.json": { + "get": { + "operationId": "getAgentToolsAnthropic", + "tags": [ + "Discovery" + ], + "summary": "Agent-tools catalog in Anthropic's tool shape", + "description": "Computed at request time from the tool contract registry, filtered to what this deployment serves. Carries a weak ETag; send `if-none-match` to get a 304 instead of the body.", + "security": [], + "responses": { + "200": { + "description": "Agent-tools catalog in Anthropic's tool shape", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "generated_at": { + "type": "string" + }, + "executor": { + "type": "object", + "properties": { + "transport": { + "type": "string", + "enum": [ + "streamable-http" + ] + }, + "url": { + "type": "string" + }, + "method": { + "type": "string", + "enum": [ + "tools/call" + ] + } + }, + "required": [ + "transport", + "url", + "method" + ] + }, + "tools": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "description": { + "type": "string" + }, + "input_schema": { + "type": "object", + "properties": {}, + "additionalProperties": { + "nullable": true + }, + "description": "JSON Schema (draft 2020-12) for a tool's arguments or result." + } + }, + "required": [ + "name", + "description", + "input_schema" + ] + } + } + }, + "required": [ + "generated_at", + "executor", + "tools" + ] + } + } + } + }, + "304": { + "description": "The caller's `if-none-match` already names this document" + } + } + } } }, "servers": [ diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 90f438f21..2641c0d11 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -175,7 +175,12 @@ function loopOperationMeta(method: string, path: string, tag: string): { operati * that had to restate this list would go quiet the moment a new registrar was added, which is the rot the * anti-rot guards in this repo keep finding. */ -export const SPEC_REGISTRARS: ReadonlyArray<(registry: OpenAPIRegistry) => void> = [registerOrbAndControlRouteSpecs, registerInternalAndPublicRouteSpecs]; +export const SPEC_REGISTRARS: ReadonlyArray<(registry: OpenAPIRegistry) => void> = [ + registerOrbAndControlRouteSpecs, + // #9526: served by this app and computed at request time from the contract registry. + registerDiscoveryRouteSpecs, + registerInternalAndPublicRouteSpecs, +]; export function buildOpenApiSpec() { const registry = new OpenAPIRegistry(); @@ -2249,16 +2254,7 @@ export function buildOpenApiSpec() { // Registered from their own module rather than inline here because each entry declares an auth // level that DERIVES its security stanza, instead of having one bolted on afterwards by // applySecurityMetadata's path-prefix guesswork. -<<<<<<< HEAD for (const register of SPEC_REGISTRARS) register(registry); -||||||| parent of 42b72c8f5 (feat(mcp): derive every client-config surface and tag proxied calls (#9526)) - registerOrbAndControlRouteSpecs(registry); - registerInternalAndPublicRouteSpecs(registry); -======= - registerOrbAndControlRouteSpecs(registry); - registerDiscoveryRouteSpecs(registry); - registerInternalAndPublicRouteSpecs(registry); ->>>>>>> 42b72c8f5 (feat(mcp): derive every client-config surface and tag proxied calls (#9526)) const generator = new OpenApiGeneratorV3(registry.definitions); const document = generator.generateDocument({ diff --git a/test/unit/mcp-cli-basics.test.ts b/test/unit/mcp-cli-basics.test.ts index 0ccec1706..ce5616b82 100644 --- a/test/unit/mcp-cli-basics.test.ts +++ b/test/unit/mcp-cli-basics.test.ts @@ -125,6 +125,33 @@ describe("loopover-mcp CLI — basics", () => { await expect(runInProcess(["init-client", "--print", "claude", "--mode", "carrier-pigeon"])).rejects.toThrow(/Unsupported mode.*stdio, remote, miner/); }); + it("names the hosts it accepts when told nothing, or something it does not know (#9526)", async () => { + // The error has to enumerate: "unsupported client" without the list leaves a reader guessing at a + // five-value set they cannot see from the outside. + await expect(runInProcess(["init-client"])).rejects.toThrow(/Pass --print with one of: codex, claude, cursor, mcp, vscode/); + await expect(runInProcess(["init-client", "--print", "emacs"])).rejects.toThrow(/Unsupported client: emacs.*codex, claude, cursor, mcp, vscode/); + }); + + it("accepts --client as well as --print, since both spellings reached this command (#9526)", async () => { + const viaClient = JSON.parse(await runInProcess(["init-client", "--client", "cursor", "--json"])) as { client: string; snippet: string }; + expect(viaClient.client).toBe("cursor"); + expect(viaClient.snippet).toBe(clientConfigSnippet("cursor", "stdio")); + }); + + it("refuses a host/mode pair it cannot vouch for, naming both (#9526)", async () => { + // The generic `mcpServers` bucket is an unnamed host; guessing its remote dialect would print config + // that fails on paste, and the stdio gateway already serves it the remote tools. + await expect(runInProcess(["init-client", "--print", "mcp", "--mode", "remote"])).rejects.toThrow(/cannot connect over the Remote streamable-http mode/); + }); + + it("carries the host's own remote caveat when it has one, and nothing when it does not (#9526)", async () => { + const codex = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--mode", "remote", "--json"])) as { notes: string[] }; + expect(codex.notes.join("\n")).toContain("experimental_use_rmcp_client"); + + const cursor = JSON.parse(await runInProcess(["init-client", "--print", "cursor", "--mode", "remote", "--json"])) as { notes: string[] }; + expect(cursor.notes.join("\n")).not.toContain("experimental_use_rmcp_client"); + }); + it("prints human-approved agent profile instructions for supported MCP clients", async () => { const payload = JSON.parse(await runInProcess(["init-client", "--print", "codex", "--agent-profile", "miner-planner", "--json"])) as { agentProfile: {