Compare commits

..

11 Commits

Author SHA1 Message Date
Michal
bbd2195c64 test(mcplocal): prove chat SSE streams live through the proxy, not buffered
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m17s
CI/CD / test (pull_request) Successful in 1m27s
CI/CD / lint (pull_request) Successful in 2m55s
CI/CD / smoke (pull_request) Failing after 1m59s
CI/CD / build (pull_request) Successful in 4m33s
CI/CD / publish (pull_request) Has been skipped
`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
2026-08-10 22:27:21 +01:00
Michal
5a8185d7c9 fix(mcplocal): stop the 30s proxy timeout killing agent turns
`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
2026-08-10 22:19:56 +01:00
ae5a6203f8 Merge PR #108: fail the release when smoke tests fail
Some checks failed
CI/CD / lint (push) Successful in 1m12s
CI/CD / test (push) Successful in 1m26s
CI/CD / typecheck (push) Successful in 3m7s
CI/CD / smoke (push) Failing after 1m59s
CI/CD / build (push) Successful in 2m15s
CI/CD / publish (push) Has been skipped
2026-08-10 16:16:05 +00:00
Michal
822c1bb047 build: fail the release when smoke tests fail, and fix the SSE test that hung
Some checks failed
CI/CD / lint (pull_request) Successful in 1m11s
CI/CD / test (pull_request) Successful in 1m24s
CI/CD / typecheck (pull_request) Successful in 3m6s
CI/CD / smoke (pull_request) Failing after 1m57s
CI/CD / build (pull_request) Successful in 4m40s
CI/CD / publish (pull_request) Has been skipped
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
2026-08-10 17:15:40 +01:00
cd94e855aa Merge PR #107: smoke-aws-docs readiness probe
Some checks failed
CI/CD / typecheck (push) Successful in 1m18s
CI/CD / lint (push) Successful in 2m46s
CI/CD / test (push) Successful in 1m25s
CI/CD / smoke (push) Failing after 1m56s
CI/CD / build (push) Successful in 4m52s
CI/CD / publish (push) Has been skipped
2026-08-10 16:00:08 +00:00
Michal
b8cedd6262 test(smoke): give the smoke-aws-docs fixture a readiness probe
Some checks failed
CI/CD / lint (pull_request) Successful in 1m13s
CI/CD / typecheck (pull_request) Successful in 1m18s
CI/CD / test (pull_request) Successful in 1m26s
CI/CD / build (pull_request) Successful in 2m15s
CI/CD / smoke (pull_request) Failing after 3m23s
CI/CD / publish (pull_request) Has been skipped
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
2026-08-10 16:59:41 +01:00
13f1ff28eb Merge PR #106: statusline project resolution + stale-build gate
Some checks failed
CI/CD / lint (push) Successful in 1m14s
CI/CD / test (push) Successful in 1m25s
CI/CD / typecheck (push) Successful in 3m5s
CI/CD / build (push) Has been cancelled
CI/CD / publish (push) Has been cancelled
CI/CD / smoke (push) Has been cancelled
2026-08-10 15:52:50 +00:00
Michal
96e27c8716 build: extend the main-sync gate to the image build and the k8s deploy
Some checks failed
CI/CD / lint (pull_request) Successful in 1m15s
CI/CD / test (pull_request) Successful in 1m23s
CI/CD / typecheck (pull_request) Successful in 2m59s
CI/CD / smoke (pull_request) Failing after 1m57s
CI/CD / build (pull_request) Successful in 4m58s
CI/CD / publish (pull_request) Has been skipped
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
2026-08-10 16:41:26 +01:00
Michal
dd29f98f82 build: refuse to package from a branch that is behind main
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
2026-08-10 16:38:19 +01:00
Michal
2a7bba11ea fix(claude): stop pre-migration .mcp.json residue outranking a project switch
`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
2026-08-10 12:15:46 +01:00
2b87cfdbf1 Merge pull request 'fix(pi-ext): stop importing @earendil-works/pi-ai at runtime' (#105) from fix/pi-ext-module-resolution into main
Some checks failed
CI/CD / lint (push) Successful in 1m12s
CI/CD / test (push) Failing after 13m0s
CI/CD / typecheck (push) Failing after 13m49s
CI/CD / smoke (push) Has been cancelled
CI/CD / build (push) Has been cancelled
CI/CD / publish (push) Has been cancelled
2026-08-09 23:15:30 +00:00
23 changed files with 1206 additions and 67 deletions

View File

@@ -101,13 +101,48 @@ to open, and re-scoping it would silently change which skills sync into it.
⏵⏵ bypass permissions on · ← for agents ⏵⏵ bypass permissions on · ← for agents
``` ```
`mcpctl statusline` resolves the project from a directory-scoped `.mcp.json` `mcpctl statusline` reads the directory from the JSON Claude Code pipes in, so it
first (a repo that pinned itself wins), then the user-scope entry in follows `/cwd` rather than reporting wherever the binary was launched, and prints
`.claude.json`, then a `.mcpctl-project` marker up the tree so a checkout that is scoped but not yet **nothing** when no project is active — an empty status line beats one saying
wired still reports. It reads the directory from the JSON Claude Code pipes in, "none" on every unrelated repo.
so it follows `/cwd` rather than reporting wherever the binary was launched, and
prints **nothing** when no project is active — an empty status line beats one It then takes the project from the most deliberate source that names one:
saying "none" on every unrelated repo.
1. a canonical `mcpctl` entry in that directory's `.mcp.json` — a repo that
pinned itself wins, and it is the scope Claude Code itself prefers when both
define that server name;
2. the user-scope entry in `.claude.json` — what `config claude --project`
writes, so a switch takes effect everywhere it is not overridden;
3. a **legacy** project-named entry in `.mcp.json` (`homeautomation`,
`docmost`, …), left by an mcpctl older than the constant server name;
4. a `.mcpctl-project` marker up the tree, so a checkout that is scoped but not
yet wired still reports.
> **Legacy entries rank below user scope on purpose.** They used to outrank it,
> which made switching look broken: a user-scope switch never rewrites a
> checkout's `.mcp.json`, so the leftover kept naming the old project for good.
> A pin is a decision; residue is not.
A server Claude Code has switched off for that directory (`disabledMcpServers` /
`disabledMcpjsonServers`) is skipped at every step — a disabled server is not
mounted, so naming its project would be a lie. A `.mcp.json` server that is in
neither list is still awaiting its approval prompt and does count, since blanking
the status line on a fresh checkout is the more confusing failure.
### When a directory contradicts a switch
Claude Code merges the two scopes rather than picking one, so switching in user
scope cannot clean up what a directory declares. `config claude` says so rather
than reporting plain success:
```
Warning: /path/to/repo/.mcp.json still registers 'homeautomation' for this
directory — mounted alongside 'sre', not replaced by it.
Re-run with --scope project to retire it, or delete the entry by hand.
```
A canonical entry pinned to another project gets the stronger wording — it
*overrides* the switch in that directory rather than sitting beside it.
### It is never installed over yours ### It is never installed over yours

View File

@@ -884,6 +884,39 @@ All pushed to `mysources.co.uk/michal/` registry.
source .env && bash scripts/release.sh source .env && bash scripts/release.sh
``` ```
**The build refuses to run from a branch that is behind `main`.** Everyone
branches off main, so a stale branch still builds and installs cleanly — it just
ships a binary missing whatever landed on main meanwhile, and `rpm -U --force`
overwrites the good one with it. That happened on 2026-08-10: a build from a
stale checkout replaced `/usr/bin/mcpctl` with one that had no `statusline`
command, months after the status line landed. `scripts/check-main-sync.sh`
fetches `main`, compares, and fails before any work happens, listing the commits
you are missing. It gates every path that produces something others consume:
`build-rpm.sh`, `build-deb.sh`, `build-mcpd.sh` (each is also run standalone, so
none can rely on another having checked) and `deploy-k8s.sh` — where a stale
branch would pin its sha in Pulumi and make it the cluster's source of truth.
`deploy-k8s.sh --dry-run` skips the check: it builds and cuts over nothing, and
blocking a read-only inspection only teaches people to export the escape hatch
permanently, disabling the gate for real deploys too.
```bash
git merge main # the fix
MCPCTL_ALLOW_BEHIND_MAIN=1 bash scripts/release.sh # deliberate old-tree build
MCPCTL_BASE_BRANCH=release-2.x bash scripts/build-rpm.sh # compare to another branch
```
Offline it falls back to the last fetched `origin/main`, then to a local `main`,
and says which it used; outside a git checkout it skips entirely.
**A failing smoke run fails the release.** It used to print
`WARNING: Smoke tests failed!` and exit 0 — which is exactly how four broken
readiness probes shipped unnoticed (see `docs/reliability.md`): the warning
scrolled past and the release reported success. Note what the gate does and does
not do — smoke runs *last*, against the installed binary, so the package is
already published and installed by the time it fails. It reports the breakage
rather than preventing it, so investigate the fleet rather than assuming the
artifact is bad. Override with `MCPCTL_ALLOW_SMOKE_FAILURE=1`.
Installs via nfpm: Installs via nfpm:
- `/usr/bin/mcpctl` — CLI binary (bun compiled) - `/usr/bin/mcpctl` — CLI binary (bun compiled)
- `/usr/bin/mcpctl-local` — Local proxy binary (bun compiled) - `/usr/bin/mcpctl-local` — Local proxy binary (bun compiled)

View File

@@ -19,6 +19,11 @@ source "$SCRIPT_DIR/arch-helper.sh"
resolve_arch "${MCPCTL_TARGET_ARCH:-}" resolve_arch "${MCPCTL_TARGET_ARCH:-}"
# Sets: NFPM_ARCH, BUN_TARGET, ARCH_SUFFIX # Sets: NFPM_ARCH, BUN_TARGET, ARCH_SUFFIX
# Same guard as build-rpm.sh: this script is also run on its own, so it cannot
# rely on that one having checked.
source "$SCRIPT_DIR/check-main-sync.sh"
check_main_sync
# Check and install missing build dependencies # Check and install missing build dependencies
source "$SCRIPT_DIR/ensure-deps.sh" source "$SCRIPT_DIR/ensure-deps.sh"
ensure_build_deps ensure_build_deps

View File

@@ -16,6 +16,11 @@ if [ -f .env ]; then
set -a; source .env; set +a set -a; source .env; set +a
fi fi
# This pushes an image to the registry, so the same staleness gate as the package
# builds applies. Run standalone as well as from deploy-k8s.sh, hence its own copy.
source "$SCRIPT_DIR/check-main-sync.sh"
check_main_sync
# Push directly to internal address (external proxy has body size limit) # Push directly to internal address (external proxy has body size limit)
REGISTRY="10.0.0.194:3012" REGISTRY="10.0.0.194:3012"
IMAGE="mcpd" IMAGE="mcpd"

View File

@@ -19,6 +19,11 @@ source "$SCRIPT_DIR/arch-helper.sh"
resolve_arch "${MCPCTL_TARGET_ARCH:-}" resolve_arch "${MCPCTL_TARGET_ARCH:-}"
# Sets: NFPM_ARCH, BUN_TARGET, ARCH_SUFFIX # Sets: NFPM_ARCH, BUN_TARGET, ARCH_SUFFIX
# Before anything expensive: a branch behind main packages a binary missing
# whatever landed there, and installing it silently downgrades the machine.
source "$SCRIPT_DIR/check-main-sync.sh"
check_main_sync
# Check and install missing build dependencies # Check and install missing build dependencies
source "$SCRIPT_DIR/ensure-deps.sh" source "$SCRIPT_DIR/ensure-deps.sh"
ensure_build_deps ensure_build_deps

101
scripts/check-main-sync.sh Executable file
View File

@@ -0,0 +1,101 @@
#!/bin/bash
# Refuse to build a package from a branch that main has already moved past.
#
# WHY
#
# Everyone branches off main and builds from their own branch. A branch that is
# behind main still builds and installs perfectly — it just quietly ships a
# binary missing whatever landed on main in the meantime, and `rpm -U --force`
# overwrites the good one with it.
#
# That is not hypothetical: on 2026-08-10 a build from a stale checkout replaced
# /usr/bin/mcpctl with one that had no `statusline` command at all, months after
# the status line landed on main. Nothing reported an error — the release
# succeeded, the feature just vanished from the installed CLI.
#
# So this is a hard failure rather than a warning. A warning scrolls past in a
# build log; the whole point is to stop before the artifact exists.
#
# ESCAPE HATCH
#
# MCPCTL_ALLOW_BEHIND_MAIN=1 build anyway (deliberate build of an old tree)
# MCPCTL_BASE_BRANCH=<name> compare against something other than main
#
# Skips itself entirely outside a git checkout, so tarball builds still work.
# Resolve the ref to compare against, echoing it on stdout. Prefers a fresh
# fetch; falls back to whatever is already on disk so an offline build is
# degraded rather than blocked. Returns 1 when there is nothing to compare to.
_main_sync_ref() {
local base="$1" remote="$2"
if [ -n "$remote" ] && git fetch --quiet "$remote" "$base" 2>/dev/null; then
# FETCH_HEAD rather than refs/remotes/<remote>/<base>: it is what this fetch
# just wrote, so it cannot be a stale opportunistic update.
echo "FETCH_HEAD"
return 0
fi
if [ -n "$remote" ] && git rev-parse --verify --quiet "refs/remotes/$remote/$base" >/dev/null; then
echo " (could not reach $remote — comparing against the last fetched $remote/$base)" >&2
echo "refs/remotes/$remote/$base"
return 0
fi
if git rev-parse --verify --quiet "refs/heads/$base" >/dev/null; then
echo " (no reachable remote — comparing against local $base)" >&2
echo "refs/heads/$base"
return 0
fi
return 1
}
check_main_sync() {
local base="${MCPCTL_BASE_BRANCH:-main}"
if ! git rev-parse --git-dir >/dev/null 2>&1; then
return 0 # not a checkout; nothing to be behind
fi
if [ "${MCPCTL_ALLOW_BEHIND_MAIN:-}" = "1" ]; then
echo "==> Skipping the '$base' sync check (MCPCTL_ALLOW_BEHIND_MAIN=1)"
return 0
fi
echo "==> Checking this branch is not behind '$base'..."
local remote ref
remote="$(git remote | head -1)"
if ! ref="$(_main_sync_ref "$base" "$remote")"; then
echo " (no '$base' branch found anywhere — skipping)"
return 0
fi
local behind
behind="$(git rev-list --count "HEAD..$ref" 2>/dev/null || echo 0)"
if [ "$behind" -eq 0 ]; then
echo " up to date with $base"
return 0
fi
local branch
branch="$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo HEAD)"
echo "" >&2
echo "ERROR: '$branch' is $behind commit(s) behind $base — refusing to build." >&2
echo "" >&2
# Deliberately artifact-agnostic: the same helper gates RPM/DEB packages, the
# mcpd image, and the k8s deploy.
echo " Building now would produce an artifact without these, and shipping it" >&2
echo " would replace a good one with a version missing them:" >&2
echo "" >&2
git log --oneline --no-decorate "HEAD..$ref" | head -15 | sed 's/^/ /' >&2
if [ "$behind" -gt 15 ]; then
echo " … and $((behind - 15)) more" >&2
fi
echo "" >&2
echo " Fix it: git merge $base # or: git rebase $base" >&2
echo " Anyway: MCPCTL_ALLOW_BEHIND_MAIN=1 $0" >&2
echo "" >&2
return 1
}

View File

@@ -80,6 +80,20 @@ cat <<EOF
EOF EOF
[ -f "$PULUMI_YAML" ] || die "Pulumi config not found: $PULUMI_YAML" [ -f "$PULUMI_YAML" ] || die "Pulumi config not found: $PULUMI_YAML"
# ── 0. Staleness gate ──
# Same hazard as the RPM build, with the cluster on the receiving end: a branch
# behind main deploys images missing whatever landed there, and the sha pinned in
# Pulumi makes that the new source of truth. Skipped for --dry-run, which builds
# and cuts over nothing — blocking a read-only inspection only teaches people to
# export MCPCTL_ALLOW_BEHIND_MAIN=1 permanently, which would disable the gate for
# the real deploys too.
if [ "$DRY_RUN" = true ]; then
warn "dry-run: skip the main-sync check"
else
source "$SCRIPT_DIR/check-main-sync.sh"
check_main_sync || die "branch is behind main — merge it before deploying"
fi
# ── 1. Test gate ── # ── 1. Test gate ──
if [ "$SKIP_TESTS" = true ]; then warn "skipping unit tests (--skip-tests)"; else if [ "$SKIP_TESTS" = true ]; then warn "skipping unit tests (--skip-tests)"; else
say "1/7 Unit tests (pnpm test:run)" say "1/7 Unit tests (pnpm test:run)"

View File

@@ -75,9 +75,28 @@ echo "==> Running smoke tests..."
export PATH="$HOME/.npm-global/bin:$PATH" export PATH="$HOME/.npm-global/bin:$PATH"
if pnpm test:smoke; then if pnpm test:smoke; then
echo "==> Smoke tests passed!" echo "==> Smoke tests passed!"
elif [ "${MCPCTL_ALLOW_SMOKE_FAILURE:-}" = "1" ]; then
echo "==> WARNING: Smoke tests failed, continuing (MCPCTL_ALLOW_SMOKE_FAILURE=1)."
else else
echo "==> WARNING: Smoke tests failed! Check mcplocal/mcpd are running." # This used to print a warning and exit 0. That is how four broken readiness
echo " Continuing anyway — deployment is complete, but verify manually." # probes shipped unnoticed on 2026-08-10: the warning scrolled past in the
# build log and the release reported success. A failing smoke run means
# something in the live fleet is genuinely broken — say so in the exit code.
#
# Note what this does and does not do: smoke runs LAST, against the installed
# binary, so the package is already published and installed by now. Failing
# here reports the breakage, it does not prevent it — investigate, do not
# assume the artifact is bad.
echo "" >&2
echo "ERROR: smoke tests failed — the release is published and installed, but" >&2
echo " something in the live fleet is broken. Investigate before relying" >&2
echo " on this build; do not just re-run." >&2
echo "" >&2
echo " Common causes: mcplocal/mcpd not running, a readiness probe pointing at" >&2
echo " a tool the upstream renamed, or an expired credential." >&2
echo " Override: MCPCTL_ALLOW_SMOKE_FAILURE=1 $0" >&2
echo "" >&2
exit 1
fi fi
echo "" echo ""

View File

@@ -46,6 +46,8 @@ import {
mergeUserScopeServer, mergeUserScopeServer,
userScopeProject, userScopeProject,
activeProjectIn, activeProjectIn,
canonicalProjectIn,
legacyEntriesIn,
claudeJsonPath, claudeJsonPath,
type McpJson, type McpJson,
type ClaudeJson, type ClaudeJson,
@@ -104,11 +106,51 @@ function readMcpJson(path: string): McpJson | null {
} }
} }
/**
* Warnings about a `.mcp.json` in `dir` that contradicts a user-scope switch to
* `project`.
*
* Claude Code merges the two scopes rather than picking one, so a
* directory-scoped entry does not go away when you switch globally:
* - a canonical `mcpctl` entry shares the name, and project scope wins — the
* switch has no effect in this directory at all;
* - a legacy project-named entry has a *different* name, so it is simply
* mounted alongside and the old project keeps answering here.
* Either way the user is owed the file path, because nothing else will tell
* them. Exported for tests.
*/
export function shadowWarnings(dir: string, project: string | undefined): string[] {
if (project === undefined || project === '') return [];
const path = join(dir, '.mcp.json');
const parsed = readMcpJson(path);
if (parsed === null) return [];
const pinned = canonicalProjectIn(parsed);
if (pinned !== null && pinned !== project) {
return [
`Warning: ${path} pins '${MCPCTL_SERVER_NAME}' to '${pinned}' for this directory, which overrides the switch here.`,
` Re-run with --scope project to repoint it, or delete the '${MCPCTL_SERVER_NAME}' entry to follow the user-scope project.`,
];
}
const stale = legacyEntriesIn(parsed).filter((e) => e.project !== project);
if (stale.length > 0) {
const names = stale.map((e) => `'${e.server}'`).join(', ');
return [
`Warning: ${path} still registers ${names} for this directory — mounted alongside '${project}', not replaced by it.`,
` Re-run with --scope project to retire ${stale.length === 1 ? 'it' : 'them'}, or delete the ${stale.length === 1 ? 'entry' : 'entries'} by hand.`,
];
}
return [];
}
export interface ConfigCommandDeps { export interface ConfigCommandDeps {
configDeps: Partial<ConfigLoaderDeps>; configDeps: Partial<ConfigLoaderDeps>;
log: (...args: string[]) => void; log: (...args: string[]) => void;
/** API client for the skills sync side-effect of `config claude --project`. Optional so existing call sites work; without it we skip the sync step. */ /** API client for the skills sync side-effect of `config claude --project`. Optional so existing call sites work; without it we skip the sync step. */
apiClient?: ApiClient; apiClient?: ApiClient;
/** Working directory to check for a shadowing `.mcp.json`. Injectable so tests need not chdir. */
cwd?: () => string;
} }
export interface ConfigApiDeps { export interface ConfigApiDeps {
@@ -124,6 +166,7 @@ const defaultDeps: ConfigCommandDeps = {
export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?: ConfigApiDeps): Command { export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?: ConfigApiDeps): Command {
const { configDeps, log } = { ...defaultDeps, ...deps }; const { configDeps, log } = { ...defaultDeps, ...deps };
const cwd = deps?.cwd ?? ((): string => process.cwd());
// PR-5: api client used by `mcpctl config claude --project` to run the // PR-5: api client used by `mcpctl config claude --project` to run the
// initial skills sync after wiring the .mcp.json. Threaded through from // initial skills sync after wiring the .mcp.json. Threaded through from
// index.ts; falls back to apiDeps.client when not explicitly passed (the // index.ts; falls back to apiDeps.client when not explicitly passed (the
@@ -406,6 +449,11 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
if (userScope) { if (userScope) {
// The whole point of user scope: you do this once, not per checkout. // The whole point of user scope: you do this once, not per checkout.
log('This applies in every directory — no need to re-run it per repo.'); log('This applies in every directory — no need to re-run it per repo.');
// ...except where a directory-scoped entry contradicts it. That file
// is never rewritten by a user-scope switch, so staying silent is how
// a switch ends up looking like it did nothing: the status line keeps
// naming the old project, and its server keeps answering here.
for (const line of shadowWarnings(cwd(), opts.project)) log(line);
} }
// PR-5: write project marker, run initial skills sync, install // PR-5: write project marker, run initial skills sync, install

View File

@@ -2,7 +2,16 @@ import { Command } from 'commander';
import { readFileSync } from 'node:fs'; import { readFileSync } from 'node:fs';
import { join, resolve } from 'node:path'; import { join, resolve } from 'node:path';
import { homedir } from 'node:os'; import { homedir } from 'node:os';
import { activeProjectIn, claudeJsonPath, userScopeProject, type McpJson, type ClaudeJson } from '../config/claude-mcp.js'; import {
MCPCTL_SERVER_NAME,
canonicalProjectIn,
claudeJsonPath,
disabledServersFor,
legacyEntriesIn,
userScopeProject,
type McpJson,
type ClaudeJson,
} from '../config/claude-mcp.js';
import { findProjectMarker } from '../utils/project-marker.js'; import { findProjectMarker } from '../utils/project-marker.js';
/** /**
@@ -16,9 +25,10 @@ import { findProjectMarker } from '../utils/project-marker.js';
* `setStatus`. * `setStatus`.
* *
* Claude Code pipes a JSON blob in on stdin (session id, model, workspace). We * Claude Code pipes a JSON blob in on stdin (session id, model, workspace). We
* only need the directory the project is whatever `.mcp.json` there mounts, * only need the directory; the project is then resolved from the most
* falling back to a `.mcpctl-project` marker up the tree so a checkout that is * deliberate source that names one — see the ranking in the action below.
* scoped but not yet wired still reports. * Whatever it reports has to be a project that is genuinely mounted, so a
* server Claude Code has switched off for that directory is skipped.
* *
* Prints nothing at all when no project is active: an empty status line is * Prints nothing at all when no project is active: an empty status line is
* better than one that says "none" on every unrelated repo you open. * better than one that says "none" on every unrelated repo you open.
@@ -53,25 +63,47 @@ export function resolveDirectory(input: StatusLineInput, fallback: string): stri
return input.workspace?.current_dir ?? input.workspace?.project_dir ?? input.cwd ?? fallback; return input.workspace?.current_dir ?? input.workspace?.project_dir ?? input.cwd ?? fallback;
} }
/** The project Claude Code's user-scope config mounts, or null. */ /** Claude Code's user-scope config, or null if it is missing or unreadable. */
export function projectFromUserScope(path: string): string | null { export function readClaudeJson(path: string): ClaudeJson | null {
try { try {
return userScopeProject(JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson); return JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson;
} catch { } catch {
return null; return null;
} }
} }
/** The project `.mcp.json` in `dir` mounts, or null. */ /** The project Claude Code's user-scope config mounts, or null. */
export function projectFromMcpJson(dir: string): string | null { export function projectFromUserScope(doc: ClaudeJson | null): string | null {
return userScopeProject(doc);
}
/** The `.mcp.json` in `dir`, or null if there isn't a readable one. */
export function readDirMcpJson(dir: string): McpJson | null {
try { try {
const parsed = JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson; return JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson;
return activeProjectIn(parsed);
} catch { } catch {
return null; return null;
} }
} }
/**
* The project the canonical `mcpctl` entry in `dir`'s `.mcp.json` pins, or null
* — skipped when Claude Code has that server switched off for `dir`.
*/
export function projectFromDirPin(mcpJson: McpJson | null, disabled: Set<string>): string | null {
if (disabled.has(MCPCTL_SERVER_NAME)) return null;
return canonicalProjectIn(mcpJson);
}
/**
* The project a *legacy* project-named entry in `dir`'s `.mcp.json` mounts, or
* null. Disabled entries are skipped, so a leftover the user already turned off
* in `/mcp` stops being reported.
*/
export function projectFromDirLegacy(mcpJson: McpJson | null, disabled: Set<string>): string | null {
return legacyEntriesIn(mcpJson).find((e) => !disabled.has(e.server))?.project ?? null;
}
/** Format for the status line. Empty string means "render nothing". */ /** Format for the status line. Empty string means "render nothing". */
export function formatStatus(project: string | null, prefix: string): string { export function formatStatus(project: string | null, prefix: string): string {
return project !== null && project !== '' ? `${prefix}${project}` : ''; return project !== null && project !== '' ? `${prefix}${project}` : '';
@@ -96,12 +128,29 @@ export function createStatuslineCommand(deps?: Partial<StatuslineDeps>): Command
const input = opts.directory !== undefined ? {} : await readStdinJson(); const input = opts.directory !== undefined ? {} : await readStdinJson();
const dir = opts.directory !== undefined ? resolve(opts.directory) : resolveDirectory(input, cwd()); const dir = opts.directory !== undefined ? resolve(opts.directory) : resolveDirectory(input, cwd());
// Directory-scoped wiring wins: a repo with its own .mcp.json entry has const claudeJson = readClaudeJson(claudeJsonPath());
// deliberately pinned itself, and that beats the global default. const mcpJson = readDirMcpJson(dir);
let project = projectFromMcpJson(dir) ?? projectFromUserScope(claudeJsonPath()); const disabled = disabledServersFor(claudeJson, dir);
// Ranked by how deliberate each source is, because a switch has to be
// able to win:
// 1. a canonical `mcpctl` entry in this directory's .mcp.json — a
// deliberate pin, and the scope Claude Code itself prefers when both
// define the same server name;
// 2. user scope — what `config claude --project` writes, so switching
// projects must beat anything less deliberate than a pin;
// 3. a *legacy* project-named entry in .mcp.json. This used to outrank
// user scope, which made a switch look like it had done nothing: the
// residue an older mcpctl left in a checkout is not a pin, and never
// gets rewritten by a user-scope switch, so it reported the old
// project forever;
// 4. the .mcpctl-project marker — the other thing `config claude`
// writes, and what skills sync already trusts.
let project =
projectFromDirPin(mcpJson, disabled)
?? projectFromUserScope(claudeJson)
?? projectFromDirLegacy(mcpJson, disabled);
if (project === null) { if (project === null) {
// Not wired here (or wired above this directory) — the marker is the
// other thing `config claude` writes, and skills sync already trusts it.
const marker = await findProjectMarker(dir, homeDir()).catch(() => null); const marker = await findProjectMarker(dir, homeDir()).catch(() => null);
project = marker?.project ?? null; project = marker?.project ?? null;
} }

View File

@@ -100,16 +100,30 @@ export function isLegacyMcpctlEntry(name: string, entry: unknown): boolean {
return projectOfEntry(entry) === name; return projectOfEntry(entry) === name;
} }
/** The project the canonical `mcpctl` entry mounts, or null if there isn't one. */
export function canonicalProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null {
return projectOfEntry(config?.mcpServers?.[MCPCTL_SERVER_NAME]);
}
/**
* Legacy project-named entries still present, in file order.
*
* Kept separate from the canonical entry because the two mean different things
* to a reader: the canonical entry is a deliberate pin, a legacy entry is
* residue from an older mcpctl that nothing has cleaned up yet. Callers that
* rank sources (the status line) must be able to tell them apart.
*/
export function legacyEntriesIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): { server: string; project: string }[] {
const servers = config?.mcpServers;
if (!servers) return [];
return Object.entries(servers)
.filter(([name, entry]) => isLegacyMcpctlEntry(name, entry))
.map(([name]) => ({ server: name, project: name }));
}
/** The project currently mounted by `.mcp.json`, preferring the canonical entry. */ /** The project currently mounted by `.mcp.json`, preferring the canonical entry. */
export function activeProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null { export function activeProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null {
const servers = config?.mcpServers; return canonicalProjectIn(config) ?? legacyEntriesIn(config)[0]?.project ?? null;
if (!servers) return null;
const canonical = projectOfEntry(servers[MCPCTL_SERVER_NAME]);
if (canonical !== null) return canonical;
for (const [name, entry] of Object.entries(servers)) {
if (isLegacyMcpctlEntry(name, entry)) return name;
}
return null;
} }
export interface MergeResult { export interface MergeResult {
@@ -169,12 +183,39 @@ export function claudeJsonPath(env: NodeJS.ProcessEnv = process.env, homeDir?: s
: join(home, '.claude.json'); : join(home, '.claude.json');
} }
/** Per-directory state Claude Code keeps in `.claude.json`'s `projects` map. */
export interface ClaudeProjectEntry {
/** Servers switched off for this directory, whatever scope they came from. */
disabledMcpServers?: string[];
/** `.mcp.json` servers declined at the approval prompt. */
disabledMcpjsonServers?: string[];
[key: string]: unknown;
}
/** Shape of the bits of `.claude.json` we touch. Everything else is preserved. */ /** Shape of the bits of `.claude.json` we touch. Everything else is preserved. */
export interface ClaudeJson { export interface ClaudeJson {
mcpServers?: Record<string, McpServerEntry>; mcpServers?: Record<string, McpServerEntry>;
projects?: Record<string, ClaudeProjectEntry>;
[key: string]: unknown; [key: string]: unknown;
} }
/**
* Server names Claude Code has switched off in `dir`.
*
* A disabled server is not mounted, so naming its project as "active" is a
* plain lie — this is what lets the status line skip one. Only *explicit*
* disables count: a `.mcp.json` server in neither list is pending its approval
* prompt, and treating pending as off would blank the status line on a fresh
* checkout, which is the more confusing failure.
*/
export function disabledServersFor(doc: ClaudeJson | null | undefined, dir: string): Set<string> {
const entry = doc?.projects?.[dir];
return new Set([
...(Array.isArray(entry?.disabledMcpServers) ? entry.disabledMcpServers : []),
...(Array.isArray(entry?.disabledMcpjsonServers) ? entry.disabledMcpjsonServers : []),
]);
}
/** /**
* Set the user-scope entry, returning the new document and any legacy * Set the user-scope entry, returning the new document and any legacy
* project-named entries retired from it. * project-named entries retired from it.

View File

@@ -359,4 +359,46 @@ describe('config claude — user scope', () => {
process.exitCode = prevExit; process.exitCode = prevExit;
expect(output.join('\n')).toContain("unknown --scope 'global'"); expect(output.join('\n')).toContain("unknown --scope 'global'");
}); });
// A user-scope switch never rewrites a directory's .mcp.json, so anything of
// ours left in one keeps answering in that directory. Saying so is the only
// way the user finds out — the switch otherwise reports plain success.
describe('warns when the working directory contradicts the switch', () => {
const switchTo = async (project: string): Promise<string> => {
await createConfigCommand({ configDeps: {}, log, cwd: () => tmpDir })
.parseAsync(['claude', '--project', project, '--skip-skills', '--skip-ui'], { from: 'user' });
return output.join('\n');
};
it('names a legacy entry that stays mounted alongside the new project', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: { homeautomation: { command: 'mcpctl', args: ['mcp', '-p', 'homeautomation'] } },
}));
const out = await switchTo('sre');
expect(out).toContain(join(tmpDir, '.mcp.json'));
expect(out).toContain("'homeautomation'");
expect(out).toContain('mounted alongside');
});
it('says a canonical pin overrides the switch in that directory', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: { mcpctl: { command: 'mcpctl', args: ['mcp', '-p', 'docmost'] } },
}));
expect(await switchTo('sre')).toContain('overrides the switch here');
});
it('stays quiet when the directory already agrees, or wires nothing of ours', async () => {
writeFileSync(join(tmpDir, '.mcp.json'), JSON.stringify({
mcpServers: {
mcpctl: { command: 'mcpctl', args: ['mcp', '-p', 'sre'] },
'their-server': { command: 'docker', args: ['run', 'x'] },
},
}));
expect(await switchTo('sre')).not.toContain('Warning:');
});
it('stays quiet when there is no .mcp.json at all', async () => {
expect(await switchTo('sre')).not.toContain('Warning:');
});
});
}); });

View File

@@ -0,0 +1,121 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { createStatuslineCommand } from '../../src/commands/statusline.js';
/**
* The status line is what tells you which project you are in, so the property
* under test throughout is: after a switch, does it name the project you
* switched to?
*
* These drive the real command rather than the resolution helpers, because the
* bug they cover was in the *ranking* of sources, not in any one source.
*/
const bridge = (project: string): Record<string, unknown> => ({
command: 'mcpctl',
args: ['mcp', '-p', project],
});
let home: string;
let dir: string;
/** Claude Code's user-scope config, at the path `claudeJsonPath()` resolves. */
function writeClaudeJson(doc: unknown): void {
writeFileSync(join(home, '.claude.json'), JSON.stringify(doc));
}
function writeMcpJson(doc: unknown): void {
writeFileSync(join(dir, '.mcp.json'), JSON.stringify(doc));
}
/** Run `statusline` for `dir` and return exactly what it printed. */
async function statusline(): Promise<string> {
const out: string[] = [];
const cmd = createStatuslineCommand({ log: (l) => out.push(l), cwd: () => dir, homeDir: () => home });
await cmd.parseAsync(['--directory', dir], { from: 'user' });
return out.join('');
}
beforeEach(() => {
home = mkdtempSync(join(tmpdir(), 'mcpctl-statusline-home-'));
dir = mkdtempSync(join(tmpdir(), 'mcpctl-statusline-dir-'));
// Point claudeJsonPath() at the fake home; the CLI reads $CLAUDE_CONFIG_DIR
// first, which keeps this off the developer's real ~/.claude.json.
process.env['CLAUDE_CONFIG_DIR'] = home;
});
afterEach(() => {
delete process.env['CLAUDE_CONFIG_DIR'];
rmSync(home, { recursive: true, force: true });
rmSync(dir, { recursive: true, force: true });
});
describe('mcpctl statusline', () => {
it('reports the user-scope project when the directory wires nothing', async () => {
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('prints nothing at all when no project is active', async () => {
writeClaudeJson({ mcpServers: {} });
expect(await statusline()).toBe('');
});
it('lets a canonical .mcp.json pin override the user-scope project', async () => {
// Same server name in both scopes: Claude Code prefers project scope, so a
// deliberate pin is genuinely what is mounted here.
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
writeMcpJson({ mcpServers: { mcpctl: bridge('docmost') } });
expect(await statusline()).toBe('mcpctl:docmost');
});
it('does not let a legacy project-named entry outrank a user-scope switch', async () => {
// The regression: an older mcpctl wrote `homeautomation` into a checkout,
// and a user-scope switch never rewrites that file — so the status line
// reported the old project forever and the switch looked like a no-op.
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('still reports a legacy entry when nothing more deliberate names a project', async () => {
writeClaudeJson({ mcpServers: {} });
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('mcpctl:homeautomation');
});
it('skips a directory server Claude Code has switched off', async () => {
// A disabled server is not mounted, so naming its project is a lie.
writeClaudeJson({
mcpServers: {},
projects: { [dir]: { disabledMcpServers: ['homeautomation'] } },
});
writeMcpJson({ mcpServers: { homeautomation: bridge('homeautomation') } });
expect(await statusline()).toBe('');
});
it('skips a disabled pin and falls through to the user-scope project', async () => {
writeClaudeJson({
mcpServers: { mcpctl: bridge('sre') },
projects: { [dir]: { disabledMcpjsonServers: ['mcpctl'] } },
});
writeMcpJson({ mcpServers: { mcpctl: bridge('docmost') } });
expect(await statusline()).toBe('mcpctl:sre');
});
it('falls back to a .mcpctl-project marker when nothing is wired', async () => {
writeClaudeJson({ mcpServers: {} });
writeFileSync(join(dir, '.mcpctl-project'), 'lab\n');
expect(await statusline()).toBe('mcpctl:lab');
});
it('honours a custom prefix', async () => {
writeClaudeJson({ mcpServers: { mcpctl: bridge('sre') } });
const out: string[] = [];
const cmd = createStatuslineCommand({ log: (l) => out.push(l), cwd: () => dir, homeDir: () => home });
await cmd.parseAsync(['--directory', dir, '--prefix', 'proj '], { from: 'user' });
expect(out.join('')).toBe('proj sre');
});
});

View File

@@ -5,6 +5,9 @@ import {
projectOfEntry, projectOfEntry,
isLegacyMcpctlEntry, isLegacyMcpctlEntry,
activeProjectIn, activeProjectIn,
canonicalProjectIn,
legacyEntriesIn,
disabledServersFor,
} from '../../src/config/claude-mcp.js'; } from '../../src/config/claude-mcp.js';
const bridge = (project: string): Record<string, unknown> => ({ const bridge = (project: string): Record<string, unknown> => ({
@@ -62,6 +65,50 @@ describe('activeProjectIn', () => {
}); });
}); });
describe('canonicalProjectIn / legacyEntriesIn', () => {
it('tells a deliberate pin apart from pre-migration residue', () => {
const config = { mcpServers: { [MCPCTL_SERVER_NAME]: bridge('sre'), homeautomation: bridge('homeautomation') } };
expect(canonicalProjectIn(config)).toBe('sre');
expect(legacyEntriesIn(config)).toEqual([{ server: 'homeautomation', project: 'homeautomation' }]);
});
it('reports no canonical entry when only legacy ones are present', () => {
const config = { mcpServers: { docmost: bridge('docmost') } };
expect(canonicalProjectIn(config)).toBeNull();
expect(legacyEntriesIn(config)).toEqual([{ server: 'docmost', project: 'docmost' }]);
});
it('leaves servers that are not ours out of both', () => {
const config = { mcpServers: { other: { command: 'echo' } } };
expect(canonicalProjectIn(config)).toBeNull();
expect(legacyEntriesIn(config)).toEqual([]);
expect(legacyEntriesIn(null)).toEqual([]);
});
});
describe('disabledServersFor', () => {
const doc = {
projects: {
'/repo': { disabledMcpServers: ['homeautomation'], disabledMcpjsonServers: ['mcpctl'] },
'/other': { disabledMcpServers: ['sre'] },
},
};
it('unions both of Claude Code\'s disable lists for that directory', () => {
expect([...disabledServersFor(doc, '/repo')].sort()).toEqual(['homeautomation', 'mcpctl']);
});
it('is scoped to the directory asked about', () => {
expect([...disabledServersFor(doc, '/other')]).toEqual(['sre']);
expect([...disabledServersFor(doc, '/unknown')]).toEqual([]);
expect([...disabledServersFor(null, '/repo')]).toEqual([]);
});
it('survives a malformed entry rather than throwing on the status line', () => {
expect([...disabledServersFor({ projects: { '/repo': { disabledMcpServers: 'nope' } } }, '/repo')]).toEqual([]);
});
});
describe('mergeMcpctlServers', () => { describe('mergeMcpctlServers', () => {
it('writes one constant entry regardless of project', () => { it('writes one constant entry regardless of project', () => {
const { config } = mergeMcpctlServers(null, { project: 'my-fancy-project' }); const { config } = mergeMcpctlServers(null, { project: 'my-fancy-project' });

View File

@@ -2,7 +2,7 @@ export { createHttpServer } from './server.js';
export type { HttpServerDeps } from './server.js'; export type { HttpServerDeps } from './server.js';
export { loadHttpConfig } from './config.js'; export { loadHttpConfig } from './config.js';
export type { HttpConfig } from './config.js'; export type { HttpConfig } from './config.js';
export { McpdClient, AuthenticationError, ConnectionError } from './mcpd-client.js'; export { McpdClient, AuthenticationError, ConnectionError, UpstreamTimeoutError } from './mcpd-client.js';
export { registerProxyRoutes } from './routes/proxy.js'; export { registerProxyRoutes } from './routes/proxy.js';
export { registerMcpEndpoint } from './mcp-endpoint.js'; export { registerMcpEndpoint } from './mcp-endpoint.js';
export { registerProjectMcpEndpoint } from './project-mcp-endpoint.js'; export { registerProjectMcpEndpoint } from './project-mcp-endpoint.js';

View File

@@ -20,9 +20,41 @@ export class ConnectionError extends Error {
} }
} }
/**
* Thrown when mcpd was reachable but did not finish in time.
*
* Deliberately NOT a ConnectionError. Folding timeouts into "cannot connect"
* is what made this class of failure so expensive to diagnose: mcpd answered
* /healthz in 32ms while the proxy insisted the daemon was down. A timeout and
* an unreachable daemon need different messages and different status codes.
*/
export class UpstreamTimeoutError extends Error {
constructor(readonly url: string, readonly timeoutMs: number) {
super(`mcpd did not respond within ${String(timeoutMs)}ms: ${url}`);
this.name = 'UpstreamTimeoutError';
}
}
/** True when `err` is an AbortSignal.timeout() firing. */
function isTimeout(err: unknown): boolean {
return err instanceof DOMException && err.name === 'TimeoutError';
}
/** Default timeout for mcpd requests (ms). Prevents indefinite hangs on slow upstream tool calls. */ /** Default timeout for mcpd requests (ms). Prevents indefinite hangs on slow upstream tool calls. */
export const DEFAULT_TIMEOUT_MS = 30_000; export const DEFAULT_TIMEOUT_MS = 30_000;
/**
* Budget for routes that are *expected* to run long: agent/project chat and
* raw inference. An agent turn is a multi-turn tool-use loop and legitimately
* runs for minutes, so the 30s default is not a safety net there — it is a
* guaranteed failure. Matches `STREAM_TIMEOUT_MS` in the CLI's chat command
* (src/cli/src/commands/chat.ts), which already allowed 10 minutes; mcplocal
* sitting in the middle with 30s was the binding constraint.
*
* Override with `MCPLOCAL_LONG_TIMEOUT_MS`.
*/
export const LONG_RUNNING_TIMEOUT_MS = Number(process.env['MCPLOCAL_LONG_TIMEOUT_MS']) || 600_000;
/** /**
* Discovery-class operations (tools/list, resources/list, prompts/list) should not share * Discovery-class operations (tools/list, resources/list, prompts/list) should not share
* the full tool-call timeout budget — a single dead upstream would stall session init for * the full tool-call timeout budget — a single dead upstream would stall session init for
@@ -121,9 +153,7 @@ export class McpdClient {
try { try {
res = await fetch(url, init); res = await fetch(url, init);
} catch (err: unknown) { } catch (err: unknown) {
if (err instanceof DOMException && err.name === 'TimeoutError') { if (isTimeout(err)) throw new UpstreamTimeoutError(this.baseUrl, this.timeoutMs);
throw new ConnectionError(this.baseUrl, new Error(`Request timed out after ${this.timeoutMs}ms`));
}
throw new ConnectionError(this.baseUrl, err); throw new ConnectionError(this.baseUrl, err);
} }
@@ -131,7 +161,18 @@ export class McpdClient {
throw new AuthenticationError(); throw new AuthenticationError();
} }
const text = await res.text(); // The body read MUST be inside a try. mcpd writes SSE headers immediately
// on chat routes, so fetch() resolves long before the turn finishes and the
// abort lands here instead — previously escaping as a raw DOMException and
// surfacing to the user as an opaque `500 code:23`.
let text: string;
try {
text = await res.text();
} catch (err: unknown) {
if (isTimeout(err)) throw new UpstreamTimeoutError(this.baseUrl, this.timeoutMs);
throw new ConnectionError(this.baseUrl, err);
}
let parsed: unknown; let parsed: unknown;
try { try {
parsed = JSON.parse(text); parsed = JSON.parse(text);
@@ -142,6 +183,51 @@ export class McpdClient {
return { status: res.status, body: parsed }; return { status: res.status, body: parsed };
} }
/**
* Forward a request and hand back the raw Response, body unread.
*
* `forward()` buffers through `res.text()`, which is fine for CRUD but
* defeats streaming entirely: an SSE chat arrives at the client as one blob
* after the turn ends, so the token-by-token output the CLI draws never
* appears. Streaming routes use this instead and pipe the body straight
* through.
*/
async forwardStream(
method: string,
path: string,
query: string,
body: unknown | undefined,
authOverride?: string,
): Promise<Response> {
const url = `${this.baseUrl}${path}${query ? `?${query}` : ''}`;
const headers: Record<string, string> = {
...this.extraHeaders,
'Authorization': `Bearer ${authOverride ?? this.token}`,
// Accept both: mcpd picks SSE or JSON based on the request's `stream` flag.
'Accept': 'text/event-stream, application/json',
};
const init: RequestInit = {
method,
headers,
signal: AbortSignal.timeout(this.timeoutMs),
};
if (body !== undefined && body !== null && method !== 'GET' && method !== 'HEAD') {
headers['Content-Type'] = 'application/json';
init.body = JSON.stringify(body);
}
try {
const res = await fetch(url, init);
if (res.status === 401) throw new AuthenticationError();
return res;
} catch (err: unknown) {
if (err instanceof AuthenticationError) throw err;
if (isTimeout(err)) throw new UpstreamTimeoutError(this.baseUrl, this.timeoutMs);
throw new ConnectionError(this.baseUrl, err);
}
}
private async request<T>(method: string, path: string, body?: unknown): Promise<T> { private async request<T>(method: string, path: string, body?: unknown): Promise<T> {
const result = await this.forward(method, path, '', body); const result = await this.forward(method, path, '', body);

View File

@@ -1,10 +1,62 @@
/** /**
* Catch-all proxy route that forwards /api/v1/* requests to mcpd. * Catch-all proxy route that forwards /api/v1/* requests to mcpd.
*/ */
import type { FastifyInstance } from 'fastify'; import { Readable } from 'node:stream';
import { AuthenticationError, ConnectionError } from '../mcpd-client.js';
import type { FastifyInstance, FastifyReply } from 'fastify';
import { AuthenticationError, ConnectionError, UpstreamTimeoutError, LONG_RUNNING_TIMEOUT_MS } from '../mcpd-client.js';
import type { McpdClient } from '../mcpd-client.js'; import type { McpdClient } from '../mcpd-client.js';
/**
* Routes that are expected to run long and/or stream.
*
* An agent turn is a multi-turn tool-use loop — minutes, not seconds — so the
* 30s default budget guarantees failure rather than guarding against it. These
* also stream SSE, which must be piped rather than buffered or the client sees
* one blob at the end instead of live output.
*/
const LONG_RUNNING = [
/^\/api\/v1\/agents\/[^/]+\/chat\b/,
/^\/api\/v1\/projects\/[^/]+\/chat\b/,
/^\/api\/v1\/llms\/[^/]+\/infer\b/,
/^\/api\/v1\/inference-tasks\/[^/]+\/stream\b/,
];
function isLongRunning(path: string): boolean {
return LONG_RUNNING.some((re) => re.test(path));
}
/** Headers worth preserving from mcpd; everything else is re-derived by Fastify. */
const PASSTHROUGH_HEADERS = ['content-type', 'cache-control', 'x-accel-buffering'];
function sendUpstreamError(reply: FastifyReply, err: unknown): FastifyReply | undefined {
if (err instanceof AuthenticationError) {
return reply.code(401).send({
error: 'unauthorized',
message: 'Authentication with mcpd failed. Run `mcpctl login` to refresh your token.',
});
}
if (err instanceof UpstreamTimeoutError) {
// 504, not 503 — mcpd was reachable, it just did not finish. Reporting this
// as "cannot reach mcpd" sent a previous debugging session chasing a
// network fault while /healthz answered in 32ms.
return reply.code(504).send({
error: 'upstream_timeout',
message:
`mcpd did not respond within ${String(err.timeoutMs)}ms. The daemon is reachable — the ` +
'request itself ran long. Raise MCPLOCAL_LONG_TIMEOUT_MS if this is a legitimately slow turn.',
});
}
if (err instanceof ConnectionError) {
return reply.code(503).send({
error: 'service_unavailable',
message: 'Cannot reach mcpd daemon. Is it running?',
});
}
return undefined;
}
export function registerProxyRoutes(app: FastifyInstance, client: McpdClient): void { export function registerProxyRoutes(app: FastifyInstance, client: McpdClient): void {
app.all('/api/v1/*', async (request, reply) => { app.all('/api/v1/*', async (request, reply) => {
const path = (request.url.split('?')[0]) ?? '/'; const path = (request.url.split('?')[0]) ?? '/';
@@ -19,25 +71,78 @@ export function registerProxyRoutes(app: FastifyInstance, client: McpdClient): v
// Forward the user's auth token to mcpd so RBAC applies per-user. // Forward the user's auth token to mcpd so RBAC applies per-user.
// If no user token is present, mcpd will use its auth hook to reject. // If no user token is present, mcpd will use its auth hook to reject.
const authHeader = request.headers['authorization'] as string | undefined; const authHeader = request.headers['authorization'] as string | undefined;
const userToken = authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : undefined; const userToken = authHeader !== undefined && authHeader.startsWith('Bearer ')
? authHeader.slice(7)
: undefined;
if (isLongRunning(path)) {
return proxyStreaming(reply, client, request.method, path, querystring, body, userToken);
}
try { try {
const result = await client.forward(request.method, path, querystring, body, userToken); const result = await client.forward(request.method, path, querystring, body, userToken);
return reply.code(result.status).send(result.body); return reply.code(result.status).send(result.body);
} catch (err: unknown) { } catch (err: unknown) {
if (err instanceof AuthenticationError) { const handled = sendUpstreamError(reply, err);
return reply.code(401).send({ if (handled) return handled;
error: 'unauthorized',
message: 'Authentication with mcpd failed. Run `mcpctl login` to refresh your token.',
});
}
if (err instanceof ConnectionError) {
return reply.code(503).send({
error: 'service_unavailable',
message: 'Cannot reach mcpd daemon. Is it running?',
});
}
throw err; throw err;
} }
}); });
} }
/**
* Pipe a long-running response straight through, headers and all.
*
* Hijacks the reply so Fastify does not try to serialize a stream, then copies
* mcpd's status and content-type before piping. `x-accel-buffering` matters:
* mcpd sets it to `no` so intermediaries don't buffer SSE, and dropping it here
* would reintroduce the exact stall we are fixing.
*/
async function proxyStreaming(
reply: FastifyReply,
client: McpdClient,
method: string,
path: string,
querystring: string,
body: unknown,
userToken: string | undefined,
): Promise<void> {
const longClient = client.withTimeout(LONG_RUNNING_TIMEOUT_MS);
let res: Response;
try {
res = await longClient.forwardStream(method, path, querystring, body, userToken);
} catch (err: unknown) {
const handled = sendUpstreamError(reply, err);
if (handled) return;
throw err;
}
const headers: Record<string, string> = {};
for (const name of PASSTHROUGH_HEADERS) {
const value = res.headers.get(name);
if (value !== null) headers[name] = value;
}
reply.hijack();
reply.raw.writeHead(res.status, headers);
if (res.body === null) {
reply.raw.end();
return;
}
try {
// Node's Readable.fromWeb bridges the fetch ReadableStream onto the socket.
await new Promise<void>((resolve, reject) => {
const upstream = Readable.fromWeb(res.body as Parameters<typeof Readable.fromWeb>[0]);
upstream.on('error', reject);
reply.raw.on('close', () => { upstream.destroy(); resolve(); });
upstream.pipe(reply.raw).on('finish', resolve).on('error', reject);
});
} catch {
// Headers are already on the wire, so there is no status left to change.
// Close the socket; the client surfaces the truncated stream.
if (!reply.raw.writableEnded) reply.raw.end();
}
}

View File

@@ -11,7 +11,7 @@ export type { MainResult } from './main.js';
export { ProviderRegistry } from './providers/index.js'; export { ProviderRegistry } from './providers/index.js';
export type { LlmProvider, CompletionOptions, CompletionResult, ChatMessage } from './providers/index.js'; export type { LlmProvider, CompletionOptions, CompletionResult, ChatMessage } from './providers/index.js';
export { OpenAiProvider, AnthropicProvider, OllamaProvider, GeminiCliProvider, DeepSeekProvider } from './providers/index.js'; export { OpenAiProvider, AnthropicProvider, OllamaProvider, GeminiCliProvider, DeepSeekProvider } from './providers/index.js';
export { createHttpServer, loadHttpConfig, McpdClient, AuthenticationError, ConnectionError, registerProxyRoutes } from './http/index.js'; export { createHttpServer, loadHttpConfig, McpdClient, AuthenticationError, ConnectionError, UpstreamTimeoutError, registerProxyRoutes } from './http/index.js';
export type { HttpConfig, HttpServerDeps } from './http/index.js'; export type { HttpConfig, HttpServerDeps } from './http/index.js';
export type { export type {
JsonRpcRequest, JsonRpcRequest,

View File

@@ -1,6 +1,6 @@
import { describe, it, expect, afterAll, afterEach } from 'vitest'; import { describe, it, expect, afterAll, afterEach } from 'vitest';
import http from 'node:http'; import http from 'node:http';
import { McpdClient, ConnectionError } from '../src/http/mcpd-client.js'; import { McpdClient, ConnectionError, UpstreamTimeoutError } from '../src/http/mcpd-client.js';
/** /**
* Create a local HTTP server for testing McpdClient behavior. * Create a local HTTP server for testing McpdClient behavior.
@@ -85,7 +85,7 @@ describe('McpdClient', () => {
// ── Timeout behavior ── // ── Timeout behavior ──
it('times out on slow responses and throws ConnectionError', async () => { it('times out on slow responses and throws UpstreamTimeoutError', async () => {
const { server, url } = await createTestServer((_req, _res) => { const { server, url } = await createTestServer((_req, _res) => {
// Never respond — simulates a hanging upstream tool call // Never respond — simulates a hanging upstream tool call
}); });
@@ -96,7 +96,7 @@ describe('McpdClient', () => {
const start = Date.now(); const start = Date.now();
await expect(client.post('/api/v1/mcp/proxy', { serverId: 's1' })).rejects.toThrow( await expect(client.post('/api/v1/mcp/proxy', { serverId: 's1' })).rejects.toThrow(
/timed out/, /did not respond within/,
); );
const elapsed = Date.now() - start; const elapsed = Date.now() - start;
@@ -105,7 +105,7 @@ describe('McpdClient', () => {
expect(elapsed).toBeLessThan(3000); expect(elapsed).toBeLessThan(3000);
}); });
it('timeout error is a ConnectionError with descriptive message', async () => { it('timeout is NOT a ConnectionError — a slow daemon is not an absent one', async () => {
const { server, url } = await createTestServer((_req, _res) => { const { server, url } = await createTestServer((_req, _res) => {
// Never respond // Never respond
}); });
@@ -117,8 +117,12 @@ describe('McpdClient', () => {
await client.get('/test'); await client.get('/test');
expect.unreachable('Should have thrown'); expect.unreachable('Should have thrown');
} catch (err) { } catch (err) {
expect(err).toBeInstanceOf(ConnectionError); // Reporting a timeout as "cannot connect" is what sent a previous
expect((err as Error).message).toContain('Request timed out after 200ms'); // debugging session chasing a network fault that did not exist.
expect(err).toBeInstanceOf(UpstreamTimeoutError);
expect(err).not.toBeInstanceOf(ConnectionError);
expect((err as UpstreamTimeoutError).timeoutMs).toBe(200);
expect((err as Error).message).toContain('did not respond within 200ms');
} }
}); });
@@ -146,7 +150,7 @@ describe('McpdClient', () => {
const derived = client.withHeaders({ 'X-Custom': 'val' }); const derived = client.withHeaders({ 'X-Custom': 'val' });
const start = Date.now(); const start = Date.now();
await expect(derived.get('/test')).rejects.toThrow(/timed out/); await expect(derived.get('/test')).rejects.toThrow(/did not respond within/);
const elapsed = Date.now() - start; const elapsed = Date.now() - start;
expect(elapsed).toBeLessThan(2000); expect(elapsed).toBeLessThan(2000);
}); });

View File

@@ -0,0 +1,255 @@
import http from 'node:http';
import Fastify, { type FastifyInstance } from 'fastify';
import { describe, it, expect, afterEach } from 'vitest';
import {
McpdClient,
UpstreamTimeoutError,
ConnectionError,
LONG_RUNNING_TIMEOUT_MS,
DEFAULT_TIMEOUT_MS,
} from '../src/http/mcpd-client.js';
import { registerProxyRoutes } from '../src/http/routes/proxy.js';
/**
* Regression cover for the 30s proxy timeout that made `mcpctl chat` fail with
* a misleading "Cannot reach mcpd daemon" 503 while mcpd was answering
* /healthz in 32ms.
*
* Three separate defects are pinned here:
* 1. chat routes inherited the 30s CRUD budget, so any turn longer than 30s
* failed — and an agent turn is a tool-use loop that routinely exceeds it;
* 2. a timeout was reported as a connection failure, sending diagnosis after
* a network fault that did not exist;
* 3. SSE was buffered through res.text(), so streaming never reached the
* client even when the turn finished in time.
*/
let app: FastifyInstance | null = null;
let upstream: FastifyInstance | null = null;
afterEach(async () => {
if (app) { await app.close(); app = null; }
if (upstream) { await upstream.close(); upstream = null; }
});
/** A stand-in mcpd. Returns its base URL. */
async function startUpstream(register: (a: FastifyInstance) => void): Promise<string> {
upstream = Fastify();
register(upstream);
await upstream.listen({ port: 0, host: '127.0.0.1' });
const addr = upstream.server.address();
if (addr === null || typeof addr === 'string') throw new Error('no address');
return `http://127.0.0.1:${String(addr.port)}`;
}
async function startProxy(baseUrl: string, timeoutMs?: number): Promise<FastifyInstance> {
app = Fastify();
registerProxyRoutes(app, new McpdClient(baseUrl, 'test-token', {}, timeoutMs));
await app.ready();
return app;
}
describe('proxy — long-running route budget', () => {
it('gives chat routes the long budget, not the 30s CRUD default', () => {
// The constants themselves are the contract: a 30s cap on an agent turn is
// a guaranteed failure, not a safety net.
expect(DEFAULT_TIMEOUT_MS).toBe(30_000);
expect(LONG_RUNNING_TIMEOUT_MS).toBeGreaterThanOrEqual(600_000);
});
it('does not abort an agent chat that outlives the CRUD budget', async () => {
const base = await startUpstream((a) => {
a.post('/api/v1/agents/:name/chat', async () => {
// Longer than the (deliberately tiny) CRUD budget below. Before the
// fix this inherited that budget and 503'd.
await new Promise((r) => setTimeout(r, 250));
return { answer: 'pong' };
});
});
// CRUD budget of 50ms — a chat route must NOT inherit it.
const proxy = await startProxy(base, 50);
const res = await proxy.inject({
method: 'POST',
url: '/api/v1/agents/reviewer/chat',
payload: { message: 'hi' },
});
expect(res.statusCode).toBe(200);
expect(res.json()).toEqual({ answer: 'pong' });
});
it('still applies the short budget to ordinary CRUD routes', async () => {
const base = await startUpstream((a) => {
a.get('/api/v1/servers', async () => {
await new Promise((r) => setTimeout(r, 300));
return [];
});
});
const proxy = await startProxy(base, 50);
const res = await proxy.inject({ method: 'GET', url: '/api/v1/servers' });
// Times out — and is now reported honestly as a timeout, not a connection fault.
expect(res.statusCode).toBe(504);
expect(res.json().error).toBe('upstream_timeout');
});
it('reports a timeout as 504, never as "cannot reach mcpd"', async () => {
const base = await startUpstream((a) => {
a.get('/api/v1/servers', async () => {
await new Promise((r) => setTimeout(r, 300));
return [];
});
});
const proxy = await startProxy(base, 50);
const res = await proxy.inject({ method: 'GET', url: '/api/v1/servers' });
const body = res.json();
expect(body.message).toMatch(/did not respond within/);
expect(body.message).not.toMatch(/Cannot reach mcpd/);
expect(body.message).toMatch(/reachable/);
});
it('streams SSE through instead of buffering it', async () => {
const base = await startUpstream((a) => {
a.post('/api/v1/agents/:name/chat', async (_req, reply) => {
reply.raw.writeHead(200, {
'Content-Type': 'text/event-stream',
'Cache-Control': 'no-cache',
'X-Accel-Buffering': 'no',
});
reply.raw.write('data: {"type":"text","delta":"po"}\n\n');
reply.raw.write('data: {"type":"text","delta":"ng"}\n\n');
reply.raw.write('data: [DONE]\n\n');
reply.raw.end();
return reply;
});
});
const proxy = await startProxy(base, 50);
const res = await proxy.inject({
method: 'POST',
url: '/api/v1/agents/reviewer/chat',
payload: { message: 'hi', stream: true },
});
expect(res.statusCode).toBe(200);
// Content-type must survive — a client that gets application/json will not
// parse the event stream.
expect(res.headers['content-type']).toMatch(/text\/event-stream/);
// x-accel-buffering=no must survive too, or intermediaries re-buffer the
// stream and reintroduce the stall.
expect(res.headers['x-accel-buffering']).toBe('no');
expect(res.body).toContain('"delta":"po"');
expect(res.body).toContain('"delta":"ng"');
expect(res.body).toContain('[DONE]');
});
it('delivers each SSE frame while the upstream is still generating', async () => {
// The buffering regression is invisible to the pass-through test above:
// `inject()` collects the whole body, so a proxy that buffers via
// res.text() still passes it. This test proves *progressive* delivery by
// making the upstream withhold its final frame until the client has
// observed the first one. A buffering proxy can never satisfy that
// ordering — the 3s guard resolves the gate so the run fails cleanly
// instead of deadlocking.
let openGate: (seen: boolean) => void = () => {};
const clientSawFirstFrame = new Promise<boolean>((r) => { openGate = r; });
const guard = setTimeout(() => openGate(false), 3_000);
const base = await startUpstream((a) => {
a.post('/api/v1/agents/:name/chat', async (_req, reply) => {
reply.raw.writeHead(200, { 'Content-Type': 'text/event-stream' });
reply.raw.write('data: {"type":"text","delta":"live"}\n\n');
await clientSawFirstFrame;
reply.raw.write('data: {"type":"final"}\n\n');
reply.raw.write('data: [DONE]\n\n');
reply.raw.end();
return reply;
});
});
const proxy = await startProxy(base, 50);
await proxy.listen({ port: 0, host: '127.0.0.1' });
const addr = proxy.server.address();
if (addr === null || typeof addr === 'string') throw new Error('no address');
const body = await new Promise<string>((resolve, reject) => {
const req = http.request({
hostname: '127.0.0.1',
port: addr.port,
path: '/api/v1/agents/reviewer/chat',
method: 'POST',
headers: { 'Content-Type': 'application/json' },
}, (res) => {
let acc = '';
res.setEncoding('utf-8');
res.on('data', (chunk: string) => {
acc += chunk;
if (acc.includes('"delta":"live"')) openGate(true);
});
res.on('end', () => resolve(acc));
res.on('error', reject);
});
req.on('error', reject);
req.end(JSON.stringify({ message: 'hi', stream: true }));
});
clearTimeout(guard);
// The ordering proof: the first frame reached the client while the
// upstream was still holding the stream open.
await expect(clientSawFirstFrame).resolves.toBe(true);
expect(body).toContain('"type":"final"');
expect(body).toContain('[DONE]');
});
it('relays a non-200 status from a streaming route', async () => {
const base = await startUpstream((a) => {
a.post('/api/v1/agents/:name/chat', async (_req, reply) => {
return reply.code(404).send({ error: 'Agent not found' });
});
});
const proxy = await startProxy(base, 50);
const res = await proxy.inject({
method: 'POST',
url: '/api/v1/agents/ghost/chat',
payload: { message: 'hi' },
});
expect(res.statusCode).toBe(404);
expect(res.body).toContain('Agent not found');
});
it('still reports a genuinely unreachable daemon as 503', async () => {
// Port 1 is reserved and refuses instantly.
const proxy = await startProxy('http://127.0.0.1:1', 500);
const res = await proxy.inject({ method: 'GET', url: '/api/v1/servers' });
expect(res.statusCode).toBe(503);
expect(res.json().error).toBe('service_unavailable');
});
it('propagates 401 from a streaming route so login guidance still fires', async () => {
const base = await startUpstream((a) => {
a.post('/api/v1/agents/:name/chat', async (_req, reply) => reply.code(401).send({}));
});
const proxy = await startProxy(base, 50);
const res = await proxy.inject({
method: 'POST',
url: '/api/v1/agents/reviewer/chat',
payload: { message: 'hi' },
});
expect(res.statusCode).toBe(401);
expect(res.json().message).toMatch(/mcpctl login/);
});
});
describe('error taxonomy', () => {
it('keeps timeout and unreachable as distinct types', () => {
const timeout = new UpstreamTimeoutError('http://mcpd', 30_000);
expect(timeout).not.toBeInstanceOf(ConnectionError);
expect(timeout.timeoutMs).toBe(30_000);
expect(timeout.message).toMatch(/did not respond within 30000ms/);
});
});

