fix(health): a passing tools/list is live, not healthy #104

Merged

michal merged 3 commits from worktree-fix-healthcheck-unifi into main 2026-08-09 23:01:02 +00:00

Conversation 0 Commits 3 Files Changed 21 +685 -57

michal commented 2026-08-09 20:24:07 +00:00

Owner

Copy Link

The bug

mcpctl get instance showed all eight servers healthy while the UniFi one had never once reached its controller.

The default probe is tools/list. MCP servers answer that from a static in-process table — no credentials, no upstream, ~3ms. It cannot fail for any reason a user cares about, so it reported healthy for every process that managed to start.

The fix

Status	Probe	Means
healthy	readiness — tools/call on healthCheck.tool	upstream answered
live	liveness — tools/list only	process up, upstream unverified

live is the default for a server with no healthCheck.tool. It isn't a warning — it's an admission that nothing is watching that server's upstream. Probe events now name which probe ran and which tool.

Also:

• healthCheck.tool optional, so timings are tunable without inventing a readiness probe
• create server --health-check-tool/-args/-interval/-timeout/-failure-threshold (everything applyable is a create flag); merges over a --from-template healthCheck
• describe instance explains a live verdict
• create.ts held a raw NUL byte in a string literal — grep treated the whole file as binary and silently skipped it. Escaped as \\0.

Verified on the live fleet

Readiness probes configured on all eight servers; two that had read healthy for months turned red immediately:

my-grafana    unhealthy   Grafana API error (403): Forbidden
my-node-red   degraded    Connect Timeout (100.117.35.98:1880 — a Tailscale address)

UniFi's probe latency went 3ms (tools/list, lying) → 1847ms (first real list_sites: login, TLS, controller round trip).

Companion infra change: kubernetes-deployment fix/mcpctl-server-egress-targets (pod egress was capped at 80/443; the UniFi controller is on :8443).

2464 unit tests pass; typecheck clean; no new lint findings (874 pre-existing, unchanged).

🤖 Generated with Claude Code

https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb

## The bug `mcpctl get instance` showed **all eight servers healthy** while the UniFi one had never once reached its controller. The default probe is `tools/list`. MCP servers answer that from a static in-process table — no credentials, no upstream, ~3ms. It cannot fail for any reason a user cares about, so it reported `healthy` for every process that managed to start. ## The fix | Status | Probe | Means | |---|---|---| | `healthy` | readiness — `tools/call` on `healthCheck.tool` | upstream answered | | `live` | liveness — `tools/list` only | process up, upstream **unverified** | `live` is the default for a server with no `healthCheck.tool`. It isn't a warning — it's an admission that nothing is watching that server's upstream. Probe events now name which probe ran and which tool. Also: - `healthCheck.tool` optional, so timings are tunable without inventing a readiness probe - `create server --health-check-tool/-args/-interval/-timeout/-failure-threshold` (everything applyable is a create flag); merges over a `--from-template` healthCheck - `describe instance` explains a `live` verdict - `create.ts` held a raw NUL byte in a string literal — grep treated the whole file as binary and silently skipped it. Escaped as `\\0`. ## Verified on the live fleet Readiness probes configured on all eight servers; two that had read healthy for months turned red immediately: ``` my-grafana unhealthy Grafana API error (403): Forbidden my-node-red degraded Connect Timeout (100.117.35.98:1880 — a Tailscale address) ``` UniFi's probe latency went 3ms (`tools/list`, lying) → 1847ms (first real `list_sites`: login, TLS, controller round trip). Companion infra change: kubernetes-deployment `fix/mcpctl-server-egress-targets` (pod egress was capped at 80/443; the UniFi controller is on :8443). 2464 unit tests pass; typecheck clean; no new lint findings (874 pre-existing, unchanged). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb

michal added 1 commit 2026-08-09 20:24:07 +00:00

fix(health): a passing tools/list is live, not healthy ... b2547429ca

`mcpctl get instances` showed all eight servers healthy while the UniFi one
had never once reached its controller. The default probe is `tools/list`,
which MCP servers answer from a static in-process table — no credentials, no
upstream, ~3ms. It cannot fail for any reason the user cares about, so it was
reporting `healthy` for every process that managed to start.

Split the two passes:

  healthy  — readiness: `tools/call` on `healthCheck.tool`. The upstream
             answered, so the server can actually do its job.
  live     — liveness: `tools/list` only. Process up, upstream unverified.

`live` is now the default for any server without a `healthCheck.tool`. It is
not a warning; it is an admission that nothing is watching that server. Probe
events name which probe ran and which tool ("Readiness check (list_sites)
passed"), so the events log distinguishes the two after the fact.

Also:
- `healthCheck.tool` is optional now, so the timings can be tuned without
  inventing a readiness probe.
- `create server --health-check-tool/-args/-interval/-timeout/
  -failure-threshold`, per the rule that everything applyable is a create
  flag. Merges over a `--from-template` healthCheck rather than replacing it.
- `describe instance` explains a `live` verdict instead of leaving it cryptic.
- create.ts held a raw NUL byte in a string literal, which made grep treat the
  whole file as binary and silently skip it. Escaped as `\0`.

Verified against the live fleet: with readiness probes configured, my-grafana
went unhealthy (Grafana API 403) and my-node-red degraded (connect timeout to
a Tailscale address) — both had read healthy for months.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb

michal added 1 commit 2026-08-09 21:54:35 +00:00

docs(reliability): record the three shapes a failing readiness probe takes ... 732ca98ccc

Turning readiness probes on took the fleet from 8/8 healthy to three real
failures in under a minute, and all three were network shape rather than code:
an egress port (UniFi :8443), an ingress hairpin through the Envoy L7 policy
(Grafana 403 `Access denied` with a token that worked from a laptop), and a
Tailscale address a pod can never reach (Node-RED, since retired).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb

michal added 1 commit 2026-08-09 22:53:25 +00:00

fix(templates): make the shipped templates match reality ... a158e49ec2

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0114dg56YmVacyqhp5fitcTb

michal merged commit e4e2e063f1 into main 2026-08-09 23:01:02 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

michal

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: michal/mcpctl#104