Michal
c2a419b4f7
A per-call timeout bounds one call. It does not bound a stage that calls the
LLM in a loop -- and summarize-tree does worse than loop: buildTree recurses to
maxDepth (3 by default) and iterates per section at every level, then
groupSections iterates again. Hundreds of sequential calls are reachable, so a
10s per-call cap is a multiplier, not a bound.
StageBudget is one wall-clock budget shared across everything a single stage
invocation does. Once it is gone the stage takes its deterministic path --
first-line excerpts, numbered pages -- and says so. The executor builds one per
stage from config.budgetMs ?? MCPCTL_STAGE_LLM_BUDGET_MS (30s) and disposes it
in a finally, so a long-lived mcplocal never accumulates timers.
Two details that matter more than they look:
- A warm cache is never budget-gated. cachedSummarize and generatePageTitles
used ctx.cache.getOrCompute, which makes the budget check impossible to
place correctly; split into get/set so a cached summary -- which costs
nothing -- is still returned when the budget is spent.
- "No LLM configured" is NOT a degradation. It is a deliberate choice, and
numbered pages are the expected output there, so it gets no warning. Only
failures, timeouts and exhausted budgets do. Crying wolf on a working
configuration would train people to ignore the notice.
summarize-tree's single-block path previously had no try/catch at all, so a
failure escaped the stage entirely and executor.ts discarded the whole stage's
work. It now degrades like the others.
Degradation is reported three ways, all from util/degrade.ts: the reason in the
content behind the established "⚠ <feature> unavailable (<reason>)" prefix, the
count of affected sections (the recursion can hit one exhausted budget dozens of
times, so the user needs the reason once plus what it cost), and
degraded/degradedReason on the stage_execution audit event -- previously a
degradation was invisible in the trace.
docs/reliability.md now names ONE helper rather than a list of compliant call
sites. That list is exactly how this drifted: the doc named the gate and
llm/pagination.ts, and the newer stages never joined it.
Tests: the 20-section recursion completes in ~300ms against a 300ms budget;
a warm cache still serves real titles with the budget exhausted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2
175 lines
9.3 KiB
Markdown
175 lines
9.3 KiB
Markdown
# Reliability: don't let a bad LLM take mcpctl down
|
||
|
||
The homelab model changes often (fast ↔ thinking, model swaps, backends that
|
||
drift or go down). mcpctl must stay responsive and honest through all of it.
|
||
|
||
## Principle
|
||
|
||
**LLM-*optional* operations must be time-bounded, fall back deterministically,
|
||
and report the degradation — never hang and never degrade silently.**
|
||
|
||
- **Bounded:** every optional LLM call is wrapped in
|
||
[`withTimeout`](../src/mcplocal/src/util/with-timeout.ts) (Promise.race + an
|
||
`AbortSignal` so fetch-based providers actually cancel). A thinking model that
|
||
streams for minutes can never block the caller.
|
||
- **Deterministic fallback:** when the LLM times out or errors, use the
|
||
non-LLM path (priority/keyword ordering, byte-range pages).
|
||
- **Loud, not silent:** log the reason (`[gate] …`, `[pagination] …`) and tell
|
||
the user. `begin_session` prepends `⚠ Smart prompt-selection unavailable
|
||
(<reason>)…` and sets `degraded: true` + `degradedReason` on the audit
|
||
`gate_decision` event.
|
||
|
||
**One implementation:** [`util/degrade.ts`](../src/mcplocal/src/util/degrade.ts)'s
|
||
`bounded()`. It cannot throw — the caller always gets a value or a reason — so
|
||
the deterministic fallback is unconditional rather than something a `catch`
|
||
block has to remember. `degradationNotice()` produces the `⚠ … unavailable
|
||
(reason). hint` wording, and `degradationAudit()` the `degraded`/`degradedReason`
|
||
payload, so every degradation reads the same whichever subsystem produced it.
|
||
|
||
**Nothing optional is unbounded by construction.** The budget lives in
|
||
`LLMProviderAdapter.complete()` (`proxymodel/llm-adapter.ts`), not at each call
|
||
site, so a stage that passes no options is still bounded and a stage written
|
||
next year inherits the guarantee. One budget spans the whole failover chain —
|
||
a per-provider timeout would make the worst case N × timeout.
|
||
|
||
| Knob | Default | Bounds |
|
||
|---|---|---|
|
||
| `MCPCTL_LLM_CALL_BUDGET_MS` | 20s | one `ctx.llm.complete()`, failover included |
|
||
| `MCPCTL_LLM_PROVIDER_TIMEOUT_MS` | 10s | a single provider attempt inside that budget |
|
||
| `MCPCTL_STAGE_LLM_BUDGET_MS` | 30s | all LLM work in one stage invocation |
|
||
| `MCPCTL_GATE_LLM_TIMEOUT_MS` | 8s | the gate's `begin_session` prompt selection |
|
||
| `MCPCTL_PAGINATION_LLM_TIMEOUT_MS` | 10s | pagination's smart index (`llm/pagination.ts`) |
|
||
|
||
The **stage** budget exists because a per-call timeout multiplies rather than
|
||
bounds when a stage loops: `summarize-tree` recurses to `maxDepth` (3) and loops
|
||
per section at every level, so hundreds of sequential calls are reachable. One
|
||
budget is shared across the whole recursion; when it is gone the stage switches
|
||
to first-line excerpts and says so. A **warm cache is never budget-gated** — a
|
||
cached summary costs nothing, so an exhausted budget must not degrade a result
|
||
we already hold.
|
||
|
||
`read_prompts` is LLM-free by design.
|
||
|
||
### Why this is written down twice
|
||
|
||
This document stated the principle while `proxymodel/stages/paginate.ts` awaited
|
||
`ctx.llm.complete()` with no timeout at all. When a provider hung rather than
|
||
erroring, the promise never settled, the tool call never returned, and the
|
||
client waited out its own 1800s timeout — three such requests in production,
|
||
misdiagnosed twice as an upstream "transport fault". The doc named the gate and
|
||
`llm/pagination.ts` as the compliant sites, and the newer stages simply never
|
||
joined the list. Naming **one** helper here, rather than a list of call sites,
|
||
is what stops that drift recurring.
|
||
|
||
Note: the gate's prompt-ranking uses the **heavy client provider's own model** —
|
||
it deliberately does *not* force the project's vLLM model onto it (doing so made
|
||
every selection fail silently when the model wasn't anthropic-servable).
|
||
|
||
## Instance health: `live` is not `healthy`
|
||
|
||
An MCP server answers `tools/list` from a **static, in-process table**. It costs
|
||
a few milliseconds, needs no credentials, and reaches no upstream — so it stays
|
||
green while the thing the server exists to talk to is unreachable. Treating that
|
||
as a health signal is how `mcpctl get instances` showed eight healthy servers
|
||
while the UniFi one had never once reached its controller.
|
||
|
||
So the probe reports two different passes:
|
||
|
||
| Status | Probe | Means |
|
||
|---|---|---|
|
||
| `healthy` | **readiness** — `tools/call` on `healthCheck.tool` | The upstream answered. The server can do its job. |
|
||
| `live` | **liveness** — `tools/list` only | The process is up and speaks MCP. Its upstream is **unverified**. |
|
||
| `degraded` | either, failing | Failing, but under `failureThreshold`. |
|
||
| `unhealthy` | either, failing | Failed `failureThreshold` times in a row. |
|
||
|
||
`live` is the default for any server with no `healthCheck.tool`. It is not a
|
||
warning — it is an admission that nothing is watching that server's upstream.
|
||
|
||
**Configure a readiness probe on every server.** Pick a read-only tool that
|
||
genuinely round-trips to the upstream, and verify it passes before configuring
|
||
it — a probe naming a local-only tool (`get_..._version`) or a tool the server
|
||
doesn't expose reproduces the same false green it was meant to remove.
|
||
|
||
```bash
|
||
mcpctl create server unifi-network --health-check-tool list_sites \
|
||
--health-check-interval 60 --health-check-timeout 15 --force
|
||
```
|
||
|
||
or declaratively — `healthCheck` round-trips through `get -o yaml | apply -f`:
|
||
|
||
```yaml
|
||
healthCheck:
|
||
tool: list_sites
|
||
arguments: {}
|
||
intervalSeconds: 60
|
||
timeoutSeconds: 15
|
||
failureThreshold: 3
|
||
```
|
||
|
||
Omit `tool` to keep liveness while still tuning the timings.
|
||
|
||
Latency is the tell: a probe answering in single-digit milliseconds is reading a
|
||
local table, not crossing a network. The UniFi probe went from 3ms (`tools/list`,
|
||
lying) to 1847ms on its first real `list_sites` — login, TLS, controller round
|
||
trip — and ~40ms once the session was warm.
|
||
|
||
### Where a failing readiness probe usually points
|
||
|
||
Turning these probes on for the first time took the fleet from "8/8 healthy" to
|
||
three genuine failures in under a minute. All three were network shape, not
|
||
code — check these before suspecting the server:
|
||
|
||
1. **Egress port.** MCP server pods default to TCP 80/443 only
|
||
(`servers-allow-external-egress`). Any upstream on another port — the UniFi
|
||
controller on `:8443` — times out on every call. Declare it in Pulumi's
|
||
`mcpctl.serverEgressTargets`; don't widen the blanket rule.
|
||
2. **Ingress hairpin.** A co-located service reached over its *public* hostname
|
||
goes out and back through the per-host Envoy L7 policy, which doesn't
|
||
reliably carry the caller's identity and replies with a bare `Access denied`.
|
||
Grafana 403'd on every call this way while the identical token succeeded from
|
||
a laptop. The tell is the error *shape*: plain text, not the upstream's own
|
||
JSON error. Use the ClusterIP (`serverEgressTargets` with `namespace:`).
|
||
3. **Address reachability.** A pod cannot reach a **Tailscale** `100.64.0.0/10`
|
||
address. Config pointing at one connect-timeouts forever. Use LAN IPs. (This
|
||
one turned out to be a retired service, which is its own kind of answer.)
|
||
|
||
Also check the *dialect*: UniFi's `controller_type` must be `classic` for a
|
||
self-hosted controller (login `/api/login`, no `/proxy/network` prefix).
|
||
`unifi_os` sends every request to a path that 404s.
|
||
|
||
## LLM-*essential* operations — failover chain
|
||
|
||
Chat needs *an* LLM but not a *specific* one. Instead of failing when the pinned
|
||
model is down, chat **fails over across an ordered chain** and **reports which
|
||
model actually answered**.
|
||
|
||
- **Chain:** an `Llm` declares fallbacks in `extraConfig.fallbacks: string[]`
|
||
(Llm names, in order). The dispatcher builds an ordered candidate list —
|
||
the primary's pool, then each fallback's pool — and tries them in order.
|
||
- **Fails over on real failures**, not just transport: a non-2xx status
|
||
(e.g. a drifted model's `400`) or an empty/invalid completion now advances to
|
||
the next candidate (`chat.service.ts` `runOneInference`). Streaming fails over
|
||
pre-first-chunk.
|
||
- **Transparency:** `ChatResult` / the SSE `final` frame carry `llm`, `model`,
|
||
and `failedOver`. The CLI prints `model: <llm> (<model>)` per turn, and
|
||
`⚠ failed over → answered by <llm> (<model>)` when a fallback was used.
|
||
- **Exhaustion is clear:** if every candidate fails, the error names the last
|
||
model + upstream status/body (not "no choice").
|
||
|
||
Homelab chain: `vllm-current` (the served vLLM) → an `anthropic-fallback` server
|
||
Llm as the always-up last resort. The chain is owned declaratively by Pulumi
|
||
(`kubernetes-deployment/deployments/mcpctl/llm-target.ts`, `fallbacks` arg) so a
|
||
`pulumi up` can't wipe it — mcpd *replaces* `extraConfig` on update.
|
||
|
||
**A real last resort needs an independent cloud credential.** LiteLLM only fronts
|
||
the single local vLLM, so any local model shares the same GPU-box failure as the
|
||
primary. `anthropic-fallback` points at `api.anthropic.com` and only serves once
|
||
mcpd Secret `anthropic-key` holds a genuine **`sk-ant-api03` API key** (from the
|
||
Anthropic Console). A Claude *subscription* OAuth token (`sk-ant-oat…`, what
|
||
`claude setup-token` mints) does **not** work: Anthropic gates those to the Claude
|
||
Code client — probed directly they 404 on older model ids and 429 on current ones.
|
||
mcpd's anthropic adapter does send OAuth tokens via `Authorization: Bearer` (so
|
||
auth *passes*), but the gating is server-side and unavoidable. Until a real key is
|
||
set, the fallback is wired-but-inert: an outage fails over to it and surfaces a
|
||
clear `anthropic-fallback (model) HTTP 4xx` error rather than improving uptime.
|