Migrating docmost and my-home-assistant, both rendered their secret and
neither picked it up. The pod spec showed why:
command: (empty)
args: ["node","build/index.js"]
server.entrypoint: (unset)
wrapCommand consulted only `server.entrypoint` for non-package servers, so
with `entrypoint` unset it returned undefined, the wrapper was skipped
entirely, and the container ran its normal command — which never sourced
/vault/secrets/<name>. The failure is silent by construction: the agent
init container succeeds, the file is there, and the server simply starts
with empty credentials.
An explicit `command` on an image server is already a complete command
line — mcpd's exec mode would run exactly it — so it should be wrapped
verbatim. `entrypoint` is only needed when there is no command at all and
the image's own ENTRYPOINT would take over.
Now branches on the three real shapes: package server (prepend the runner
entrypoint mcpd owns), image + command (use verbatim), image only (require
the declared entrypoint).
Seven tests, one per shape plus the two undefined cases. Reintroducing the
old logic fails two of them — checked before keeping.
Both servers were rolled back to secretDelivery: env and are healthy; they
can migrate once this ships.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
With injected delivery the credentials exist ONLY in PID 1's environment:
the container command is a shell that sources /vault/secrets/<name> and
execs the real server, so the values live in that process and nowhere
else — not in the pod spec, which is the whole point.
mcpd picks its STDIO mode from the server's shape: an explicit command or
a packageName selects `exec`, a bare dockerImage selects `attach`. `exec`
spawns a NEW process inside the container, which never sourced the file
and therefore starts with empty credentials. The server comes up, answers
tools/list, and fails every authenticated call — precisely the silent
empty-token failure this feature exists to prevent.
Seen as `Readiness check (list_datasources) failed: process exited 1` on a
pod whose PID 1 demonstrably held the token (verified via /proc/1/environ:
GRAFANA_URL set, token 46 chars, nothing in the pod spec).
So secretDelivery: injector now forces attach regardless of server shape.
Safe because wrapCommandForInjector execs rather than forks, so PID 1 IS
the server. It also means no bouncer or HTTP shim is needed — mcpd's
PersistentStdio already holds one long-lived connection; it was simply
pointed at the wrong process.
Note this inverts an assumption I had earlier: image-entrypoint servers
were already on the attach path and would have worked; it is the
package-based ones that were broken. gitea remains the sole exception, and
for the unrelated reason that it is distroless and has no shell to source
the file.
Extracts the decision as a pure `chooseStdioMode()` so it can be tested —
it is subtle and fails silently. Six cases pinned, including that env
delivery still execs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Caught migrating a real server: the pod crashlooped with
`.: cannot open /vault/secrets/grafana-creds`, and the reason was in the
generated spec:
args: ["/bin/sh","-c",". /vault/secrets/grafana-creds; exec \"$0\" \"$@\"",
"@leval/mcp-grafana"]
mcpd deliberately maps ContainerSpec.command -> k8s `args` so a package
server keeps its runner image's ENTRYPOINT (`npx -y`, `uvx`). Putting the
sourcing wrapper there meant the pod actually ran
`npx -y /bin/sh -c '...'` — npx trying to resolve a package called
/bin/sh. The agent had rendered the file correctly; nothing ever sourced it.
Adds `ContainerSpec.entrypoint`, which maps to k8s `command` and so
REPLACES the image entrypoint, and has wrapCommand fold that entrypoint
into the argv it returns (`npx -y` / `uvx` for package servers, the
server's own `entrypoint` field for dockerImage servers — already required
at validation for exactly this reason).
Two tests pin it: a wrapped server emits `command` and no `args`; an
unwrapped one still emits `args` and no `command`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
`mcpctl patch server my-grafana secretDelivery=injector` printed
"patched server 'my-grafana'" and changed nothing. The repository maps
update/create fields explicitly, one by one, so a new column silently
does nothing until it is added there — and the silence is total: the API
returns 200, the CLI reports success, and `get -o yaml` still shows the
old value.
Caught by trying to migrate a real server, not by any test.
Adds the two fields to both create and update, plus tests that assert the
mapping directly. Those tests fail against the unfixed repository (3 of 4)
— verified before keeping them.
This class of bug will recur: the mapping is manual and nothing links a
schema column to it. The tests at least make the next omission loud for
these two fields.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Agent worktrees are scratch checkouts. Without this a plain `git add -A`
sweeps them in as embedded git repositories, which clone as empty
directories for everyone else.
Completes the path that stops mcpd writing secret VALUES into MCP server
pod specs. With `secretDelivery: injector`, the pod fetches its own
secrets from OpenBao through the agent injector, under a ServiceAccount
and role scoped to just that server's secrets — so the value never enters
etcd, and gitea-mcp cannot read the Grafana token.
Opt-in per server, defaulting to `env`. Every existing server is
bit-for-bit unchanged, and migrating is one reversible decision at a time
rather than a flag day.
The two invariants under most risk, both tested:
- **Opted-out servers produce an identical manifest.** No annotations, no
serviceAccountName, automountServiceAccountToken still false.
- **Opted-in servers still fail LOUDLY on a bad ref.** Once mcpd stops
reading a server's secrets, the check e6cd735 added no longer fires for
it, and a typo'd secretRef would degrade into a vault-agent-init
crashloop that mcpd reports as a generic pod failure — the same class of
bug that had gitea-mcp running for weeks on an empty token while
reporting healthy. `validateServerEnvRefs` resolves every ref and throws
the value away, purely to keep that error. After the value cache it is a
cache hit and costs nothing.
Shell quoting is the other silent-failure trap and is treated as part of
the contract: the agent renders `export NAME='value'` and the container
command sources it, so a value containing a space, `$`, a quote or a
newline would truncate and yield an empty token. `shellSingleQuote` is
tested by executing a real /bin/sh over nine adversarial values including
`'; export PWNED=1; '` — and those tests fail against naive quoting,
confirmed before keeping them.
`sh -c <script> arg0 arg1 …` preserves argv via $0/$@, and `exec` keeps
PID 1 as the real process, which matters because mcpd attaches to PID 1's
stdin/stdout for STDIO servers.
Docker/Podman declare `capabilities.secretRefs: false` and fall back to
inline resolution, so local development is untouched. Deleting a server
revokes its identity, after its pods are gone and best-effort — a role no
pod can authenticate as grants nothing, and failing the delete over it
would strand the row.
Per the CLI rules, `secretDelivery`/`entrypoint` are `create` flags,
round-trip through apply -f, and show in `describe server` — which now
also flags servers still inlining secrets into their pod spec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
ServerIdentityService shipped without tests. Containment is the whole
point of the design, so it needs assertions rather than trust: a shared
role would let third-party MCP images (gitea-mcp, ha-mcp) read every
secret under secret/mcpctl/*, which is worse than the pod-spec exposure
it replaces.
Eight cases, all about what a server must NOT get: the grant covers only
the secrets that server declares; a secret referenced twice is one grant,
not two; inline env values never widen it; another server's secrets never
appear. Plus the ordering invariant (ServiceAccount before the role that
binds it — the reverse lets a pod start, fail to log in and crashloop
while the role is still being written), and that a backend which cannot
scope identities REFUSES rather than silently succeeding, which would
leave a pod believing it held access it never got.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Found by the readiness probe added in the previous commit, on its first
run against production: `mcpctl status` reported
Secrets: bao-k8s* ✗ auth failed: OpenBao list: HTTP 400 Bad Request
That is a real bug, not a probe artifact. `bao-k8s` is configured with
the public ingress URL, and Cilium's ingress Envoy rejects the
non-standard LIST HTTP method outright. Verified live from the mcpd pod:
LIST https://bao.ad.itaz.eu/v1/secret/metadata/mcpctl/ -> 400 Bad Request
GET https://bao.ad.itaz.eu/v1/secret/metadata/mcpctl/?list=true -> 403 permission denied (bogus token, i.e. reached bao)
LIST http://openbao.openbao.svc:8200/... (ClusterIP, no Envoy) -> 403 permission denied
So the verb was fine against bao and fatal through the ingress. OpenBao
accepts both forms and documents the GET form for exactly this reason.
This was never noticed because nothing called list() in production —
it backs `mcpctl migrate secrets`, which would have failed with an opaque
HTTP 400 against any ingress-fronted backend.
Also adds the per-server scoping primitives Phase 2 needs, with tests:
buildServerSecretPolicyHcl (no wildcards, read-only, stable under
reordering), buildServerProvisioningPolicyHcl (prefix-confined so mcpd
cannot grant itself more than it holds), ensureKubernetesAuthRole /
delete helpers, the driver-level ensureServerIdentity/removeServerIdentity
capability, and ServerIdentityService.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
SecretBackendRotatorLoop had zero tests, despite being the detector added
in e51b924 specifically so a re-initialised OpenBao surfaces the moment
mcpd boots rather than 24h later when the scheduled rotation finally
fires. The class already injects setTimeout/clearTimeout and a logger, so
this needed no production change.
Nine cases, weighted to what actually breaks: the boot health check runs
per rotatable backend; a dead token emits kind BACKEND_TOKEN_DEAD through
the injected logger (the point of the earlier console.error removal — a
bare console call never reaches ErrorLogBuffer, so `mcpctl errors` could
not see it); a throwing health check does not abort start(); overdue
backends rotate immediately and still get scheduled; the 60s floor holds
across 50 adversarial-jitter draws; and stop() both clears timers and
trips the `stopped` guard against rescheduling, which had never been
exercised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Two of these four assertions currently FAIL against the live cluster, which
is the point: mcpd there still renders `Secrets: bao-k8s* ✓` from the
rotation field, and its JSON status carries no live/ready pair. Per the
project rule the fix is to deploy, not to relax the assertion.
The status check specifically rejects a bare "name ✓" with no qualifier —
that string is the old rendering and proves the probe was never consulted.
Deliberately does not take the real OpenBao down. Simulating an outage
against shared infrastructure to satisfy a test would be worse than the
bug it covers; the outage paths are unit-tested with an injected clock.
Docs: adds a Reliability section (request hardening, the stale-while-error
table, the cold-cache gap and why persisting values is not the answer,
live-vs-ready) and replaces the stale "Kubernetes ServiceAccount auth is
not shipped yet" note — it shipped in 5152066, and it is what the live
backend has used since June.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
`mcpctl status` derived its Secrets verdict entirely from
`tokenMeta.lastRotationError`. The rotator writes that field only for
`auth: 'token'` backends (SecretBackendRotator.isRotatable), so a
`kubernetes`-auth backend never wrote it and the line rendered a green
tick unconditionally — including with OpenBao sealed, unreachable, or
answering 403 to every read. The one signal we had was structurally
incapable of going red for the backend we actually run.
New `GET /api/v1/secretbackends/:id/health` reports two signals, kept
separate on purpose:
live — reachable at all? (unauthenticated sys/health)
ready — can we read through it? (uses our credentials)
live-but-not-ready is the exact shape of a re-initialised OpenBao handing
back valid-looking tokens that grant nothing; collapsing both into one
boolean is what hid that for four days. Needs no RBAC mapping — it falls
through to the generic `secretbackends` resource, so a GET is
`view:secretbackends`.
`mcpctl status` now renders four states — reachable / degraded (serving N
cached secrets) / unreachable / auth failed — with rotation error demoted
to a trailing clause rather than the verdict. A failed probe renders
"? unknown", never green: not knowing is not health. JSON output carries
the same probe, so scripts stop being told every k8s-auth backend is fine.
Also adds a boot-time cache warm. The stale-while-error cache can only
absorb an outage for secrets it has already seen, so a cold mcpd during a
backend outage still fails; resolving each running server's refs once at
startup closes that for the common case. Best-effort and deliberately
partial — if the backend is also down at boot this is a no-op and
instances fail loudly, which is correct. Persisting last-known-good to
Postgres or disk would just be plaintext-at-rest again.
Tests: 15 new. The five status assertions were confirmed to fail against
the old rotation-only logic before being kept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
mcpd re-read the secret backend on every use — server env resolution, LLM
api keys, chat, git providers, code repos, webhooks — with no value cache,
no request timeout, and a retry that only fired on HTTP 403. A few seconds
of OpenBao unavailability therefore turned into minutes of degraded
service: instances that restarted during the blip failed env resolution,
got marked ERROR, and entered the 30s x5 then 5min backoff.
Three changes, in dependency order:
1. Typed errors. `SecretNotFoundError` (definitive) vs
`SecretBackendUnavailableError` (transport). The distinction has to be
typed rather than string-matched — a mis-classified "not found" would
resurrect deleted secrets, and a mis-classified auth failure would
paper over revoked grants, which is how an upstream re-init once broke
every secret write for four days (e51b924).
2. Driver resilience. Every request now carries an AbortSignal timeout
(there was none, so an unreachable backend hung its caller) and retries
5xx/429/network with full-jitter backoff — 503 is what a sealed
OpenBao returns and used to be an immediate hard failure. The 403
purge-and-retry stays single-shot and outside the retry budget: it is a
credential refresh, not a backend-unavailable condition, and looping on
it would hide a genuinely revoked grant.
`healthCheck()` no longer routes through the authenticated path, so an
expired role stops reporting as "OpenBao is down"; it maps OpenBao's
status codes (sealed/standby/uninitialised) instead. New `authCheck()`
covers the readiness half via list(), which exercises the capability we
actually depend on — unlike lookup-self, which only proves the token
exists.
3. CachingSecretBackendDriver. Fresh reads inside the TTL never touch the
network; past it we always try the backend, and on a transport failure
serve the last known-good value instead of throwing. That is what stops
the ERROR storm. Deleted secrets evict and rethrow — serving those
stale would resurrect a revoked credential, strictly worse than an
outage — and non-transport errors rethrow untouched. plaintext is not
wrapped: its read() is an identity function over the row handed in.
A cold cache during an outage still fails, loudly and by design (e6cd735).
Also routes BACKEND_TOKEN_DEAD / BACKEND_ROTATION_FAILED through pino
rather than bare console.error. They bypassed the multistream feeding
ErrorLogBuffer, so the one failure `mcpctl errors` exists to surface was
the one it never showed.
Tests: 20 new. The two load-bearing guards (never serve a deleted secret
stale; never serve stale for a non-transport error) were confirmed to fail
against deliberately broken code before being kept. The 403 purge-retry
path had no coverage at all until now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki
Accept only merges if the task carries a prNumber, and nothing said how a
human sets one. It goes through the generic patch command; without this the
merge gate reads as unreachable from the CLI.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
Five resources shipped with no documentation and one documented lie:
docs/inference-tasks.md still told people `mcpctl get tasks` lists the
inference queue. Work items own that name now.
Adds docs/epics.md (the Epic -> EpicTask -> Task hierarchy, the state machine,
risk-scaled review gates, the accept-merges gate, and reconciliation),
docs/gitproviders.md (forge registration, discovery/import, merge-on-accept,
post-merge reopen) and docs/agent-authority.md (the three run modes, and
plainly which two are not implemented).
Two things found while writing them, both fixed rather than filed:
- `webhookSecretKey` was read by the webhook route but writable from NOWHERE
-- absent from the create schema, update schema, service, CLI and apply
spec. Since the endpoint fails closed on a null key, post-merge CI failures
could never reopen their task; the board would keep claiming `merged` for
work that broke the build. Adds --webhook-secret-key, the schema fields and
the apply key, with tests covering persist / omit / explicit-null /
reject-empty. Omission deliberately does not clear the column, so an
unrelated edit cannot silently disarm the webhook.
- src/cli/src/commands/create.ts contained a literal NUL byte inside a string
literal (a binary-file check written with the raw character rather than an escape). Identical behaviour, but it made the whole file read as
binary, so `grep -I` skipped it entirely and 1120 lines of CLI were
invisible to every text search. Replaced with the escape.
The Epic write path (criteria/status/tradeoffs) is API-only -- documented as
such rather than papered over, and filed as task 180.
Full suite 2612 green; lint at baseline 794/77.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
Closes 177. An agent carries a project's prompts and its MCP tools, and the only
check on a chat turn was run:agents:<name> - so anyone who could talk to an
agent inherited tool access to every project that agent had joined. Membership
was the capability. AgentProject widened it from one pinned project to many.
The rule now: knowledge travels with the agent, action is bounded by the
authority of the run. Prompts and skills still load regardless of who is asking -
a QA reviewer that forgets the project's conventions when a different person
asks is not a reviewer. Every MCP tool call is gated.
Authority is resolved once per run (services/run-authority.ts) and is per-RUN,
not per-agent: the same agent answers a human under that human's authority and,
once a scheduler exists, runs unattended under its own. `caller` intersects the
agent's memberships with the caller's run:projects - reusing the exact grant the
project-chat route already demands, rather than inventing a parallel permission.
`agent` takes every membership without consulting a user. Both are implemented;
`service-account` throws 501 rather than honouring a forgeable x-service-account
header, and task 179 records what it needs first.
Gated in BOTH listTools and callTool, deliberately: withholding a tool from the
list is presentation, not enforcement - a model can invent a tool name it was
never shown, and callTool is what would run it.
The accepted consequence is stated plainly rather than papered over: a user can
ask about a project they cannot reach and the agent will answer from its
knowledge. The system block tells the model when it is holding such context so a
persona can apply it without reciting it. That is guidance to a model, not a
boundary - the boundary is that the tools are absent.
Two sibling holes found while exploring and closed here:
- Thread hijack. resolveThreadId on the agent path had no owner check, so anyone
with run:agents:<name> could append to another user's thread by id and read
its history back. The project path already did this correctly; mirrored, 404
not 403 so ids cannot be probed.
- McpToken project escape. `project` travels in the request BODY, so the
URL-based token scope check in main.ts never saw it - a token bound to project
A could drive an agent into B's tools. The binding is now a ceiling.
Tests: 19 authority tests (intersection, fail-closed with no user and with no
RBAC wired, token ceiling, agent authority bypassing RBAC entirely, the 501) and
4 through the real chat path asserting prompts load while tools do not.
Three existing assertions updated for the new dispatcher signature - a
deliberate interface change, not tests bent to fit.
Local: build clean, workspace 2608 passed (+16), lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
A container restart (OOMKill, crash, transient npx failure) used to strand
the instance in ERROR forever while its pod sat 1/1 Running, because five
gaps lined up (#114):
- the exec/attach websocket died without ending the stdout PassThrough
(client-node installs no onclose), so PersistentStdioClient — whose only
death signal was stdout 'end' — kept believing it was connected and every
request rode the 120s timeout into a dead pipe;
- the stdioClients cache is keyed by pod name, which survives a restart, so
nothing ever evicted the corpse;
- syncStatus never re-inspected ERROR rows and never read restartCount, so
neither the recovery nor the in-place restart was visible;
- its ERROR writes clobbered retry metadata, making the row instantly
dueForRetry, and the retry recreated the pod under the SAME name — an
uncaught 409 that looped ERROR against a healthy pod;
- the stuck row consumed the whole replica budget, blocking a fresh-id
replacement.
The fix, layer by layer:
- ws 'close'/'error' now end stdout in both k8s interactive paths (and the
docker interactive path mirrors its own one-shot handlers), funneling into
an identity-guarded teardown in PersistentStdioClient that also listens
for stream 'close'/'error' — a late event from a previous session cannot
clobber a reconnected one;
- syncStatus re-inspects ERROR rows (pod running again → back to RUNNING
with retry metadata cleared), tracks restartCount in instance metadata to
catch restarts between polls, MERGES retry metadata instead of clobbering
it, and evicts the cached stdio client through a setter-injected hook
whenever the pipe is known-dead;
- createContainer adopts an alive pod on 409 instead of throwing (and only
replaces a genuinely dead one, waiting out the deletion grace period);
- attach mode retries once through a fresh client before surfacing an error,
so the first call after a detected death succeeds;
- the health probe evicts the cached client when failures cross the
threshold, and shutdown finally calls closeAll().
Closes#114
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
Closes 151 — the last loop in the lifecycle. A task reaches `merged`, CI then
fails on the merge commit, and the same task comes back as `reopened` instead of
the breakage going unnoticed because the board already said done.
Two properties make this safe rather than a liability.
**It fails closed.** The endpoint is unauthenticated by nature — a forge calls
it — so it accepts nothing unless the provider has a configured webhook signing
secret AND the HMAC verifies. No secret, no effect. The signature is computed
over the RAW body: re-serializing a parsed object changes the bytes, which is
the classic way this check silently stops working, and there is a test for it.
A 404 for an unknown provider is deliberately vague, because confirming which
provider names exist is free reconnaissance on a public endpoint.
**It matches precisely.** Accept now records `merge_commit_sha` — the commit the
merge actually produced, which is what CI runs against — so a failing status
finds exactly the task that caused it. The alternative, reopening "whatever
merged most recently", would reopen innocent work. A failure on a commit mcpctl
did not merge is somebody else's business and is ignored, as is a task that has
already moved on from `merged`.
Capturing the merge commit is best-effort by design: the merge already
succeeded, so failing to re-read the sha must not fail the accept — it only
costs the webhook its precise match later.
The route is skipped by the RBAC hook, since the forge has no mcpctl identity.
That is only safe because of the fail-closed rule above, and both the skip and
the reason are asserted in tests.
Tests: 12 covering every refusal path (no secret, unsigned, wrong key, signature
over different bytes), the vague 404, the RBAC skip, and the reopen matrix.
Local: build clean, workspace 2592 passed (+12), db 128 passed (same 4
pre-existing), lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
The instructions fetch was the one downstream call in getOrCreateRouter
still using mcpdClient — whose token is an empty string in HTTP mode — so
mcpd answered 401, the catch swallowed it, and every session initialized
with no instructions while nothing looked broken. requestClient exists
precisely for this; use it.
Closes#113
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
McpdUpstream.send routed every non-list method through the default-timeout
client, so any tool that legitimately runs past 30s — web_url_read through
a browser solver, a large PDF extraction, a slow retail site — died as
"mcpd proxy error: mcpd did not respond within 30000ms" while mcpd was
still working on it. LibreChat agents hit this constantly on real pages.
New TOOLCALL_TIMEOUT_MS (default 120s, env MCPLOCAL_TOOLCALL_TIMEOUT_MS)
sits between DISCOVERY_TIMEOUT_MS (list calls must stay short so a dead
upstream can't stall session init) and LONG_RUNNING_TIMEOUT_MS (chat and
inference, minutes). withTimeout preserves the caller token and headers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
Closes 148/149 — the accept gate now lands code instead of just moving a row.
Driver gains createPullRequest / getPullRequest / mergePullRequest, verified
against the real Gitea API shapes rather than guessed: MergePullRequestOption
requires a capitalised `Do`, and supports `head_commit_id`.
That `head_commit_id` is the point. A human approved a SPECIFIC commit; if
someone pushed after that, merging anyway lands code nobody reviewed. So the
reviewed sha travels with the request and is checked twice — once by us for a
clear MergeStaleError naming both commits, and again by the forge, which closes
the race between our read and the merge. When it trips, the message says what
happened and what to do: re-review the current head, then accept again.
Accept merges BEFORE it transitions. If the forge refuses — conflicts, failing
checks, a moved branch — the task stays in awaiting_review and no event is
written. Reporting `merged` while main does not have the code is the single
worst thing this gate could do, and there is a test asserting the transition
never happens on a failed merge.
Failure modes are answers, not stack traces: no PR attached is approval-only and
succeeds; a repo with no forge connection explains that mcpctl cannot merge it;
an already-merged PR is treated as success so a retry after a timeout is safe.
owner/repo comes from `sourceFullName` when discovery recorded it, falling back
to parsing https, ssh:// and scp-style clone URLs, and returning null rather
than guessing when it cannot tell.
The merger is injected and optional, so an mcpd without a git provider still
runs and accept degrades to approval-only — which is also what the help text
now says, restored after the earlier commit that removed a claim we could not
back up.
Tests: 7 driver PR tests (Do field, head pinning, stale refusal, idempotent
re-merge, 405 → refusal), 10 merger tests incl. URL parsing, 7 accept-gate tests.
Local: build clean, workspace 2580 passed (+23), lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
`mcpctl tasks accept --help` said "fires the merge". Nothing merges. There is
no createPullRequest or mergePullRequest anywhere in the codebase — the
git-provider client is task 148 and is not built.
Accept records human approval and moves the task to `merged`. Until 148/149
land, `merged` means "a human approved this", not "this is in main", and the
help text and service comments now say so. A user who accepted a task expecting
a PR to land would have been silently wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 7 called release.sh — which restarts mcplocal and runs the smoke suite
against the binary it just installed — and then restarted mcplocal and ran the
whole suite again. Two full suites inside a minute trips mcpd's rate limiter,
so the second run came back with six 429s and printed
SMOKE TESTS FAILED — system may be unhealthy. Consider rollback
over a deploy that was fine. Seen for real on 35d506d: the first suite was
162/162 green, the second failed only on `mcpd returned 429: Rate limit
exceeded`, and a clean re-run afterwards was 162/162 again.
A deploy script that recommends rolling back a healthy release is worse than
one that says nothing. One run, one verdict — release.sh's exit status.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU
UniFi tools were unusable from non-Claude agents. A tool result over 2000
chars is replaced with a table of contents and re-read by calling the tool
again with _resultId/_section, but those params were never declared on the
tool's inputSchema — and unifi-network and my-grafana ship
`additionalProperties: false`, so for a client that validates arguments the
drill-down call was illegal and the data unreachable. The stub's own wording
made it worse: "Use section parameter" names a param that does not exist;
sending it fell through to the upstream, re-paginated, and minted a fresh
_resultId. An unbounded loop.
UniFi took the blame because it is the one server whose results always trip
the threshold: get_devices was 11,718 chars, get_clients 136,696 across 19
pages with a 92-char page-1. Grafana and Gitea return compact results.
- content-pipeline gains onToolsList, declaring _resultId/_section on every
tool it can paginate (gate tools excluded — they are intercepted before the
pipeline runs). additionalProperties stays false: a property listed in
`properties` is already legal under it, so declaring is enough and the
upstream keeps its typo protection.
- createDefaultPlugin wired only the gate's onToolsList, so the pipeline's
would have been dropped on the floor. Both now chain, gate first.
- _resultId without _section re-shows the table of contents instead of
forwarding an unknown argument to a strict upstream.
- The stub names the tool, the live _resultId and a real section id.
- Nested MCP envelopes are collapsed. A server fronting another MCP server
returns the inner result wrapped in its own content/structuredContent pair,
so the payload arrives two or three times over. A layer is peeled only when
it carries nothing the inner value lacks, which keeps it lossless. Live
against the sre project: get_devices 11,718 -> 5,210 chars and no longer
paginates at all; get_clients 136,696 -> 62,358 across 8 pages instead of 19.
- deploy/mcplocal.service shipped MCPLOCAL_MCPD_URL=http://10.0.0.194:3100,
which is dead — every working machine had a hand-written drop-in. Points at
the k8s ingress now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU
Servers can declare persistent volumes. The backing store is keyed on the
server (mcpctl-<server>-<name>), not the instance, so data survives the
instance recreation that any server edit triggers — a Kubernetes PVC ensured
before the pod is created and never deleted with it, or a Docker named volume.
Three templates, all self-hosted and none needing an API key:
- duckduckgo — web search with no backing service at all
- searxng — better results, needs a SearXNG engine
- docs-mcp — open-source Context7/Ref alternative, uses the new volume
Verified against the cluster: PVC bound 20Gi RWO longhorn, and a sentinel plus
the SQLite index survived a full instance destroy/recreate onto a new pod.
Includes two fixes it took to get there, both latent beforehand:
- client-node v1 puts the HTTP status on ApiException.code, not .statusCode,
so every expected 404/409 was being rethrown — that also broke
removeContainer against an already-deleted pod
- pods with volumes need fsGroup, or a fresh root:root claim is unwritable to
any image that drops privileges
Observed live: turn 1 died with an anthropic 429 before the stream's `final`
frame, streamOnce resolved '' as the thread id, the REPL stored it, and every
later message sent `threadId: ""` — rejected by mcpd's z.string().min(1) with
HTTP 400. Permanently stuck: no turn could succeed again, so no `final` frame
could ever repair the id.
Two independent layers:
- streamOnce now resolves `string | undefined` — undefined when no `final`
frame arrived — and the REPL keeps its previous thread state on undefined
instead of overwriting it. One-shot mode skips the `(thread: ...)` footer
when there is none to report.
- chatBody refuses to serialize an empty threadId at all, so even a leaked ''
can never reach the wire.
Regression cover in chat-thread-brick.test.ts (7 tests), including the full
REPL chain: failed turn 1 → turn 2 body carries no threadId key. The
assertions are the direct inverse of the old behavior, so they fail pre-fix
by construction.
CLI suite 726 passed, lint clean, tsc clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016dNpnBqyyz9GxfznVcX2sP
`mcpctl chat reviewer` showed nothing until the turn finished, then dumped
the whole answer at once. mcpd streams token deltas and the CLI renders
them incrementally — the sole buffering point was mcplocal's catch-all
/api/v1/* proxy reading the whole SSE body via res.text() before replying.
The previous commit (cherry-picked from feat/agentic-teams) pipes the body
through instead; this one adds the cover that was missing:
- proxy-long-running.test.ts: a progressive-delivery test in which the
stand-in mcpd withholds its final frame until the client has observed the
first delta through the proxy. A buffering proxy cannot satisfy that
ordering — verified: the test fails in 3s (no hang) against a res.text()
proxy and passes against the piped one. The existing SSE test used
inject(), which collects the whole body and so passes either way.
- agent-chat.smoke.test.ts: a live smoke that posts through mcplocal on
localhost:3200 (the path `mcpctl chat` actually takes — every other chat
smoke uses --direct and bypasses the proxy entirely) and asserts delta
frames arrive spread across the generation window, not in one burst at
stream end. Uses its own agent: the shared smoke agent pins replies to a
single token, too short to tell live streaming from a buffer dump.
Also settles the strict-boolean-expressions lint on the auth-header check
the streaming split touched.
Local: workspace 2546 passed, proxy suite 10/10, smoke file loads + self-skips.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016dNpnBqyyz9GxfznVcX2sP
`mcpctl chat <agent>` failed with
HTTP 503 {"error":"service_unavailable","message":"Cannot reach mcpd daemon. Is it running?"}
while mcpd was answering /healthz in 32ms. The message was wrong in a way that
cost real debugging time: mcplocal was reaching mcpd fine and giving up after
30s. journalctl shows the signature plainly — statusCode 503 with
responseTime 30003.87 on POST /api/v1/agents/reviewer/chat.
This blocks the agentic-teams epic outright. An agent turn is a multi-turn
tool-use loop that runs for minutes by design, so a 30s ceiling on the chat path
is not a safety net, it is a guaranteed failure for every non-trivial turn.
Three defects, all in the same path:
1. One blanket budget for every forwarded route. DEFAULT_TIMEOUT_MS = 30_000 is
right for CRUD and wrong for chat. Chat, project chat, llm infer and
inference-task streams now get LONG_RUNNING_TIMEOUT_MS (600_000, override
with MCPLOCAL_LONG_TIMEOUT_MS) — matching STREAM_TIMEOUT_MS, which the CLI
already allowed. mcplocal in the middle was the binding constraint.
2. Timeouts were reported as connection failures. Split UpstreamTimeoutError
out of ConnectionError and map it to 504 with an accurate message that says
the daemon IS reachable. ConnectionError still means unreachable and still
returns 503. Verified nothing else branches on ConnectionError.
3. SSE was buffered. `forward()` reads the whole body through res.text(), so
even turns that finished in time arrived as one blob and the CLI's live
token output never appeared. Streaming routes now use forwardStream() and
pipe the body straight through, preserving content-type and
x-accel-buffering (dropping the latter lets intermediaries re-buffer and
reintroduces the stall).
Also closes the escape that produced the sibling `500 code:23` failure: the body
read in forward() was outside the try, so when mcpd had already written SSE
headers the raw DOMException reached Fastify unhandled.
Tests: 9 new proxy tests. The two that matter — "does not abort an agent chat
that outlives the CRUD budget" and "streams SSE through instead of buffering" —
were confirmed to FAIL against the pre-fix behaviour and pass after. Three
existing mcpd-client tests asserted the old taxonomy and were updated to assert
the new one deliberately.
Local: build clean, workspace 2375 passed, lint unchanged at 869.
NOT YET LIVE: mcplocal runs from the installed RPM, so this needs a package
rebuild + `systemctl --user restart mcplocal` to take effect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
release.sh printed `WARNING: Smoke tests failed!` and exited 0. That is how four
broken readiness probes shipped unnoticed on 2026-08-10 — the warning scrolled
past in the build log and the release reported success.
It now exits 1, with `MCPCTL_ALLOW_SMOKE_FAILURE=1` as the escape hatch. The
message is explicit that smoke runs LAST, against the installed binary: the
package is already published and installed, so the failure reports fleet
breakage rather than preventing a bad artifact.
Turning the gate on required fixing a latent hang first, or every release would
have blocked on it. `security.test.ts > /inspect SSE endpoint …` waited for a
response body that by design never ends, so it could only settle via the
socket's *inactivity* timeout — and /inspect relays every project's MCP traffic,
so during a full smoke run it is never idle. Run alone it passed and looked
flaky; run with the suite it failed every time.
httpRequest gains `headersOnly`, which resolves on the response headers and
hangs up. The assertion only ever needed the status line.
Verified: full smoke suite 158/158 (was 157/158 with this test timing out); the
gate block lifted verbatim from release.sh exits 1 with a stubbed failing smoke
run, and exits 0 reaching subsequent code under MCPCTL_ALLOW_SMOKE_FAILURE=1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wUmrfkVQR6CKcYKxENq7k
health-readiness.smoke.test.ts asserts no RUNNING server lacks a
healthCheck.tool. The smoke suite's own shared fixture had none, so the suite
reported its own scaffolding as a fleet regression — and because the fixture is
in the PROTECTED set of clean-smoke-resources.ts, it is long-lived and failed
that assertion on every run.
Mirrors the production aws-docs probe (same package): search_documentation with
a phrase, 300s interval since the call leaves the cluster.
Verified live: with this applied, "every RUNNING server has a readiness probe
configured" passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wUmrfkVQR6CKcYKxENq7k
Same hazard as the package build, with the cluster on the receiving end: a
branch behind main builds images missing whatever landed there, and deploy-k8s.sh
pins that sha in Pulumi — making the stale build the cluster's source of truth.
build-mcpd.sh gets its own call because it is run standalone as well as from
deploy-k8s.sh, so neither can rely on the other having checked.
`--dry-run` is exempt. It builds and cuts over nothing, and blocking a read-only
inspection is exactly what teaches people to export MCPCTL_ALLOW_BEHIND_MAIN=1
permanently — which would disable the gate for the real deploys too.
The failure text is now artifact-agnostic ("produce an artifact" / "shipping
it"), since one helper now speaks for packages, images and deploys.
Verified against a synthetic ref one commit ahead: build-mcpd.sh exits 1 before
any docker work, and deploy-k8s.sh exits 1 before the test gate, the pg_dump,
the image build and pulumi. Neither the working tree nor HEAD was moved to test
this — the ref was built with git commit-tree and deleted afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wUmrfkVQR6CKcYKxENq7k
Everyone branches off main and builds from their own branch. A branch that is
behind main still builds and installs perfectly — it just packages a binary
missing whatever landed on main meanwhile, and `rpm -U --force` overwrites the
good one with it. Nothing reports an error; the release succeeds and the feature
simply vanishes from the installed CLI.
That is what happened today: a build from a stale checkout replaced
/usr/bin/mcpctl with one that has no `statusline` command at all, months after
the status line landed on main (`mcpctl statusline` -> "unknown command").
`check-main-sync.sh` fetches main, compares, and fails before any work happens,
listing the commits the branch is missing and the merge that fixes it. Sourced
by build-rpm.sh and build-deb.sh — both are run standalone, so neither can rely
on the other having checked.
A hard failure rather than a warning: a warning scrolls past in a build log, and
the whole point is to stop before the artifact exists.
MCPCTL_ALLOW_BEHIND_MAIN=1 is the escape hatch for a deliberate old-tree build;
MCPCTL_BASE_BRANCH retargets the comparison. Offline it degrades to the last
fetched origin/main, then a local main, saying which it used; outside a git
checkout it skips.
Verified: passes on this branch (up to date with main); against a synthetic ref
one commit ahead, build-rpm.sh aborts with exit 1 before ensure_build_deps and
before any compilation; the escape hatch bypasses it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wUmrfkVQR6CKcYKxENq7k
A freshly provisioned PVC mounts root:root, so any image that drops privileges
cannot write to it. docs-mcp-server runs as uid 1000 and died on first start
with SQLITE_CANTOPEN; its own Dockerfile says to chown the volume 1000:1000.
Pods with volumes now carry securityContext.fsGroup, defaulting to 1000 and
overridable per volume. fsGroupChangePolicy is OnRootMismatch so Kubernetes
does not walk and re-chown the whole volume on every start.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
@kubernetes/client-node v1 raises ApiException, which carries the HTTP status
on `.code`. Every check in the orchestrator read only `.statusCode` — the
pre-1.0 HttpError field — so `status` came back undefined and each "expected"
404/409 was rethrown instead of handled.
Found in production: the first start of a server with a volume read a
not-yet-existing PVC, got a 404 that should have meant "create it", and failed
the instance instead. The same latent bug sat in removeContainer, where a
missing pod failed the delete rather than being treated as already gone.
httpStatusOf() checks .code, .statusCode and .response.statusCode, and falls
back to parsing the rendered "HTTP-Code: NNN" message for wrappers that keep
none of them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
`mcpctl config claude --project X` writes user scope and never rewrites a
checkout's `.mcp.json`. The status line, however, preferred that file
unconditionally — so a legacy project-named entry an older mcpctl left behind
(`homeautomation` -> `mcpctl mcp -p homeautomation`) kept naming the old project
for good, and every switch looked like it had done nothing.
Reproduced live: with user scope on `sre`, `mcpctl statusline --directory
~/developer/michalzxc/claude/debug` printed `mcpctl:homeautomation` — a project
Claude Code also had in `disabledMcpServers` for that directory, so the line
named a server that was not even mounted.
Rank the sources by how deliberate each one is instead: a canonical `mcpctl`
pin, then user scope, then legacy residue, then the marker. A pin is a decision
and still wins; residue is not and no longer does. At every step, skip a server
Claude Code has switched off for that directory.
`config claude` now also warns when the working directory's `.mcp.json`
contradicts the switch, naming the file — the two scopes are merged rather than
chosen between, so nothing else would tell you.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wUmrfkVQR6CKcYKxENq7k
Closes 167. The five new resources fell through to the generic key-value dump,
against the project rule that describe shows clean sections and tables. More to
the point, two things built earlier had no way to reach a human at all: the
roadmap endpoint had no client, and the Epic reconciliation was only visible as
raw JSON.
`describe epic` puts the delivery mismatch directly under the completeness
percentage it contradicts, because reading either one alone is the bug. Rendered
against a live mcpd:
Completeness: [....................] 0% (0/3)
...
Delivery: MISMATCH between what is claimed and what shipped
Tasks: 1 merged, 0 outstanding, 0 cancelled
Delivered, not claimed:
- wheels (1 merged, 0 outstanding)
No roadmap node owns it:
- engine, feel
plus the criteria checklist, accepted tradeoffs, and the roadmap with per-node
completeness and READY flags.
`describe task` derives the gates a task still owes from its risk and recorded
verdicts ("awaiting security_review, sre_review" vs "all passed"), and shows the
legal next transitions, PR linkage, verdicts and full history — so the answer to
"why won't this move" is on screen rather than requiring a failed attempt.
`describe repo` / `describe gitprovider` show the credential REFERENCE only.
The resolved value exists solely on the session-provision path and must never
reach a describe.
The roadmap, verdict and event fetches are all best-effort: an older mcpd 404s
and the resource still renders, rather than the whole describe failing over an
enrichment.
One bug found by looking at the output rather than the types: the Git section
opened on `!== undefined`, but JSON hands back null, so every task without a PR
printed an empty `Git:` header.
Local: build clean, workspace 2557 passed, lint unchanged at 794 errors.
Verified live against a local mcpd for epic, epictask and task.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
