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
The extension failed to load outright on older pi installs:
Failed to load extension ".../mcpctl-pi.ts":
Cannot find module '@earendil-works/pi-ai'
pi doesn't resolve an extension's bare specifiers the ordinary way — it
hands jiti a hard-coded alias table built from its own dependencies, and
that table differs between pi distributions. `@earendil-works/pi-coding-
agent` (0.84.1) aliases both the `@earendil-works/*` and legacy
`@mariozechner/*` names; `@mariozechner/pi-coding-agent` (0.73.1) aliases
only the old ones. Neither resolves the other's namespace, so a single
import outside the intersection takes the whole extension down: every
tool, the /mcpctl command, and the status line, all gone.
The only thing we used from pi-ai was `StringEnum`, a six-line wrapper
over `Type.Unsafe`. Inlined as a local `stringEnum` with byte-identical
output, so `typebox` — aliased by every published pi — is now the sole
bare runtime import. The call site also passes `description` through,
which the pi-ai version was silently dropping.
Guarded in tests/config/pi-extension-embed.test.ts: any runtime import in
the embedded sources that isn't `node:`, relative, or typebox now fails.
Verified against both installs with the same active project: 0.73.1
reproduced the error verbatim before the change and loads cleanly after,
and 0.84.1 keeps registering the gate tool exactly as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014tsRTqhEC7YYYYaP3cBqo8
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
Closes 175. Epic.doneCriteria and EpicTask.doneCriteria are both JSON blobs
joined only by a free-form id, and nothing checked that a node's ids existed on
its parent Epic. A typo produced a criterion belonging to nobody.
The reconciliation added in 170 surfaces those as `unowned`, which is honest but
late: the mistake was made at write time and is far cheaper to refuse there. Now
create and update both reject unknown ids, naming every one of them and listing
what the Epic actually has — or saying plainly that the Epic has no criteria yet,
which is the likelier cause when a whole roadmap comes back empty.
Enforced in the service, not the schema: production provisions with
`prisma db push`, so a constraint would never exist where it matters.
A node with no criteria of its own is still fine — not every roadmap node claims
one — and an update that doesn't touch criteria doesn't pay for the check.
This deliberately stops short of the EpicCriterion table with a real FK. That
remains the stronger answer if criteria ever need to be addressed independently,
but it costs a schema change plus migrating everything currently in the JSON
blobs, and the invariant closes the actual failure mode today.
Also files 177: agent chat --project reaches project prompts and MCP tools while
checking only run:agents:<name>. Raised by the agent that built membership,
which correctly refused to invent the policy mid-task. Pre-existing behaviour for
a legacy pin, but membership widens the blast radius considerably — it should be
a deliberate decision with a test.
Local: build clean, workspace 2557 passed (+7), lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
`git add -A` swept .claude/worktrees/ into the previous commit as embedded git
repositories. They are transient checkouts created for parallel agent runs and
have no business in the tree — removed from the index and gitignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merges feat/agent-project-membership (task 166/152). An agent can finally join
many projects: membership routes, per-membership role and optional brain
override, chat resolving context from the membership, and CLI attach-agent /
detach-agent / get memberships. The legacy Agent.projectId fallback is kept for
this release as the schema comment promises.
Conflict resolved in main.ts: that branch added its RBAC case to
mapUrlToPermission inline, while this branch had extracted the function to
middleware/url-permission.ts so it could be tested. Kept the extraction and
ported the case into it — joining a team is `edit:agents:<name>`, not
`create:agents`, which is the authority to mint a whole agent.
Also fixes something the extraction makes possible. security.test.ts contained a
hand-copied reproduction of mapUrlToPermission, because main.ts self-invokes
main() and nothing in it was importable. A security test asserting against a
copy stops testing the system the moment the real one changes, and this copy had
already drifted. It now imports the real function; all 18 assertions still pass,
against the code that actually runs.
Local: build clean, workspace 2550 passed, lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
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
The AgentProject junction has existed since the data-model commit and nothing
read it. `grep -c agentProject src/mcpd/src/services/chat.service.ts` returned
0; chat resolved an agent's project from the legacy single `Agent.projectId`
scalar. So the schema claimed "an agent is a person who joins zero, one, or
many projects" (docs/agentic-teams.md §4) while the running system still gave
every agent exactly one project. A schema that lies about behaviour is worse
than a missing feature: it reads as done, so nobody builds it.
This makes the junction load-bearing.
Membership is now a real surface (task 152):
GET /api/v1/agents/:name/projects memberships
POST /api/v1/agents/:name/projects/:project join, body {role?, llm?}
DELETE /api/v1/agents/:name/projects/:project leave
GET /api/v1/projects/:name/memberships the roster, with roles
`role` is TEXT (developer|qa|security|sre in v1) and is validated in the
service layer, not the database — production provisions with `prisma db push`,
so a CHECK constraint written into a migration would never run there. Adding a
fifth role stays a one-line change.
RBAC: joining/leaving is `edit:agents:<name>`, not the `create:agents` the
default method→action mapping would have produced. Minting a whole new agent
and adding an existing one to a team are not the same authority; this mirrors
project↔server attach/detach already being `edit:projects`. Reading a roster is
`view:` on the side you asked from.
Chat honours it. `prepareContext` resolves the project from the membership when
the caller names one, and the resolution runs *before* the Llm lookup so a
per-membership `llmId` can replace the agent's pinned brain — and therefore its
whole failover pool — rather than patching the dispatch list afterwards. That
override is why the brain-ownership question (§15 Q1) can still go either way
without a migration: today the answer is per membership.
Naming a project the agent has not joined is an error that names the fix, not a
silent fallback. Answering from a *different* project's tools and prompts than
the one asked for is the failure mode worth being loud about.
The legacy scalar keeps working this release, as the schema comment promises:
`--project` accepts a membership *or* a legacy pin naming that same project,
bare `mcpctl chat <agent>` is unchanged, and `GET /projects/:p/agents` returns
the union of members and legacy-pinned agents so nothing disappears from a
project mid-migration (mcplocal's agents plugin reads that endpoint).
CLI: `mcpctl chat <agent> --project <p>` (previously rejected as "not both"),
plus `attach-agent` / `detach-agent` alongside the existing attach-server pair,
`get agents --project`, and `get memberships` with ROLE and BRAIN columns.
Completions come from the generator, not hand-edits.
Threads stay scoped to the agent, not to (agent, project) — §4 wants the
latter, but that needs a schema change and a story for what happens to those
threads when a membership is revoked. Deliberately not decided here.
Tests: 51 new (43 mcpd, 8 CLI). Full suite 2514 pass. Lint unchanged at
794 errors / 77 warnings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guard test caught it, which is its job — the generator derives flags from
the commander program, so any new option makes the committed files stale.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merges the Epic reconciliation (task 170) and closes the gap that made it inert
from the terminal.
`mcpctl create epictask` had no `--criterion` flag while `create epic` did, so a
roadmap built from the CLI had no criteria on its nodes. The reconciliation
attributes a criterion to a node by id, so every criterion would have reported
as `unowned` and both mismatch lists would always have been empty — the feature
would have looked like it worked and detected nothing.
Files two follow-ups the reconciliation surfaced:
- 175: criteria and roadmap nodes are linked only by a free-form id convention
that nothing enforces. EpicTaskService never validates ids against the parent
Epic, ids can repeat across nodes, and a node can carry an id the Epic never
had. The reconciliation reports those as `unowned` rather than guessing an
owner, which is right, but the convention should become an invariant before
more is built on it.
- 176: `get epic -o yaml` now emits computed `completeness`/`reconciliation`,
which are not settable. apply tolerates them, but the round-trip rule says get
output is apply input.
Local: build clean, workspace 2499 passed, lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
An Epic could report 0% while every Task under it was merged. That is not a
cosmetic gap — it is exactly the blind spot the Epic exists to remove
(docs/agentic-teams.md §1b: "we completed all the tasks and got a stick with
wheels instead of a car"). An Epic that cannot see its own delivered work
cannot detect that, because `completeness()` reads only `doneCriteria[].met`
and nothing flips those except a human hitting the criteria endpoint.
So read the same question a second time, from the delivery side, and report
where the two readings disagree — in both directions:
- deliveredButUnclaimed — unmet criteria whose owning roadmap node has
merged work. Somebody delivered and nobody ticked the box.
- claimedButUndelivered — criteria marked met with nothing merged
underneath. The box is ticked and nothing shipped. The dangerous
direction, and the one that is easy to forget to look for.
- unowned — criteria no roadmap node claims at all. They cannot appear in
either list above, so without a bucket of their own they would vanish
inside a clean-looking reconciliation.
Nothing is written. `met` stays exactly as a human or agent set it: deriving
it from task state would assert that finishing the tasks means delivering the
vision, which is the assumption under audit. The mismatch is reported, not
silently resolved.
Attribution is by criterion id, because that is the only link the schema
offers — EpicTask.doneCriteria is documented as "the decomposed subset of the
Epic's doneCriteria this node owns" and carries the same ids, but nothing
enforces it. Rather than guess an owner by name similarity or ordering, the
convention is treated as a claim and its failures are surfaced as `unowned`.
A foreign key from criterion to epictask would end the ambiguity; this reports
the ambiguity instead of hiding it.
Judgement calls, made explicit so they can be argued with:
- `merged` alone counts as delivered. `done` only leaves the developer's
loop and `awaiting_review` still needs a human; `reopened` means a merge
did not hold, so it counts as outstanding.
- `cancelled` is neither delivered nor outstanding. Counting it as owed
would leave a node permanently undelivered because someone abandoned a
task; counting it as delivered is a lie. It is reported on its own.
- a node with no tasks is undelivered, not exempt — a met criterion over an
undecomposed node is still a claim with nothing behind it. The empty task
rollup is what distinguishes it from "built but not merged".
- an epictask's own `status` is another claim, not evidence, so it is
carried for context and never counted as delivery.
Surfaced on GET /api/v1/epics/:id next to the completeness it contradicts —
a mismatch is only useful where the misleading percentage is already on
screen — and standalone at GET /api/v1/epics/:id/reconciliation for callers
that do not want a 200KB Epic body with it. Deliberately kept off the list
route, which would pay a query per roadmap node per Epic.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes 134/164. The five agentic-teams resources had RBAC slots but the
non-CRUD verbs had no cases, so they fell through to the method default and
every one of them read as `create:<resource>`. That is wrong in both
directions: it let anyone who may CREATE a task also ACCEPT one — the human gate
that fires a merge — and it refused a reviewer who may drive tasks but not
create them.
Corrected, following the verb the codebase already uses for "use this resource
to do work" (llms/infer, agents/chat, projects/chat):
- POST /tasks/:id/{transition,accept,reject,verdicts} → run:tasks:<name>
- {GET,POST} /gitproviders/:id/{repos,import,probe} → run:gitproviders:<name>.
Discovery calls OUT to the forge with its stored token, so holding
view:gitproviders should let you see that a forge is registered, not use its
credential.
- POST /epics/:id/{status,tradeoffs,criteria} → edit:epics:<name>. These rewrite
the vision record.
- GET /epics/:id/roadmap → view:epics:<name>
- {POST,DELETE} /epictasks/:id/edges → edit:epictasks:<name>
mapUrlToPermission moved from main.ts to middleware/url-permission.ts. main.ts
calls main() at module load, so nothing in it is importable from a test — an
authorization mapping that cannot be tested is an authorization mapping nobody
has checked, which is exactly how these verbs shipped mis-scoped.
15 tests now assert the mapping rather than the mere presence of the strings,
including that a bare create grant does NOT cover the gated verbs, that work
items and the inference queue never resolve to the same resource for any method,
and that the pre-existing mappings (llm infer, agent chat, operations, the
personalities/skills piggybacks) still hold.
Local: build clean, workspace 2478 passed (+15), lint back to the repo's
baseline 794 errors after making the five new match guards explicit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
The tasks/repos command groups added with the CLI work sit at the root of
`mcpctl --help`, but both only function inside a project context. Beyond the
clutter, it teaches the wrong data model: a task reads as a peer of servers and
secrets rather than a leaf of the epic tree.
Convention recorded as the `scope-commands-to-their-context` prompt in the
mcpctl-development project. Note `attach-server`/`detach-server` are already
hidden, so the pattern exists and was simply not applied consistently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`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
Three findings from a cross-branch review of the competing opencode
implementations, all of which are fair.
1. `readState` type-guards the parsed JSON now. A bare try/catch does not
cover it: `JSON.parse('null')` succeeds and returns null, so the catch never
fires and the next `state.project` throws a TypeError that takes the plugin
down. Verified the crash before fixing; a test pins the guard in the embedded
copies. Credit to the competing 'opencode-mine' branch, which had this right.
2. eslint now covers `src/opencode-ext/*.tsx`. The glob was `*.ts` only, so the
300-line TUI plugin — the largest file in the addon — was linted by nothing.
It was typechecked, which is why this went unnoticed. Confirmed the rules
actually fire on it rather than the file being silently skipped. The
'abhishek' branch was the only entry that got this right.
3. docs/opencode-extension.md overstated the security argument. "The token would
sit in a 0644 opencode.json" is not a point against a `type: local` stdio
bridge, which needs no token at all because it reads your own credentials.
That reason is a consequence of having picked the HTTP gateway, not a
justification for it. The docs now lead with the real reason — live
re-pointing without a restart — and state the trade honestly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVwuCjuMoA13gmzYEfcrNP
The audit found that none of the new resources were in BACKUP_KINDS, so
git-backup never captured them and a restore would lose every epic, roadmap
node, repo and forge registration. Closes 135/165.
Backed up: gitprovider, repo, epic, epictask — configuration and the vision
layer. NOT tasks: a work item carries in-flight state (lifecycle position,
verdicts, PR links) and restoring a stale one from git would resurrect work that
has since moved on. That is the same reasoning that keeps McpInstance out of the
backup, and there is a test asserting it stays out.
Two properties the tests pin, because both are the kind of thing that fails
silently:
- **Credentials serialize as a REFERENCE, never a value.** The backup repo is a
plaintext git checkout; emitting a resolved token would export every forge
credential to disk in cleartext. credSecretId/credSecretKey are internal
fields and become `credRef: {name, key}`; restore re-resolves against whatever
the secret backend holds now.
- **Relations serialize by name, not foreign key.** A restore into a different
database has different cuids, so a backed-up projectId would dangle.
Restore ordering follows dependencies: a forge before the repos that reference
it, an epic before its roadmap nodes.
Two deliberate refusals on the restore path: an Epic's `status` is never written
by a restore, so a stale backup cannot silently mark it delivered — that
terminal state is only reachable through setStatus and its rules; and a
GitProvider is not probed on restore, because the forge may legitimately be
unreachable while a backup replays and failing the whole restore for that is
worse than a row that needs a later `probe`.
The pre-existing APPLY_ORDER length guard caught this change, which is exactly
what it is for; updated 9 → 13 and tied it to BACKUP_KINDS.length so the two
cannot drift apart again.
Also marks 153 done — `mcpctl tasks accept/reject` shipped with the CLI work; the
generated graph had it blocked behind the git-provider merge, but the gate itself
does not need it.
Local: build clean, workspace 2463 passed (+6), lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
Found while auditing what this branch is missing: skills are serialized into the
backup repo and enqueued on every mutation, but `importResource` and
`deleteResource` in main.ts have no `skill` case — so a git restore silently
drops every skill. SkillService has had upsertByName/deleteByName the whole
time; the switch arms were simply never added. Pre-existing, not introduced
here.
Also records ten tasks (164-173) for gaps found by auditing the branch against
the codebase rather than against the plan:
- 164 RBAC enforcement tests. Five resources and a namespace split shipped with
no test proving the gating works. Also flags that the non-CRUD verbs
(transition/accept/reject, import/probe) currently fall through to the default
method mapping and are probably mis-scoped.
- 165 Backup coverage for the agentic-teams resources — same class of silent
data loss as the skill bug above, for five more kinds.
- 166 AgentProject is inert: chat still resolves context from the legacy
Agent.projectId scalar, so "an agent joins many projects" — a core idea of the
design — is not actually usable yet.
- 167 describe formatters (currently a generic key-value dump, against the
project rule) including a client for the roadmap endpoint, which has no caller.
- 168 Docs, including the breaking `mcpctl get tasks` rename that exists only in
a commit message today.
- 169 Smoke tests for the forge flow.
- 170 Epic completeness ignores delivered work: an Epic can read 0% while every
task under it is merged, which is precisely the blind spot the Epic exists to
remove.
- 171 `mcpctl chat mcpctl` has never been run end to end — the tool layer is
verified, the LLM-picks-a-tool loop and the mcplocal token hop are not.
- 172 Nothing on this branch is live; everything was verified against a local
mcpd on the test database.
- 173 Web UI + Pulumi kinds (cosmetic).
Local: build clean, workspace 2457 passed, lint unchanged at 794 errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