View File

@@ -18,8 +18,12 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import http from 'node:http'; import http from 'node:http';
import https from 'node:https'; import https from 'node:https';
import { spawnSync, execSync } from 'node:child_process'; import { spawnSync, execSync } from 'node:child_process';
import { existsSync, readFileSync } from 'node:fs';
import { join } from 'node:path';
import { homedir } from 'node:os';
const MCPD_URL = process.env.MCPD_URL ?? 'https://mcpctl.ad.itaz.eu'; const MCPD_URL = process.env.MCPD_URL ?? 'https://mcpctl.ad.itaz.eu';
const MCPLOCAL_URL = process.env.MCPLOCAL_URL ?? 'http://localhost:3200';
const LLM_URL = process.env.MCPCTL_SMOKE_LLM_URL; const LLM_URL = process.env.MCPCTL_SMOKE_LLM_URL;
const LLM_MODEL = process.env.MCPCTL_SMOKE_LLM_MODEL ?? 'qwen3-thinking'; const LLM_MODEL = process.env.MCPCTL_SMOKE_LLM_MODEL ?? 'qwen3-thinking';
const LLM_KEY = process.env.MCPCTL_SMOKE_LLM_KEY; const LLM_KEY = process.env.MCPCTL_SMOKE_LLM_KEY;
@@ -27,6 +31,10 @@ const SUFFIX = Date.now().toString(36);
const SECRET_NAME = `smoke-chat-sec-${SUFFIX}`; const SECRET_NAME = `smoke-chat-sec-${SUFFIX}`;
const LLM_NAME = `smoke-chat-llm-${SUFFIX}`; const LLM_NAME = `smoke-chat-llm-${SUFFIX}`;
const AGENT_NAME = `smoke-chat-agent-${SUFFIX}`; const AGENT_NAME = `smoke-chat-agent-${SUFFIX}`;
// Dedicated agent for the streaming-timing test: the shared agent's system
// prompt pins the reply to a single token, which is too short to distinguish
// live streaming from an end-of-turn buffer dump.
const STREAM_AGENT_NAME = `smoke-stream-agent-${SUFFIX}`;
interface CliResult { code: number; stdout: string; stderr: string } interface CliResult { code: number; stdout: string; stderr: string }
@@ -99,6 +107,7 @@ describe('agent chat smoke (live LLM)', () => {
afterAll(() => { afterAll(() => {
if (!liveLlmConfigured || !mcpdUp) return; if (!liveLlmConfigured || !mcpdUp) return;
run(`delete agent ${AGENT_NAME}`); run(`delete agent ${AGENT_NAME}`);
run(`delete agent ${STREAM_AGENT_NAME}`);
run(`delete llm ${LLM_NAME}`); run(`delete llm ${LLM_NAME}`);
run(`delete secret ${SECRET_NAME}`); run(`delete secret ${SECRET_NAME}`);
}); });
@@ -139,6 +148,92 @@ describe('agent chat smoke (live LLM)', () => {
expect(result.stderr).toMatch(/thread:\s+c[a-z0-9]+/); expect(result.stderr).toMatch(/thread:\s+c[a-z0-9]+/);
}); });
it('streams progressively THROUGH mcplocal — frames arrive during generation, not in one burst', async () => {
if (!liveLlmConfigured || !mcpdUp) return;
// The regression this pins: mcplocal's /api/v1/* proxy buffered SSE via
// res.text(), so the CLI showed nothing until the turn finished and then
// dumped the whole answer at once. The --direct tests above bypass
// mcplocal entirely and cannot catch that. This one posts to the local
// proxy (the path `mcpctl chat` actually takes) and asserts frames are
// spread across the generation window: with buffering, everything lands
// within a few ms of stream end.
if (!(await healthz(MCPLOCAL_URL))) {
// eslint-disable-next-line no-console
console.warn(`\n ○ mcplocal streaming smoke: skipped — ${MCPLOCAL_URL}/healthz unreachable.\n`);
return;
}
let token = '';
try {
const credsPath = join(homedir(), '.mcpctl', 'credentials');
if (existsSync(credsPath)) {
const creds = JSON.parse(readFileSync(credsPath, 'utf-8')) as { token?: string };
if (creds.token !== undefined) token = creds.token;
}
} catch { /* unauthenticated — the request will 401 and fail loudly */ }
run(`delete agent ${STREAM_AGENT_NAME}`);
const agent = run([
`create agent ${STREAM_AGENT_NAME}`,
`--llm ${LLM_NAME}`,
`--description "mcplocal streaming smoke"`,
`--system-prompt "You are a smoke test. Follow the user's instructions exactly."`,
'--default-temperature 0',
'--default-max-tokens 512',
].join(' '));
expect(agent.code, agent.stderr).toBe(0);
const url = new URL(`${MCPLOCAL_URL.replace(/\/$/, '')}/api/v1/agents/${STREAM_AGENT_NAME}/chat`);
const deltaTimes: number[] = [];
let endTime = 0;
let status = 0;
let raw = '';
await new Promise<void>((resolve, reject) => {
const req = http.request({
hostname: url.hostname,
port: url.port || 80,
path: url.pathname,
method: 'POST',
timeout: 120_000,
headers: {
'Content-Type': 'application/json',
...(token !== '' ? { Authorization: `Bearer ${token}` } : {}),
},
}, (res) => {
status = res.statusCode ?? 0;
res.setEncoding('utf-8');
let buf = '';
res.on('data', (chunk: string) => {
raw += chunk;
buf += chunk;
let nl: number;
while ((nl = buf.indexOf('\n\n')) !== -1) {
const frame = buf.slice(0, nl);
buf = buf.slice(nl + 2);
if (/"type":"(text|thinking)"/.test(frame)) deltaTimes.push(Date.now());
}
});
res.on('end', () => { endTime = Date.now(); resolve(); });
res.on('error', reject);
});
req.on('error', reject);
req.on('timeout', () => { req.destroy(); reject(new Error('stream timed out')); });
req.end(JSON.stringify({
message: 'Count from 1 to 40, one number per line. No other text.',
stream: true,
max_tokens: 400,
}));
});
expect(status, raw.slice(0, 500)).toBe(200);
expect(deltaTimes.length).toBeGreaterThanOrEqual(2);
// The buffering signature: every frame lands in the same final burst as
// stream end. Live streaming puts the first delta well before the end —
// a 40-line generation spans seconds; 300ms is a conservative floor.
const firstDelta = deltaTimes[0]!;
expect(endTime - firstDelta).toBeGreaterThanOrEqual(300);
}, 150_000);
it('streaming `mcpctl chat` emits text deltas', () => { it('streaming `mcpctl chat` emits text deltas', () => {
if (!liveLlmConfigured || !mcpdUp) return; if (!liveLlmConfigured || !mcpdUp) return;
// Default mode is streaming. Pipe stdout/stderr separately. // Default mode is streaming. Pipe stdout/stderr separately.

View File

@@ -12,6 +12,19 @@ servers:
env: env:
- name: FASTMCP_LOG_LEVEL - name: FASTMCP_LOG_LEVEL
value: "ERROR" value: "ERROR"
# Mirrors the production `aws-docs` probe. Without it this fixture is a
# RUNNING server with no readiness probe, so it fails the very assertion in
# health-readiness.smoke.test.ts that the fixture exists to support — the
# suite reporting its own scaffolding as a fleet regression.
# `search_documentation` needs a phrase; the 300s interval matches aws-docs,
# since the call leaves the cluster.
healthCheck:
tool: search_documentation
arguments:
search_phrase: "s3 bucket"
timeoutSeconds: 20
intervalSeconds: 300
failureThreshold: 3
projects: projects:
- name: smoke-data - name: smoke-data

View File

@@ -32,6 +32,18 @@ function httpRequest(opts: {
headers?: Record<string, string>; headers?: Record<string, string>;
body?: string; body?: string;
timeout?: number; timeout?: number;
/**
* Resolve as soon as the response headers arrive, then hang up, instead of
* waiting for the body to end.
*
* Required for a streaming endpoint: SSE responses never end, so the normal
* path can only settle via the socket's *inactivity* timeout — which never
* fires while the stream is busy. `/inspect` relays every project's MCP
* traffic, so during a full smoke run it is never idle, and the request hung
* until vitest killed the test. Alone it looked flaky; under load it failed
* every time. Reading the status does not need the body anyway.
*/
headersOnly?: boolean;
}): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> { }): Promise<{ status: number; headers: http.IncomingHttpHeaders; body: string }> {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const parsed = new URL(opts.url); const parsed = new URL(opts.url);
@@ -46,6 +58,12 @@ function httpRequest(opts: {
timeout: opts.timeout ?? 10_000, timeout: opts.timeout ?? 10_000,
}, },
(res) => { (res) => {
if (opts.headersOnly === true) {
resolve({ status: res.statusCode ?? 0, headers: res.headers, body: '' });
res.destroy();
req.destroy();
return;
}
const chunks: Buffer[] = []; const chunks: Buffer[] = [];
res.on('data', (chunk: Buffer) => chunks.push(chunk)); res.on('data', (chunk: Buffer) => chunks.push(chunk));
res.on('end', () => { res.on('end', () => {
@@ -93,17 +111,15 @@ describe('Smoke: Security — mcplocal unauthenticated endpoints', () => {
// /inspect streams ALL MCP traffic (tool calls, arguments, responses) // /inspect streams ALL MCP traffic (tool calls, arguments, responses)
// for ALL projects to any unauthenticated local client // for ALL projects to any unauthenticated local client
// headersOnly: the stream never ends, and waiting for it to go idle is what
// made this hang whenever other suites were generating traffic. The status
// line is all this assertion needs.
const res = await httpRequest({ const res = await httpRequest({
url: `${MCPLOCAL_URL}/inspect`, url: `${MCPLOCAL_URL}/inspect`,
method: 'GET', method: 'GET',
headers: { 'Accept': 'text/event-stream' }, headers: { 'Accept': 'text/event-stream' },
timeout: 3_000, timeout: 3_000,
}).catch((err) => { headersOnly: true,
// Timeout is expected (SSE keeps connection open) — still means endpoint is accessible
if ((err as Error).message.includes('timed out')) {
return { status: 200, headers: {} as http.IncomingHttpHeaders, body: '' };
}
throw err;
}); });
// Should be accessible without auth (documenting the vulnerability) // Should be accessible without auth (documenting the vulnerability)