Compare commits
14 Commits
worktree-f
...
d4c33baf03
| Author | SHA1 | Date | |
|---|---|---|---|
| d4c33baf03 | |||
|
|
bbd2195c64 | ||
|
|
5a8185d7c9 | ||
| ae5a6203f8 | |||
|
|
822c1bb047 | ||
| cd94e855aa | |||
|
|
b8cedd6262 | ||
| 13f1ff28eb | |||
|
|
96e27c8716 | ||
|
|
dd29f98f82 | ||
|
|
2a7bba11ea | ||
| 2b87cfdbf1 | |||
|
|
be7fabd467 | ||
| e4e2e063f1 |
@@ -101,13 +101,48 @@ to open, and re-scoping it would silently change which skills sync into it.
|
||||
⏵⏵ bypass permissions on · ← for agents
|
||||
```
|
||||
|
||||
`mcpctl statusline` resolves the project from a directory-scoped `.mcp.json`
|
||||
first (a repo that pinned itself wins), then the user-scope entry in
|
||||
`.claude.json`, then a `.mcpctl-project` marker up the tree so a checkout that is scoped but not yet
|
||||
wired still reports. It reads the directory from the JSON Claude Code pipes in,
|
||||
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
|
||||
saying "none" on every unrelated repo.
|
||||
`mcpctl statusline` reads the directory from the JSON Claude Code pipes in, 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 saying
|
||||
"none" on every unrelated repo.
|
||||
|
||||
It then takes the project from the most deliberate source that names one:
|
||||
|
||||
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
|
||||
|
||||
|
||||
@@ -101,9 +101,34 @@ src/pi-ext/
|
||||
mcp-http.ts # vendored Streamable-HTTP JSON-RPC client (no deps)
|
||||
```
|
||||
|
||||
The extension imports only from pi-bundled packages
|
||||
(`@earendil-works/pi-coding-agent`, `@earendil-works/pi-ai`, `typebox`), so it
|
||||
loads standalone.
|
||||
The extension imports only from pi-bundled packages, so it loads standalone.
|
||||
|
||||
### `typebox` is the only bare runtime import
|
||||
|
||||
pi does not let an extension resolve modules the ordinary way: it hands jiti a
|
||||
hard-coded alias table built from its *own* dependencies, and that table is not
|
||||
the same across pi distributions. The newer `@earendil-works/pi-coding-agent`
|
||||
aliases both the `@earendil-works/*` and the legacy `@mariozechner/*` names;
|
||||
older `@mariozechner/pi-coding-agent` installs (0.73.x and earlier) alias only
|
||||
the `@mariozechner/*` ones. Neither resolves the other's namespace.
|
||||
|
||||
So an import of anything outside the intersection kills the *whole* extension on
|
||||
someone else's pi — every tool, the `/mcpctl` command, the status line — with:
|
||||
|
||||
```
|
||||
Failed to load extension ".../mcpctl-pi.ts": Cannot find module '@earendil-works/pi-ai'
|
||||
```
|
||||
|
||||
which is exactly what `import { StringEnum } from "@earendil-works/pi-ai"` did.
|
||||
`typebox` is aliased by every published pi, so it is the only bare specifier
|
||||
allowed at runtime. Everything else must be a `node:` builtin, a relative path,
|
||||
an `import type` (erased before jiti resolves anything), or inlined — pi-ai's
|
||||
`StringEnum` is now a six-line local `stringEnum`. The
|
||||
`tests/config/pi-extension-embed.test.ts` guard fails the build on a reintroduced
|
||||
runtime import.
|
||||
|
||||
If a user does hit this error, check `type -a pi`: two installs on `$PATH` is the
|
||||
usual cause, and the extension has to load under whichever one wins.
|
||||
|
||||
## Typechecking
|
||||
|
||||
|
||||
@@ -884,6 +884,39 @@ All pushed to `mysources.co.uk/michal/` registry.
|
||||
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:
|
||||
- `/usr/bin/mcpctl` — CLI binary (bun compiled)
|
||||
- `/usr/bin/mcpctl-local` — Local proxy binary (bun compiled)
|
||||
|
||||
@@ -19,6 +19,11 @@ source "$SCRIPT_DIR/arch-helper.sh"
|
||||
resolve_arch "${MCPCTL_TARGET_ARCH:-}"
|
||||
# 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
|
||||
source "$SCRIPT_DIR/ensure-deps.sh"
|
||||
ensure_build_deps
|
||||
|
||||
@@ -16,6 +16,11 @@ if [ -f .env ]; then
|
||||
set -a; source .env; set +a
|
||||
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)
|
||||
REGISTRY="10.0.0.194:3012"
|
||||
IMAGE="mcpd"
|
||||
|
||||
@@ -19,6 +19,11 @@ source "$SCRIPT_DIR/arch-helper.sh"
|
||||
resolve_arch "${MCPCTL_TARGET_ARCH:-}"
|
||||
# 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
|
||||
source "$SCRIPT_DIR/ensure-deps.sh"
|
||||
ensure_build_deps
|
||||
|
||||
101
scripts/check-main-sync.sh
Executable file
101
scripts/check-main-sync.sh
Executable 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
|
||||
}
|
||||
@@ -80,6 +80,20 @@ cat <<EOF
|
||||
EOF
|
||||
[ -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 ──
|
||||
if [ "$SKIP_TESTS" = true ]; then warn "skipping unit tests (--skip-tests)"; else
|
||||
say "1/7 Unit tests (pnpm test:run)"
|
||||
|
||||
@@ -75,9 +75,28 @@ echo "==> Running smoke tests..."
|
||||
export PATH="$HOME/.npm-global/bin:$PATH"
|
||||
if pnpm test:smoke; then
|
||||
echo "==> Smoke tests passed!"
|
||||
elif [ "${MCPCTL_ALLOW_SMOKE_FAILURE:-}" = "1" ]; then
|
||||
echo "==> WARNING: Smoke tests failed, continuing (MCPCTL_ALLOW_SMOKE_FAILURE=1)."
|
||||
else
|
||||
echo "==> WARNING: Smoke tests failed! Check mcplocal/mcpd are running."
|
||||
echo " Continuing anyway — deployment is complete, but verify manually."
|
||||
# This used to print a warning and exit 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. 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
|
||||
echo ""
|
||||
|
||||
|
||||
@@ -46,6 +46,8 @@ import {
|
||||
mergeUserScopeServer,
|
||||
userScopeProject,
|
||||
activeProjectIn,
|
||||
canonicalProjectIn,
|
||||
legacyEntriesIn,
|
||||
claudeJsonPath,
|
||||
type McpJson,
|
||||
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 {
|
||||
configDeps: Partial<ConfigLoaderDeps>;
|
||||
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. */
|
||||
apiClient?: ApiClient;
|
||||
/** Working directory to check for a shadowing `.mcp.json`. Injectable so tests need not chdir. */
|
||||
cwd?: () => string;
|
||||
}
|
||||
|
||||
export interface ConfigApiDeps {
|
||||
@@ -124,6 +166,7 @@ const defaultDeps: ConfigCommandDeps = {
|
||||
|
||||
export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?: ConfigApiDeps): Command {
|
||||
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
|
||||
// initial skills sync after wiring the .mcp.json. Threaded through from
|
||||
// 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) {
|
||||
// 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.');
|
||||
// ...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
|
||||
|
||||
@@ -2,7 +2,16 @@ import { Command } from 'commander';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join, resolve } from 'node:path';
|
||||
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';
|
||||
|
||||
/**
|
||||
@@ -16,9 +25,10 @@ import { findProjectMarker } from '../utils/project-marker.js';
|
||||
* `setStatus`.
|
||||
*
|
||||
* 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,
|
||||
* falling back to a `.mcpctl-project` marker up the tree so a checkout that is
|
||||
* scoped but not yet wired still reports.
|
||||
* only need the directory; the project is then resolved from the most
|
||||
* deliberate source that names one — see the ranking in the action below.
|
||||
* 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
|
||||
* 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;
|
||||
}
|
||||
|
||||
/** The project Claude Code's user-scope config mounts, or null. */
|
||||
export function projectFromUserScope(path: string): string | null {
|
||||
/** Claude Code's user-scope config, or null if it is missing or unreadable. */
|
||||
export function readClaudeJson(path: string): ClaudeJson | null {
|
||||
try {
|
||||
return userScopeProject(JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson);
|
||||
return JSON.parse(readFileSync(path, 'utf-8')) as ClaudeJson;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** The project `.mcp.json` in `dir` mounts, or null. */
|
||||
export function projectFromMcpJson(dir: string): string | null {
|
||||
/** The project Claude Code's user-scope config mounts, or 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 {
|
||||
const parsed = JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson;
|
||||
return activeProjectIn(parsed);
|
||||
return JSON.parse(readFileSync(join(dir, '.mcp.json'), 'utf-8')) as McpJson;
|
||||
} catch {
|
||||
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". */
|
||||
export function formatStatus(project: string | null, prefix: string): string {
|
||||
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 dir = opts.directory !== undefined ? resolve(opts.directory) : resolveDirectory(input, cwd());
|
||||
|
||||
// Directory-scoped wiring wins: a repo with its own .mcp.json entry has
|
||||
// deliberately pinned itself, and that beats the global default.
|
||||
let project = projectFromMcpJson(dir) ?? projectFromUserScope(claudeJsonPath());
|
||||
const claudeJson = readClaudeJson(claudeJsonPath());
|
||||
const mcpJson = readDirMcpJson(dir);
|
||||
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) {
|
||||
// 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);
|
||||
project = marker?.project ?? null;
|
||||
}
|
||||
|
||||
@@ -100,16 +100,30 @@ export function isLegacyMcpctlEntry(name: string, entry: unknown): boolean {
|
||||
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. */
|
||||
export function activeProjectIn(config: Pick<McpJson, 'mcpServers'> | null | undefined): string | null {
|
||||
const servers = config?.mcpServers;
|
||||
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;
|
||||
return canonicalProjectIn(config) ?? legacyEntriesIn(config)[0]?.project ?? null;
|
||||
}
|
||||
|
||||
export interface MergeResult {
|
||||
@@ -169,12 +183,39 @@ export function claudeJsonPath(env: NodeJS.ProcessEnv = process.env, homeDir?: s
|
||||
: 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. */
|
||||
export interface ClaudeJson {
|
||||
mcpServers?: Record<string, McpServerEntry>;
|
||||
projects?: Record<string, ClaudeProjectEntry>;
|
||||
[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
|
||||
* project-named entries retired from it.
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -359,4 +359,46 @@ describe('config claude — user scope', () => {
|
||||
process.exitCode = prevExit;
|
||||
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:');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
121
src/cli/tests/commands/statusline.test.ts
Normal file
121
src/cli/tests/commands/statusline.test.ts
Normal 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');
|
||||
});
|
||||
});
|
||||
@@ -5,6 +5,9 @@ import {
|
||||
projectOfEntry,
|
||||
isLegacyMcpctlEntry,
|
||||
activeProjectIn,
|
||||
canonicalProjectIn,
|
||||
legacyEntriesIn,
|
||||
disabledServersFor,
|
||||
} from '../../src/config/claude-mcp.js';
|
||||
|
||||
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', () => {
|
||||
it('writes one constant entry regardless of project', () => {
|
||||
const { config } = mergeMcpctlServers(null, { project: 'my-fancy-project' });
|
||||
|
||||
@@ -29,6 +29,34 @@ describe('embedded pi extension', () => {
|
||||
expect(PI_EXTENSION_FILES['mcpctl-pi.ts']).toContain('./mcp-http.js');
|
||||
});
|
||||
|
||||
/**
|
||||
* pi resolves an extension's bare specifiers through a hard-coded alias table
|
||||
* in its own loader, and that table is not the same across pi distributions:
|
||||
* `@earendil-works/*` exists only in the newer packages, `@mariozechner/*`
|
||||
* installs alias only the old names, and neither resolves the other. An
|
||||
* import of a package outside the intersection makes the whole extension fail
|
||||
* to load with `Cannot find module` — every tool gone, on someone else's pi.
|
||||
*
|
||||
* `typebox` is aliased by every published pi, so it is the only safe bare
|
||||
* runtime import. Type-only imports are erased before jiti resolves anything,
|
||||
* so they may name whatever they like.
|
||||
*/
|
||||
it('imports nothing at runtime that some pi build cannot resolve', () => {
|
||||
// `import x from "s"` / `import {..} from "s"` (but not `import type`),
|
||||
// plus the side-effect form `import "s"`.
|
||||
const runtimeImport =
|
||||
/^\s*import\s+(?!type\s)[^;]*?from\s*["']([^"']+)["']|^\s*import\s*["']([^"']+)["']/gm;
|
||||
const allowed = /^(node:|\.\/|\.\.\/|typebox$|typebox\/)/;
|
||||
|
||||
for (const name of PI_EXTENSION_FILENAMES) {
|
||||
const src = PI_EXTENSION_FILES[name] ?? '';
|
||||
for (const match of src.matchAll(runtimeImport)) {
|
||||
const specifier = match[1] ?? match[2] ?? '';
|
||||
expect(specifier, `${name} runtime-imports ${specifier}`).toMatch(allowed);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('carries the fixes the pi API requires', () => {
|
||||
const main = PI_EXTENSION_FILES['mcpctl-pi.ts'] ?? '';
|
||||
// ctx.ui.select takes string[] and returns the chosen string.
|
||||
|
||||
@@ -2,7 +2,7 @@ export { createHttpServer } from './server.js';
|
||||
export type { HttpServerDeps } from './server.js';
|
||||
export { loadHttpConfig } 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 { registerMcpEndpoint } from './mcp-endpoint.js';
|
||||
export { registerProjectMcpEndpoint } from './project-mcp-endpoint.js';
|
||||
|
||||
@@ -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. */
|
||||
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
|
||||
* the full tool-call timeout budget — a single dead upstream would stall session init for
|
||||
@@ -121,9 +153,7 @@ export class McpdClient {
|
||||
try {
|
||||
res = await fetch(url, init);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof DOMException && err.name === 'TimeoutError') {
|
||||
throw new ConnectionError(this.baseUrl, new Error(`Request timed out after ${this.timeoutMs}ms`));
|
||||
}
|
||||
if (isTimeout(err)) throw new UpstreamTimeoutError(this.baseUrl, this.timeoutMs);
|
||||
throw new ConnectionError(this.baseUrl, err);
|
||||
}
|
||||
|
||||
@@ -131,7 +161,18 @@ export class McpdClient {
|
||||
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;
|
||||
try {
|
||||
parsed = JSON.parse(text);
|
||||
@@ -142,6 +183,51 @@ export class McpdClient {
|
||||
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> {
|
||||
const result = await this.forward(method, path, '', body);
|
||||
|
||||
|
||||
@@ -1,10 +1,62 @@
|
||||
/**
|
||||
* Catch-all proxy route that forwards /api/v1/* requests to mcpd.
|
||||
*/
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import { AuthenticationError, ConnectionError } from '../mcpd-client.js';
|
||||
import { Readable } from 'node:stream';
|
||||
|
||||
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';
|
||||
|
||||
/**
|
||||
* 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 {
|
||||
app.all('/api/v1/*', async (request, reply) => {
|
||||
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.
|
||||
// If no user token is present, mcpd will use its auth hook to reject.
|
||||
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 {
|
||||
const result = await client.forward(request.method, path, querystring, body, userToken);
|
||||
return reply.code(result.status).send(result.body);
|
||||
} catch (err: unknown) {
|
||||
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 ConnectionError) {
|
||||
return reply.code(503).send({
|
||||
error: 'service_unavailable',
|
||||
message: 'Cannot reach mcpd daemon. Is it running?',
|
||||
});
|
||||
}
|
||||
const handled = sendUpstreamError(reply, err);
|
||||
if (handled) return handled;
|
||||
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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,7 +11,7 @@ export type { MainResult } from './main.js';
|
||||
export { ProviderRegistry } 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 { 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 {
|
||||
JsonRpcRequest,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, afterAll, afterEach } from 'vitest';
|
||||
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.
|
||||
@@ -85,7 +85,7 @@ describe('McpdClient', () => {
|
||||
|
||||
// ── 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) => {
|
||||
// Never respond — simulates a hanging upstream tool call
|
||||
});
|
||||
@@ -96,7 +96,7 @@ describe('McpdClient', () => {
|
||||
|
||||
const start = Date.now();
|
||||
await expect(client.post('/api/v1/mcp/proxy', { serverId: 's1' })).rejects.toThrow(
|
||||
/timed out/,
|
||||
/did not respond within/,
|
||||
);
|
||||
const elapsed = Date.now() - start;
|
||||
|
||||
@@ -105,7 +105,7 @@ describe('McpdClient', () => {
|
||||
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) => {
|
||||
// Never respond
|
||||
});
|
||||
@@ -117,8 +117,12 @@ describe('McpdClient', () => {
|
||||
await client.get('/test');
|
||||
expect.unreachable('Should have thrown');
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(ConnectionError);
|
||||
expect((err as Error).message).toContain('Request timed out after 200ms');
|
||||
// Reporting a timeout as "cannot connect" is what sent a previous
|
||||
// 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 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;
|
||||
expect(elapsed).toBeLessThan(2000);
|
||||
});
|
||||
|
||||
255
src/mcplocal/tests/proxy-long-running.test.ts
Normal file
255
src/mcplocal/tests/proxy-long-running.test.ts
Normal 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/);
|
||||
});
|
||||
});
|
||||
@@ -18,8 +18,12 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import http from 'node:http';
|
||||
import https from 'node:https';
|
||||
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 MCPLOCAL_URL = process.env.MCPLOCAL_URL ?? 'http://localhost:3200';
|
||||
const LLM_URL = process.env.MCPCTL_SMOKE_LLM_URL;
|
||||
const LLM_MODEL = process.env.MCPCTL_SMOKE_LLM_MODEL ?? 'qwen3-thinking';
|
||||
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 LLM_NAME = `smoke-chat-llm-${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 }
|
||||
|
||||
@@ -99,6 +107,7 @@ describe('agent chat smoke (live LLM)', () => {
|
||||
afterAll(() => {
|
||||
if (!liveLlmConfigured || !mcpdUp) return;
|
||||
run(`delete agent ${AGENT_NAME}`);
|
||||
run(`delete agent ${STREAM_AGENT_NAME}`);
|
||||
run(`delete llm ${LLM_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]+/);
|
||||
});
|
||||
|
||||
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', () => {
|
||||
if (!liveLlmConfigured || !mcpdUp) return;
|
||||
// Default mode is streaming. Pipe stdout/stderr separately.
|
||||
|
||||
@@ -12,6 +12,19 @@ servers:
|
||||
env:
|
||||
- name: FASTMCP_LOG_LEVEL
|
||||
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:
|
||||
- name: smoke-data
|
||||
|
||||
@@ -32,6 +32,18 @@ function httpRequest(opts: {
|
||||
headers?: Record<string, string>;
|
||||
body?: string;
|
||||
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 }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const parsed = new URL(opts.url);
|
||||
@@ -46,6 +58,12 @@ function httpRequest(opts: {
|
||||
timeout: opts.timeout ?? 10_000,
|
||||
},
|
||||
(res) => {
|
||||
if (opts.headersOnly === true) {
|
||||
resolve({ status: res.statusCode ?? 0, headers: res.headers, body: '' });
|
||||
res.destroy();
|
||||
req.destroy();
|
||||
return;
|
||||
}
|
||||
const chunks: Buffer[] = [];
|
||||
res.on('data', (chunk: Buffer) => chunks.push(chunk));
|
||||
res.on('end', () => {
|
||||
@@ -93,17 +111,15 @@ describe('Smoke: Security — mcplocal unauthenticated endpoints', () => {
|
||||
|
||||
// /inspect streams ALL MCP traffic (tool calls, arguments, responses)
|
||||
// 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({
|
||||
url: `${MCPLOCAL_URL}/inspect`,
|
||||
method: 'GET',
|
||||
headers: { 'Accept': 'text/event-stream' },
|
||||
timeout: 3_000,
|
||||
}).catch((err) => {
|
||||
// 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;
|
||||
headersOnly: true,
|
||||
});
|
||||
|
||||
// Should be accessible without auth (documenting the vulnerability)
|
||||
|
||||
@@ -20,9 +20,16 @@
|
||||
* or via settings: "extensions": ["/abs/path/to/mcpctl-pi.ts"]
|
||||
*
|
||||
* Only imports pi-bundled packages — no @mcpctl/*, no ~/.claude.
|
||||
*
|
||||
* RUNTIME IMPORTS ARE LOAD-BEARING: pi resolves an extension's bare specifiers
|
||||
* through a fixed alias table in its own loader, and that table differs between
|
||||
* pi distributions — `@earendil-works/*` exists only in the newer packages,
|
||||
* while `@mariozechner/*` installs alias only the old names. `typebox` is the
|
||||
* one specifier every published pi aliases, so it is the ONLY runtime import
|
||||
* allowed here. Anything else must be `import type` (erased before jiti runs)
|
||||
* or inlined — see `stringEnum` below.
|
||||
*/
|
||||
import { Type, type TSchema } from "typebox";
|
||||
import { StringEnum } from "@earendil-works/pi-ai";
|
||||
import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent";
|
||||
import {
|
||||
McpHttpSession,
|
||||
@@ -110,6 +117,23 @@ async function listProjects(mcplocalUrl: string, token?: string): Promise<string
|
||||
}
|
||||
|
||||
// ── JSON Schema → TypeBox ────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* `{ type: "string", enum: [...] }` rather than a union of literals: Google's
|
||||
* API (and other providers that reject anyOf/const) only accept the flat form.
|
||||
*
|
||||
* Inlined from pi-ai's `StringEnum` on purpose — importing it dragged in
|
||||
* `@earendil-works/pi-ai`, which older pi installs cannot resolve, and the
|
||||
* whole extension then failed to load. See the import note at the top.
|
||||
*/
|
||||
function stringEnum(values: string[], description?: string): TSchema {
|
||||
return Type.Unsafe<string>({
|
||||
type: "string",
|
||||
enum: values,
|
||||
...(description ? { description } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
function convertSchema(inputSchema: unknown): TSchema {
|
||||
if (!inputSchema || typeof inputSchema !== "object") {
|
||||
return Type.Object({});
|
||||
@@ -147,7 +171,7 @@ function convertProp(raw: unknown): TSchema {
|
||||
const enumVals = Array.isArray(s.enum) && s.enum.length > 0 ? s.enum : undefined;
|
||||
|
||||
if (enumVals && enumVals.every((v) => typeof v === "string")) {
|
||||
return StringEnum(enumVals as string[]);
|
||||
return stringEnum(enumVals as string[], desc);
|
||||
}
|
||||
if (enumVals && enumVals.every((v) => typeof v === "number")) {
|
||||
const literals = enumVals.map((v) => Type.Literal(v));
|
||||
|
||||
Reference in New Issue
Block a user