From a158e49ec24bef57b9a7f2b3888b47038445b579 Mon Sep 17 00:00:00 2001 From: Michal Date: Sun, 9 Aug 2026 23:53:22 +0100 Subject: [PATCH] fix(templates): make the shipped templates match reality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The templates are what `create server --from-template` builds from and what mcpd seeds on start, so drift there ships broken servers. Nothing ever read these files in a test, and they had rotted badly. - grafana: GRAFANA_URL now defaults to the in-cluster ClusterIP and the description spells out why the public hostname is wrong — reaching a co-located Grafana over its ingress hairpins through the per-host Envoy L7 policy, which drops the caller's identity and returns a bare `Access denied` 403 with a perfectly valid token. That cost a day of looking at the token. - unifi-network: was wrong on every field that mattered. `runtime: python` for an npm package, an env contract (UNIFI_HOST/USERNAME/PASSWORD) the package doesn't read, and no probe. Now UNIFI_TARGETS with the classic-vs-unifi_os distinction and the :8443 egress caveat written down. - docmost, gitea: both carried "health check disabled" comments citing a limitation of the old docker-exec probe, which readiness-via-proxy removed. Both probes verified against the live servers. gitea uses search_repos, not get_me, because get_me needs a `read:user` scope a repo-scoped token lacks. - filesystem: packageName was `@anthropic/filesystem-mcp`, which 404s on npm — the template could never have installed. Points at the real package. - terraform: deleted. `@anthropic/terraform-mcp` 404s too and there is no npm-published replacement to point it at. - node-red: deleted, the service is gone. Two supporting fixes: - The seeder declared no `runtime` field and never wrote the column, so a template asking for the python runner silently seeded as null and got node. - A new templates test reads every shipped file: schema-valid, a runner the orchestrator knows, some way to actually start, unique env names, and a readiness probe (without one an instance can only ever report `live`). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb --- README.md | 2 +- src/db/src/seed/index.ts | 9 ++++ src/mcpd/tests/templates.test.ts | 79 ++++++++++++++++++++++++++++++++ templates/docmost.yaml | 11 ++++- templates/filesystem.yaml | 21 ++++++++- templates/gitea.yaml | 13 +++++- templates/grafana.yaml | 16 ++++++- templates/node-red.yaml | 16 ------- templates/terraform.yaml | 6 --- templates/unifi-network.yaml | 53 +++++++++++++-------- 10 files changed, 178 insertions(+), 48 deletions(-) create mode 100644 src/mcpd/tests/templates.test.ts delete mode 100644 templates/node-red.yaml delete mode 100644 templates/terraform.yaml diff --git a/README.md b/README.md index 16ead01..71600b6 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,7 @@ name: home-automation proxyModel: default servers: - home-assistant - - node-red + - unifi-network ``` Via CLI: diff --git a/src/db/src/seed/index.ts b/src/db/src/seed/index.ts index e8872e3..6c6dfb5 100644 --- a/src/db/src/seed/index.ts +++ b/src/db/src/seed/index.ts @@ -21,6 +21,13 @@ export interface SeedTemplate { version: string; description: string; packageName?: string; + /** + * Package runner: 'node' (npx) or 'python' (uvx). McpTemplate has had this + * column all along, but the upsert below never wrote it, so a template + * declaring `runtime: python` seeded as null and every server created from + * it silently got the node runner. + */ + runtime?: string; dockerImage?: string; transport: 'STDIO' | 'SSE' | 'STREAMABLE_HTTP'; repositoryUrl?: string; @@ -45,6 +52,7 @@ export async function seedTemplates( version: tpl.version, description: tpl.description, packageName: tpl.packageName ?? null, + runtime: tpl.runtime ?? null, dockerImage: tpl.dockerImage ?? null, transport: tpl.transport, repositoryUrl: tpl.repositoryUrl ?? null, @@ -60,6 +68,7 @@ export async function seedTemplates( version: tpl.version, description: tpl.description, packageName: tpl.packageName ?? null, + runtime: tpl.runtime ?? null, dockerImage: tpl.dockerImage ?? null, transport: tpl.transport, repositoryUrl: tpl.repositoryUrl ?? null, diff --git a/src/mcpd/tests/templates.test.ts b/src/mcpd/tests/templates.test.ts new file mode 100644 index 0000000..a597a47 --- /dev/null +++ b/src/mcpd/tests/templates.test.ts @@ -0,0 +1,79 @@ +/** + * The shipped `templates/*.yaml` are seeded into mcpd and are what `mcpctl + * create server --from-template` builds from, so drift there ships broken + * servers. The unifi-network template had drifted on every field that + * mattered — python runtime for an npm package, an env contract + * (UNIFI_HOST/USERNAME/PASSWORD) the package doesn't read, and a comment + * disabling its health check for a reason that had stopped being true — and + * nothing caught it because no test ever read the files. + */ +import { describe, it, expect } from 'vitest'; +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import yaml from 'js-yaml'; +import { CreateTemplateSchema } from '../src/validation/template.schema.js'; + +const TEMPLATES_DIR = fileURLToPath(new URL('../../../templates', import.meta.url)); + +const files = readdirSync(TEMPLATES_DIR).filter((f) => f.endsWith('.yaml') || f.endsWith('.yml')); + +interface RawTemplate { + name?: string; + runtime?: string; + packageName?: string; + dockerImage?: string; + externalUrl?: string; + healthCheck?: { tool?: string }; + env?: Array<{ name?: string }>; +} + +function load(file: string): RawTemplate { + return yaml.load(readFileSync(join(TEMPLATES_DIR, file), 'utf-8')) as RawTemplate; +} + +describe('shipped templates', () => { + it('ships at least one template', () => { + expect(files.length).toBeGreaterThan(0); + }); + + it.each(files)('%s validates against CreateTemplateSchema', (file) => { + const parsed = CreateTemplateSchema.safeParse(load(file)); + expect(parsed.success ? null : parsed.error.issues).toBeNull(); + }); + + it.each(files)('%s declares a runner the orchestrator knows', (file) => { + const tpl = load(file); + // `runtime` only means anything for package-based servers, and only + // 'node' (npx) and 'python' (uvx) are wired in buildRuntimeSpawnCmd. + if (tpl.runtime !== undefined) { + expect(['node', 'python']).toContain(tpl.runtime); + } + }); + + it.each(files)('%s says how to actually run the server', (file) => { + const tpl = load(file); + const runnable = tpl.packageName !== undefined + || tpl.dockerImage !== undefined + || tpl.externalUrl !== undefined; + expect(runnable, `${file} has no packageName, dockerImage, or externalUrl`).toBe(true); + }); + + it.each(files)('%s names a readiness probe tool, not a bare liveness probe', (file) => { + const tpl = load(file); + // Without a `tool`, an instance from this template can only ever report + // `live` — nothing would ever check its upstream. See docs/reliability.md. + expect(tpl.healthCheck?.tool, `${file} has no healthCheck.tool`).toBeTruthy(); + }); + + it.each(files)('%s declares uniquely-named env entries', (file) => { + const names = (load(file).env ?? []).map((e) => e.name); + expect(new Set(names).size).toBe(names.length); + }); + + it('has no template for a retired server', () => { + // node-red was retired 2026-08-09: it answered on neither its Tailscale + // nor its LAN address and had no deployment anywhere. + expect(files).not.toContain('node-red.yaml'); + }); +}); diff --git a/templates/docmost.yaml b/templates/docmost.yaml index 10d9df1..e608d1c 100644 --- a/templates/docmost.yaml +++ b/templates/docmost.yaml @@ -4,8 +4,15 @@ description: Docmost MCP server for wiki/documentation page management and searc dockerImage: "mysources.co.uk/michal/docmost-mcp:latest" transport: STDIO repositoryUrl: https://github.com/MrMartiniMo/docmost-mcp -# Health check disabled: STDIO health probe requires packageName (npm-based servers). -# This server uses a custom dockerImage. Probe support for dockerImage STDIO servers is TODO. +healthCheck: + # get_workspace calls the Docmost API, so a pass proves URL + login. The old + # "probe requires packageName" caveat here was true of the long-gone + # docker-exec probe; readiness now goes through the MCP proxy, which works + # the same for image-based STDIO servers. Verified against the live server. + tool: get_workspace + arguments: {} + intervalSeconds: 60 + timeoutSeconds: 10 env: - name: DOCMOST_API_URL description: Docmost API URL (e.g. http://100.88.157.6:3000/api) diff --git a/templates/filesystem.yaml b/templates/filesystem.yaml index 4c14e7b..cb0c1c9 100644 --- a/templates/filesystem.yaml +++ b/templates/filesystem.yaml @@ -1,6 +1,23 @@ name: filesystem -version: "1.0.0" +version: "2.0.0" description: Filesystem MCP server for reading and writing files -packageName: "@anthropic/filesystem-mcp" +# Was "@anthropic/filesystem-mcp", which 404s on the npm registry — creating a +# server from this template failed at install. This is the real package. +packageName: "@modelcontextprotocol/server-filesystem" +runtime: node transport: STDIO repositoryUrl: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem +healthCheck: + # Zero-arg and proves the server resolved its configured roots, which is the + # only thing it can be misconfigured about. + tool: list_allowed_directories + arguments: {} + intervalSeconds: 60 + timeoutSeconds: 10 +env: + - name: ALLOWED_DIRECTORIES + description: >- + Space-separated directories the server may access. Passed as the + package's positional arguments; without at least one the server exposes + nothing. + required: true diff --git a/templates/gitea.yaml b/templates/gitea.yaml index 9ded49c..c41dbd9 100644 --- a/templates/gitea.yaml +++ b/templates/gitea.yaml @@ -7,7 +7,18 @@ repositoryUrl: https://gitea.com/gitea/gitea-mcp # No command: the image's entrypoint IS the MCP server. mcpd attaches to PID 1 # stdin/stdout (attach mode) rather than exec-ing a new process. The image is # distroless and has no node/shell, so exec-based STDIO would fail. -# Health check disabled: STDIO health probe requires node in the container. +healthCheck: + # search_repos is a real Gitea API call, deliberately chosen over get_me: + # get_me needs the `read:user` token scope, which a repo-scoped token won't + # have, so it would fail for a bookkeeping reason rather than a real one. + # (The "probe requires node in the container" caveat that used to sit here + # described the old docker-exec probe; readiness goes through the MCP proxy + # now, so a distroless image is fine.) Verified against the live server. + tool: search_repos + arguments: + query: mcpctl + intervalSeconds: 60 + timeoutSeconds: 10 env: - name: GITEA_HOST description: Gitea instance URL (e.g. https://gitea.example.com) diff --git a/templates/grafana.yaml b/templates/grafana.yaml index 2bf6992..f8445b7 100644 --- a/templates/grafana.yaml +++ b/templates/grafana.yaml @@ -1,16 +1,28 @@ name: grafana -version: "1.0.0" +version: "1.1.0" description: Grafana MCP server for dashboards, datasources, and alerts packageName: "@leval/mcp-grafana" +runtime: node transport: STDIO repositoryUrl: https://github.com/levalhq/mcp-grafana healthCheck: + # Hits the Grafana API, so a pass proves URL + token + reachability. A + # liveness probe (tools/list) cannot: it answers from the server's own tool + # table and stays green while every Grafana call 403s. tool: list_datasources arguments: {} + intervalSeconds: 60 + timeoutSeconds: 10 env: - name: GRAFANA_URL - description: Grafana instance URL (e.g. https://grafana.example.com) + description: >- + Grafana base URL. For a Grafana in this cluster use its ClusterIP + (http://grafana..svc.cluster.local:3000) — NOT its public + hostname. Reaching it over the public ingress hairpins the request back + through the per-host Envoy L7 policy, which drops the caller's identity + and answers a bare `Access denied` 403 even when the token is valid. required: true + defaultValue: http://grafana.home-automation.svc.cluster.local:3000 - name: GRAFANA_SERVICE_ACCOUNT_TOKEN description: Grafana service account token (glsa_...) required: true diff --git a/templates/node-red.yaml b/templates/node-red.yaml deleted file mode 100644 index d81b749..0000000 --- a/templates/node-red.yaml +++ /dev/null @@ -1,16 +0,0 @@ -name: node-red -version: "1.0.0" -description: Node-RED MCP server for flow management and automation -packageName: "mcp-node-red" -transport: STDIO -repositoryUrl: https://github.com/fx/mcp-node-red -healthCheck: - tool: get_settings - arguments: {} -env: - - name: NODE_RED_URL - description: Node-RED instance URL (e.g. http://nodered.local:1880) - required: true - - name: NODE_RED_TOKEN - description: Node-RED access token (optional if no auth) - required: false diff --git a/templates/terraform.yaml b/templates/terraform.yaml deleted file mode 100644 index 9fd4049..0000000 --- a/templates/terraform.yaml +++ /dev/null @@ -1,6 +0,0 @@ -name: terraform -version: "1.0.0" -description: Terraform MCP server for infrastructure documentation and state -packageName: "@anthropic/terraform-mcp" -transport: STDIO -repositoryUrl: https://github.com/modelcontextprotocol/servers/tree/main/src/terraform diff --git a/templates/unifi-network.yaml b/templates/unifi-network.yaml index 8d5c14a..242309d 100644 --- a/templates/unifi-network.yaml +++ b/templates/unifi-network.yaml @@ -1,25 +1,42 @@ name: unifi-network -version: "1.0.0" +version: "2.0.0" description: UniFi Network MCP server for managing UniFi network devices, clients, and configuration packageName: "unifi-network-mcp" -runtime: python +runtime: node transport: STDIO repositoryUrl: https://github.com/sirkirby/unifi-mcp -# Health check disabled: STDIO health probe requires packageName (npm-based servers). -# This server uses the Python runner. Probe support for Python runner STDIO servers is TODO. +healthCheck: + # list_sites calls the controller (/api/self/sites), so a pass proves the + # whole path: egress to the controller port, TLS, login, session. The old + # template disabled the probe entirely on the belief that STDIO probes only + # worked for npm packages — that stopped being true once readiness probes + # started going through the MCP proxy, and the gap let this server sit at + # "healthy" for months without ever reaching the controller. + tool: list_sites + arguments: {} + intervalSeconds: 60 + timeoutSeconds: 15 env: - - name: UNIFI_HOST - description: UniFi controller hostname or IP (e.g. unifi.example.com — without https://) + - name: UNIFI_TARGETS + description: >- + JSON array of controllers. One object per controller: + {"id", "base_url", "controller_type", "default_site", "auth": + {"username","password"}, "verify_ssl"}. + + controller_type is "classic" for a self-hosted UniFi Network controller + (login /api/login, no path prefix) or "unifi_os" for a UDM/UniFi OS + console (login /api/auth/login, API under /proxy/network). Choosing the + wrong one sends every request to a path that 404s while the server still + starts cleanly. + + base_url must carry the real controller port — a self-hosted controller + is usually :8443, and :443 on the same host is often an unrelated + service. Note that MCP server pods only egress 80/443 by default, so any + other port needs an explicit NetworkPolicy (Pulumi + `mcpctl.serverEgressTargets`). required: true - - name: UNIFI_USERNAME - description: UniFi local admin username - required: true - - name: UNIFI_PASSWORD - description: UniFi admin password - required: true - - name: UNIFI_NETWORK_PORT - description: UniFi controller port (default 443, use 8443 for standalone UniFi Controller) - required: false - - name: UNIFI_NETWORK_VERIFY_SSL - description: Verify SSL certificate (true/false, default true — set false for self-signed certs) - required: false + defaultValue: >- + [{"id": "home", "base_url": "https://unifi.example.com:8443", + "controller_type": "classic", "default_site": "default", + "auth": {"username": "CHANGE_ME", "password": "CHANGE_ME"}, + "verify_ssl": false}]