Compare commits

..

8 Commits

Author SHA1 Message Date
Michal
5fb1154190 Merge remote-tracking branch 'origin/main' into fix/stdio-restart-recovery
Some checks failed
CI/CD / lint (push) Successful in 1m12s
CI/CD / test (push) Successful in 1m25s
CI/CD / typecheck (push) Successful in 3m2s
CI/CD / smoke (push) Failing after 1m55s
CI/CD / build (push) Successful in 4m51s
CI/CD / publish (push) Has been skipped
2026-08-15 23:32:46 +01:00
Michal
b022f322f0 fix(mcpd): recover k8s STDIO instances after in-place container restarts
A container restart (OOMKill, crash, transient npx failure) used to strand
the instance in ERROR forever while its pod sat 1/1 Running, because five
gaps lined up (#114):

- the exec/attach websocket died without ending the stdout PassThrough
  (client-node installs no onclose), so PersistentStdioClient — whose only
  death signal was stdout 'end' — kept believing it was connected and every
  request rode the 120s timeout into a dead pipe;
- the stdioClients cache is keyed by pod name, which survives a restart, so
  nothing ever evicted the corpse;
- syncStatus never re-inspected ERROR rows and never read restartCount, so
  neither the recovery nor the in-place restart was visible;
- its ERROR writes clobbered retry metadata, making the row instantly
  dueForRetry, and the retry recreated the pod under the SAME name — an
  uncaught 409 that looped ERROR against a healthy pod;
- the stuck row consumed the whole replica budget, blocking a fresh-id
  replacement.

The fix, layer by layer:
- ws 'close'/'error' now end stdout in both k8s interactive paths (and the
  docker interactive path mirrors its own one-shot handlers), funneling into
  an identity-guarded teardown in PersistentStdioClient that also listens
  for stream 'close'/'error' — a late event from a previous session cannot
  clobber a reconnected one;
- syncStatus re-inspects ERROR rows (pod running again → back to RUNNING
  with retry metadata cleared), tracks restartCount in instance metadata to
  catch restarts between polls, MERGES retry metadata instead of clobbering
  it, and evicts the cached stdio client through a setter-injected hook
  whenever the pipe is known-dead;
- createContainer adopts an alive pod on 409 instead of throwing (and only
  replaces a genuinely dead one, waiting out the deletion grace period);
- attach mode retries once through a fresh client before surfacing an error,
  so the first call after a detected death succeeds;
- the health probe evicts the cached client when failures cross the
  threshold, and shutdown finally calls closeAll().

Closes #114

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
2026-08-15 23:32:23 +01:00
Michal
c66502e590 fix(mcplocal): fetch project instructions with the caller's token (#113)
Some checks failed
CI/CD / typecheck (push) Successful in 1m23s
CI/CD / test (push) Successful in 1m26s
CI/CD / lint (push) Successful in 2m55s
CI/CD / smoke (push) Failing after 1m59s
CI/CD / build (push) Successful in 4m53s
CI/CD / publish (push) Has been skipped
The instructions fetch was the one downstream call in getOrCreateRouter
still using mcpdClient — whose token is an empty string in HTTP mode — so
mcpd answered 401, the catch swallowed it, and every session initialized
with no instructions while nothing looked broken. requestClient exists
precisely for this; use it.

Closes #113

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
2026-08-15 21:43:00 +01:00
Michal
740ce31469 fix(mcplocal): give proxied tools/call a 120s budget, not the 30s default
McpdUpstream.send routed every non-list method through the default-timeout
client, so any tool that legitimately runs past 30s — web_url_read through
a browser solver, a large PDF extraction, a slow retail site — died as
"mcpd proxy error: mcpd did not respond within 30000ms" while mcpd was
still working on it. LibreChat agents hit this constantly on real pages.

New TOOLCALL_TIMEOUT_MS (default 120s, env MCPLOCAL_TOOLCALL_TIMEOUT_MS)
sits between DISCOVERY_TIMEOUT_MS (list calls must stay short so a dead
upstream can't stall session init) and LONG_RUNNING_TIMEOUT_MS (chat and
inference, minutes). withTimeout preserves the caller token and headers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir
2026-08-15 21:43:00 +01:00
b6983f036d merge: stop the deploy script running smoke twice (#112)
Some checks failed
CI/CD / lint (push) Successful in 1m11s
CI/CD / test (push) Successful in 1m24s
CI/CD / typecheck (push) Successful in 3m7s
CI/CD / smoke (push) Failing after 1m57s
CI/CD / build (push) Successful in 2m18s
CI/CD / publish (push) Has been skipped
2026-08-14 22:45:30 +00:00
Michal
21aadf6d82 fix(deploy): stop running the smoke suite twice and crying wolf
Some checks failed
CI/CD / lint (pull_request) Successful in 1m14s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / typecheck (pull_request) Successful in 3m5s
CI/CD / smoke (pull_request) Failing after 1m57s
CI/CD / build (pull_request) Successful in 5m9s
CI/CD / publish (pull_request) Has been skipped
Step 7 called release.sh — which restarts mcplocal and runs the smoke suite
against the binary it just installed — and then restarted mcplocal and ran the
whole suite again. Two full suites inside a minute trips mcpd's rate limiter,
so the second run came back with six 429s and printed

    SMOKE TESTS FAILED — system may be unhealthy. Consider rollback

over a deploy that was fine. Seen for real on 35d506d: the first suite was
162/162 green, the second failed only on `mcpd returned 429: Rate limit
exceeded`, and a clean re-run afterwards was 162/162 again.

A deploy script that recommends rolling back a healthy release is worse than
one that says nothing. One run, one verdict — release.sh's exit status.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU
2026-08-14 23:45:16 +01:00
35d506df77 merge: paginated-result contract usable by any MCP client (#111)
Some checks failed
CI/CD / lint (push) Successful in 1m16s
CI/CD / test (push) Successful in 1m29s
CI/CD / typecheck (push) Successful in 3m4s
CI/CD / smoke (push) Failing after 1m58s
CI/CD / build (push) Successful in 2m18s
CI/CD / publish (push) Has been skipped
2026-08-14 22:28:00 +00:00
Michal
03350856ea fix(mcplocal): make the paginated-result contract usable by any MCP client
Some checks failed
CI/CD / lint (pull_request) Successful in 1m13s
CI/CD / test (pull_request) Successful in 1m25s
CI/CD / typecheck (pull_request) Successful in 3m2s
CI/CD / smoke (pull_request) Failing after 2m5s
CI/CD / build (pull_request) Successful in 5m1s
CI/CD / publish (pull_request) Has been skipped
UniFi tools were unusable from non-Claude agents. A tool result over 2000
chars is replaced with a table of contents and re-read by calling the tool
again with _resultId/_section, but those params were never declared on the
tool's inputSchema — and unifi-network and my-grafana ship
`additionalProperties: false`, so for a client that validates arguments the
drill-down call was illegal and the data unreachable. The stub's own wording
made it worse: "Use section parameter" names a param that does not exist;
sending it fell through to the upstream, re-paginated, and minted a fresh
_resultId. An unbounded loop.

UniFi took the blame because it is the one server whose results always trip
the threshold: get_devices was 11,718 chars, get_clients 136,696 across 19
pages with a 92-char page-1. Grafana and Gitea return compact results.

- content-pipeline gains onToolsList, declaring _resultId/_section on every
  tool it can paginate (gate tools excluded — they are intercepted before the
  pipeline runs). additionalProperties stays false: a property listed in
  `properties` is already legal under it, so declaring is enough and the
  upstream keeps its typo protection.
- createDefaultPlugin wired only the gate's onToolsList, so the pipeline's
  would have been dropped on the floor. Both now chain, gate first.
- _resultId without _section re-shows the table of contents instead of
  forwarding an unknown argument to a strict upstream.
- The stub names the tool, the live _resultId and a real section id.
- Nested MCP envelopes are collapsed. A server fronting another MCP server
  returns the inner result wrapped in its own content/structuredContent pair,
  so the payload arrives two or three times over. A layer is peeled only when
  it carries nothing the inner value lacks, which keeps it lossless. Live
  against the sre project: get_devices 11,718 -> 5,210 chars and no longer
  paginates at all; get_clients 136,696 -> 62,358 across 8 pages instead of 19.
- deploy/mcplocal.service shipped MCPLOCAL_MCPD_URL=http://10.0.0.194:3100,
  which is dead — every working machine had a hand-written drop-in. Points at
  the k8s ingress now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU
2026-08-14 23:27:36 +01:00
29 changed files with 1495 additions and 84 deletions

View File

@@ -7,7 +7,11 @@ Type=simple
ExecStart=/usr/bin/mcpctl-local
Restart=on-failure
RestartSec=5
Environment=MCPLOCAL_MCPD_URL=http://10.0.0.194:3100
# mcpd now runs on Kubernetes behind this ingress. The previous default,
# http://10.0.0.194:3100, is dead — a fresh install pointed at it and every
# machine that worked did so via a hand-written drop-in. Override per host with:
# systemctl --user edit mcplocal -> Environment=MCPLOCAL_MCPD_URL=...
Environment=MCPLOCAL_MCPD_URL=https://mcpctl.ad.itaz.eu
Environment=MCPLOCAL_HTTP_PORT=3200
Environment=MCPLOCAL_HTTP_HOST=127.0.0.1

View File

@@ -187,14 +187,16 @@ trap - ERR
# ── 7. RPM + smoke ──
say "7/7 Build/install CLI RPM + smoke tests"
bash scripts/release.sh
systemctl --user restart mcplocal && sleep 2
if pnpm test:smoke > /tmp/deploy-smoke.log 2>&1; then
grep -E "Tests |passed" /tmp/deploy-smoke.log | tail -2
# release.sh already restarts mcplocal and runs the smoke suite against the
# binary it just installed. This step used to restart and re-run it a second
# time, which put two full suites inside a minute and tripped mcpd's rate
# limiter: the second run failed with 429s and printed a false
# "SMOKE TESTS FAILED — consider rollback" over a perfectly healthy deploy.
# One run, one verdict.
if bash scripts/release.sh; then
say "Deploy complete — $TAG live. Rollback tag: $ROLLBACK_TAG"
else
tail -40 /tmp/deploy-smoke.log
warn "SMOKE TESTS FAILED — system may be unhealthy. Consider rollback:"
warn "RELEASE OR SMOKE TESTS FAILED — system may be unhealthy. Consider rollback:"
rollback_recipe
exit 1
fi

View File

@@ -511,6 +511,10 @@ async function main(): Promise<void> {
const authService = new AuthService(prisma);
const templateService = new TemplateService(templateRepo);
const mcpProxyService = new McpProxyService(instanceRepo, serverRepo, orchestrator);
// When syncStatus observes a container restart/replacement, the cached
// STDIO pipe under that containerId is dead — evict it so the next call
// redials (mcpctl#114). Setter injection, same as setInstanceService above.
instanceService.setStdioInvalidator((cid) => mcpProxyService.removeClient(cid));
const rbacDefinitionService = new RbacDefinitionService(rbacDefinitionRepo);
const rbacService = new RbacService(rbacDefinitionRepo, prisma);
const mcpTokenService = new McpTokenService(mcpTokenRepo, projectRepo, rbacDefinitionRepo, rbacService);
@@ -969,6 +973,7 @@ async function main(): Promise<void> {
healthProbeRunner.stop();
secretBackendRotatorLoop.stop();
gitBackup.stop();
mcpProxyService.closeAll();
await prisma.$disconnect();
},
});

View File

@@ -265,6 +265,16 @@ export class DockerContainerManager implements McpOrchestrator {
const stderr = new PassThrough();
this.docker.modem.demuxStream(stream, stdout, stderr);
// demuxStream never propagates end/close/error to the demuxed streams, so
// a dead container left consumers waiting on a silent pipe — same funnel
// as the one-shot execInContainer path uses (mcpctl#114).
const endStdout = () => {
if (!stdout.destroyed && !stdout.writableEnded) stdout.end();
};
stream.on('end', endStdout);
stream.on('close', endStdout);
stream.on('error', endStdout);
return {
stdout,
write(data: string) {

View File

@@ -193,6 +193,13 @@ export class HealthProbeRunner {
? 'unhealthy'
: 'degraded';
// Crossing the threshold means every probe rode the same cached STDIO
// pipe — evict it once so the next probe/call redials instead of failing
// forever against a connection the other side already dropped.
if (!result.healthy && state.consecutiveFailures === failureThreshold && instance.containerId) {
this.mcpProxyService?.removeClient(instance.containerId);
}
// Build event
const probeLabel = probeKind === 'readiness'
? `Readiness check (${healthCheck.tool})`

View File

@@ -32,6 +32,9 @@ interface RetryMetadata {
attemptCount?: number;
lastAttemptAt?: string;
nextRetryAt?: string;
/** containerStatuses[0].restartCount at last sync — a bump means every
* cached STDIO pipe to this (unchanged) containerId is dead. */
lastRestartCount?: number;
[k: string]: unknown;
}
@@ -52,6 +55,8 @@ export class InvalidStateError extends Error {
}
export class InstanceService {
private stdioInvalidator?: (containerId: string) => void;
constructor(
private instanceRepo: IMcpInstanceRepository,
private serverRepo: IMcpServerRepository,
@@ -59,6 +64,25 @@ export class InstanceService {
private secretResolver?: SecretResolver,
) {}
/**
* Hook for evicting cached STDIO clients (McpProxyService.removeClient).
* Setter injection, matching serverService.setInstanceService in main.ts —
* McpProxyService is constructed after this service and already imports
* from this file, so a constructor arg would be a circular import.
*/
setStdioInvalidator(fn: (containerId: string) => void): void {
this.stdioInvalidator = fn;
}
private invalidateStdio(containerId: string | null | undefined): void {
if (!containerId) return;
try {
this.stdioInvalidator?.(containerId);
} catch {
/* best-effort */
}
}
async list(serverId?: string): Promise<McpInstance[]> {
return this.instanceRepo.findAll(serverId);
}
@@ -71,14 +95,60 @@ export class InstanceService {
/**
* Sync instance statuses with actual container state.
* Detects crashed/stopped containers and marks them ERROR.
*
* Beyond marking crashed containers ERROR, this is the recovery path for
* mcpctl#114: ERROR rows whose pod came back are re-adopted (the pod has
* restartPolicy Always, so kubelet restarts the container in place and the
* row must follow it back), and an in-place restart under an unchanged
* containerId — visible only as a restartCount bump — invalidates any
* cached STDIO pipe, which is dead by definition.
*
* Every metadata write here MERGES via readRetryMeta: updateStatus replaces
* the JSON column wholesale, and clobbering nextRetryAt is what used to
* make ERROR rows instantly dueForRetry and hot-loop against a 409.
*/
async syncStatus(): Promise<void> {
const instances = await this.instanceRepo.findAll();
for (const inst of instances) {
if ((inst.status === 'RUNNING' || inst.status === 'STARTING') && inst.containerId) {
if (!inst.containerId) continue;
if (inst.status !== 'RUNNING' && inst.status !== 'STARTING' && inst.status !== 'ERROR') {
continue;
}
let info: ContainerInfo;
try {
const info = await this.orchestrator.inspectContainer(inst.containerId);
info = await this.orchestrator.inspectContainer(inst.containerId);
} catch {
// Container gone entirely. ERROR rows with a missing pod stay as they
// are — the retry/backoff path owns recreating them.
if (inst.status !== 'ERROR') {
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
metadata: { ...readRetryMeta(inst), error: 'Container not found' },
});
this.invalidateStdio(inst.containerId);
}
continue;
}
const meta = readRetryMeta(inst);
if (inst.status === 'ERROR') {
// The pod outlived the ERROR verdict (kubelet restarted the container
// in place). Re-adopt instead of leaving the row stuck forever.
if (info.state === 'running') {
const { error: _e, attemptCount: _a, lastAttemptAt: _l, nextRetryAt: _n, ...rest } = meta;
await this.instanceRepo.updateStatus(inst.id, 'RUNNING', {
metadata: { ...rest, lastRestartCount: info.restartCount ?? 0 },
});
this.invalidateStdio(inst.containerId);
} else if (info.state === 'starting') {
// Keep retry metadata until it is actually running.
await this.instanceRepo.updateStatus(inst.id, 'STARTING', { metadata: meta });
this.invalidateStdio(inst.containerId);
}
// stopped/error: leave for the backoff/retry path.
continue;
}
if (info.state === 'stopped' || info.state === 'error') {
// Container died — get last logs for error context
@@ -89,19 +159,32 @@ export class InstanceService {
if (lastLog) errorMsg = lastLog;
} catch { /* best-effort */ }
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
metadata: { error: errorMsg },
metadata: { ...meta, error: errorMsg },
});
this.invalidateStdio(inst.containerId);
} else if (info.state === 'starting' && inst.status === 'RUNNING') {
// Pod went back to starting (e.g. CrashLoopBackOff restart)
await this.instanceRepo.updateStatus(inst.id, 'STARTING', {});
await this.instanceRepo.updateStatus(inst.id, 'STARTING', { metadata: meta });
this.invalidateStdio(inst.containerId);
} else if (info.state === 'running' && inst.status === 'STARTING') {
// Pod became ready — promote to RUNNING
await this.instanceRepo.updateStatus(inst.id, 'RUNNING', {});
}
} catch {
// Container gone entirely
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
metadata: { error: 'Container not found' },
// Pod became ready — promote to RUNNING and clear retry state.
const { error: _e, attemptCount: _a, lastAttemptAt: _l, nextRetryAt: _n, ...rest } = meta;
await this.instanceRepo.updateStatus(inst.id, 'RUNNING', {
metadata: { ...rest, lastRestartCount: info.restartCount ?? 0 },
});
// A fresh/restarted pod under this name invalidates any cached pipe.
this.invalidateStdio(inst.containerId);
} else if (info.state === 'running' && info.restartCount !== undefined) {
if (typeof meta.lastRestartCount === 'number' && info.restartCount > meta.lastRestartCount) {
// In-place restart between polls: same pod name, dead pipes.
await this.instanceRepo.updateStatus(inst.id, 'RUNNING', {
metadata: { ...meta, lastRestartCount: info.restartCount },
});
this.invalidateStdio(inst.containerId);
} else if (meta.lastRestartCount === undefined) {
// First sighting: record the baseline without invalidating.
await this.instanceRepo.updateStatus(inst.id, 'RUNNING', {
metadata: { ...meta, lastRestartCount: info.restartCount },
});
}
}
@@ -232,6 +315,7 @@ export class InstanceService {
} catch {
// Container may already be gone
}
this.invalidateStdio(instance.containerId);
}
await this.instanceRepo.delete(id);
@@ -256,6 +340,7 @@ export class InstanceService {
} catch {
// best-effort
}
this.invalidateStdio(inst.containerId);
}
}
}
@@ -489,6 +574,7 @@ export class InstanceService {
try {
await this.orchestrator.removeContainer(instance.containerId, true);
} catch { /* best-effort */ }
this.invalidateStdio(instance.containerId);
}
await this.instanceRepo.delete(instance.id);
}

View File

@@ -81,6 +81,18 @@ function podToContainerInfo(pod: V1Pod): ContainerInfo {
info.port = ports[0].containerPort;
}
// Restart visibility: a container that crashes and restarts IN PLACE keeps
// the same pod name and reports state=running again — restartCount is the
// only durable evidence, and syncStatus uses it to invalidate stale STDIO
// pipes (mcpctl#114).
const cs = pod.status?.containerStatuses?.[0];
if (cs?.restartCount !== undefined) {
info.restartCount = cs.restartCount;
}
if (cs?.state?.running?.startedAt) {
info.startedAt = new Date(cs.state.running.startedAt as unknown as string);
}
return info;
}
@@ -116,10 +128,39 @@ export class KubernetesOrchestrator implements McpOrchestrator {
}
const manifest = generatePodSpec(spec, this.namespace);
const pod = await this.client.core.createNamespacedPod({
let pod;
try {
pod = await this.client.core.createNamespacedPod({
namespace: this.namespace,
body: manifest as V1Pod,
});
} catch (err) {
if (httpStatusOf(err) !== 409) throw err;
// AlreadyExists: a retry raced a pod that is still there. If it is
// alive, ADOPT it — recreating under the same name can never succeed
// and used to loop the instance in ERROR forever (mcpctl#114). Only a
// genuinely dead pod is replaced.
const podName = (manifest as V1Pod).metadata!.name!;
const existing = await this.inspectContainer(podName);
if (existing.state === 'running' || existing.state === 'starting') {
return existing;
}
await this.removeContainer(podName, true);
// deleteNamespacedPod returns before the pod is gone (grace period);
// recreating while it is Terminating would 409 again. Bounded wait.
for (let i = 0; i < 15; i++) {
await new Promise((resolve) => setTimeout(resolve, 1000));
try {
await this.inspectContainer(podName);
} catch {
break; // 404 — pod is gone
}
}
pod = await this.client.core.createNamespacedPod({
namespace: this.namespace,
body: manifest as V1Pod,
});
}
// Wait briefly for pod to start scheduling
await new Promise((resolve) => setTimeout(resolve, 500));
@@ -276,6 +317,17 @@ export class KubernetesOrchestrator implements McpOrchestrator {
// Wait for WebSocket connection to establish
const ws = await wsPromise;
// client-node's WebSocketHandler only ends stdout on an explicit
// CloseStream/Status frame from the apiserver. An abnormal socket death
// (container OOMKilled, node reboot) emits neither — without these
// handlers the PassThrough stays open and the consumer never learns the
// pipe is dead (mcpctl#114).
const endStdout = () => {
if (!stdout.destroyed && !stdout.writableEnded) stdout.end();
};
ws.on('close', endStdout);
ws.on('error', endStdout);
return {
stdout,
write(data: string) {
@@ -316,6 +368,13 @@ export class KubernetesOrchestrator implements McpOrchestrator {
false, // tty
);
// Same abnormal-death funnel as execInteractive — see comment there.
const endStdout = () => {
if (!stdout.destroyed && !stdout.writableEnded) stdout.end();
};
ws.on('close', endStdout);
ws.on('error', endStdout);
return {
stdout,
write(data: string) {

View File

@@ -180,12 +180,19 @@ export class McpProxyService {
} catch (err) {
this.removeClient(instance.containerId);
// Fall back to one-shot exec when we have a command to run.
// Attach mode has no equivalent one-shot fallback — surface the error.
if (mode.kind === 'exec') {
return sendViaStdio(this.orchestrator, instance.containerId, packageName, method, params, 120_000, command, runtime);
}
const detail = formatError(err);
console.error(`[mcp-proxy] attach to ${instance.containerId} failed:`, err);
// Attach mode has no one-shot equivalent, but the failure is usually
// a stale pipe from an in-place container restart — retry once
// through a fresh client (which redials) before surfacing the error,
// so the FIRST call after a detected death succeeds (mcpctl#114).
try {
return await this.sendViaPersistentStdio(instance.containerId, mode, method, params);
} catch (retryErr) {
this.removeClient(instance.containerId);
const detail = formatError(retryErr);
console.error(`[mcp-proxy] attach to ${instance.containerId} failed (after retry):`, retryErr);
return {
jsonrpc: '2.0',
id: 1,
@@ -196,6 +203,7 @@ export class McpProxyService {
};
}
}
}
// SSE or STREAMABLE_HTTP: need a base URL
const baseUrl = await this.resolveBaseUrl(instance, server);

View File

@@ -69,6 +69,14 @@ export interface ContainerInfo {
/** Container IP on the first non-default network (for internal communication) */
ip?: string;
createdAt: Date;
/**
* Times the container restarted in place (k8s containerStatuses[0]).
* A bump with an unchanged containerId means every cached STDIO pipe to it
* is dead — syncStatus uses this to invalidate them (mcpctl#114).
*/
restartCount?: number;
/** When the current container process started (k8s state.running.startedAt). */
startedAt?: Date;
}
export interface ContainerLogs {

View File

@@ -121,15 +121,15 @@ export class PersistentStdioClient {
this.processBuffer();
});
exec.stdout.on('end', () => {
this.initialized = false;
this.exec = null;
for (const [, pending] of this.pendingRequests) {
clearTimeout(pending.timer);
pending.reject(new Error('STDIO process exited'));
}
this.pendingRequests.clear();
});
// All three events funnel into the same teardown: 'end' is the graceful
// path, but an abnormal websocket death may only surface as 'close' or
// 'error' on the stream — before this, isConnected stayed true and every
// request rode out the full timeout against a dead pipe (mcpctl#114).
exec.stdout.on('end', () => this.teardown(exec, 'STDIO process exited'));
exec.stdout.on('close', () => this.teardown(exec, 'STDIO stream closed'));
exec.stdout.on('error', (err: Error) =>
this.teardown(exec, `STDIO stream error: ${err.message}`),
);
// Run MCP init handshake
const initId = this.nextId++;
@@ -174,6 +174,23 @@ export class PersistentStdioClient {
this.initialized = true;
}
/**
* Tear down a dead session: reject in-flight requests and mark the client
* disconnected so the next send() redials via ensureReady(). The identity
* guard is load-bearing: after a reconnect, a LATE 'close'/'end' from the
* previous session's stream must not clobber the new session.
*/
private teardown(exec: InteractiveExec, reason: string): void {
if (this.exec !== exec) return;
this.initialized = false;
this.exec = null;
for (const [, pending] of this.pendingRequests) {
clearTimeout(pending.timer);
pending.reject(new Error(reason));
}
this.pendingRequests.clear();
}
private write(msg: Record<string, unknown>): void {
if (!this.exec) throw new Error('Not connected');
this.exec.write(JSON.stringify(msg) + '\n');

View File

@@ -511,3 +511,130 @@ describe('InstanceService', () => {
});
});
});
// ── syncStatus (mcpctl#114) — first-ever coverage of this method ──
describe('syncStatus', () => {
let instanceRepo: IMcpInstanceRepository;
let serverRepo: IMcpServerRepository;
let orchestrator: McpOrchestrator;
let service: InstanceService;
let invalidator: ReturnType<typeof vi.fn>;
function inspect(info: Partial<{ state: string; restartCount: number }>) {
vi.mocked(orchestrator.inspectContainer).mockResolvedValue({
containerId: 'ctr-abc',
name: 'test',
state: (info.state ?? 'running') as 'running',
createdAt: new Date(),
...(info.restartCount !== undefined ? { restartCount: info.restartCount } : {}),
});
}
beforeEach(() => {
instanceRepo = mockInstanceRepo();
serverRepo = mockServerRepo();
orchestrator = mockOrchestrator();
service = new InstanceService(instanceRepo, serverRepo, orchestrator);
invalidator = vi.fn();
service.setStdioInvalidator(invalidator);
});
it('marks a stopped container ERROR and MERGES retry metadata instead of clobbering it', async () => {
const future = new Date(Date.now() + 60_000).toISOString();
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({ status: 'RUNNING', metadata: { attemptCount: 3, nextRetryAt: future } }),
]);
inspect({ state: 'stopped' });
await service.syncStatus();
const [, status, fields] = vi.mocked(instanceRepo.updateStatus).mock.calls[0]!;
expect(status).toBe('ERROR');
const meta = fields!.metadata as Record<string, unknown>;
expect(meta['error']).toBe('log output'); // last log line
expect(meta['attemptCount']).toBe(3); // preserved — was clobbered before
expect(meta['nextRetryAt']).toBe(future); // preserved — hot-loop killer
expect(invalidator).toHaveBeenCalledWith('ctr-abc');
});
it('recovers an ERROR instance whose pod is running again (the stuck state)', async () => {
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({
status: 'ERROR',
metadata: { error: 'Container stopped', attemptCount: 4, nextRetryAt: 'x', lastAttemptAt: 'y' },
}),
]);
inspect({ state: 'running', restartCount: 2 });
await service.syncStatus();
const [, status, fields] = vi.mocked(instanceRepo.updateStatus).mock.calls[0]!;
expect(status).toBe('RUNNING');
const meta = fields!.metadata as Record<string, unknown>;
expect(meta['error']).toBeUndefined();
expect(meta['attemptCount']).toBeUndefined();
expect(meta['nextRetryAt']).toBeUndefined();
expect(meta['lastRestartCount']).toBe(2);
expect(invalidator).toHaveBeenCalledWith('ctr-abc');
});
it('leaves an ERROR instance alone when its pod is genuinely gone', async () => {
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({ status: 'ERROR', metadata: { error: 'x' } }),
]);
vi.mocked(orchestrator.inspectContainer).mockRejectedValue(new Error('not found'));
await service.syncStatus();
expect(instanceRepo.updateStatus).not.toHaveBeenCalled();
expect(invalidator).not.toHaveBeenCalled();
});
it('detects an in-place restart via restartCount and invalidates the stdio pipe', async () => {
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({ status: 'RUNNING', metadata: { lastRestartCount: 1 } }),
]);
inspect({ state: 'running', restartCount: 2 });
await service.syncStatus();
const [, status, fields] = vi.mocked(instanceRepo.updateStatus).mock.calls[0]!;
expect(status).toBe('RUNNING');
expect((fields!.metadata as Record<string, unknown>)['lastRestartCount']).toBe(2);
expect(invalidator).toHaveBeenCalledWith('ctr-abc');
});
it('baselines a first-seen restartCount without invalidating', async () => {
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({ status: 'RUNNING', metadata: {} }),
]);
inspect({ state: 'running', restartCount: 0 });
await service.syncStatus();
const [, , fields] = vi.mocked(instanceRepo.updateStatus).mock.calls[0]!;
expect((fields!.metadata as Record<string, unknown>)['lastRestartCount']).toBe(0);
expect(invalidator).not.toHaveBeenCalled();
});
it('does nothing for a steady RUNNING instance with an unchanged restartCount', async () => {
vi.mocked(instanceRepo.findAll).mockResolvedValue([
makeInstance({ status: 'RUNNING', metadata: { lastRestartCount: 2 } }),
]);
inspect({ state: 'running', restartCount: 2 });
await service.syncStatus();
expect(instanceRepo.updateStatus).not.toHaveBeenCalled();
expect(invalidator).not.toHaveBeenCalled();
});
it('remove() evicts the cached stdio client', async () => {
vi.mocked(instanceRepo.findById).mockResolvedValue(makeInstance({ status: 'RUNNING' }));
await service.remove('inst-1');
expect(invalidator).toHaveBeenCalledWith('ctr-abc');
});
});

View File

@@ -63,12 +63,22 @@ vi.mock('@kubernetes/client-node', () => {
makeApiClient = vi.fn(() => mockCore);
}
// Track the live Exec/Attach instances so tests can program their vi.fn()s
// (each K8sOfficialClient constructs fresh ones).
const instances: { exec?: { exec: ReturnType<typeof vi.fn> }; attach?: { attach: ReturnType<typeof vi.fn> } } = {};
class MockExec {
exec = vi.fn();
constructor() {
instances.exec = this;
}
}
class MockAttach {
attach = vi.fn();
constructor() {
instances.attach = this;
}
}
class MockLog {
@@ -82,7 +92,7 @@ vi.mock('@kubernetes/client-node', () => {
Attach: MockAttach,
Log: MockLog,
// Export test helpers
__testHelpers: { setHandler, getHandler, clearHandlers, mockCore },
__testHelpers: { setHandler, getHandler, clearHandlers, mockCore, instances },
};
});
@@ -379,3 +389,134 @@ describe('httpStatusOf', () => {
expect(httpStatusOf({ code: 'ECONNREFUSED' })).toBeUndefined();
});
});
// ── mcpctl#114: interactive-session death signals, restartCount, 409-adopt ──
import { EventEmitter } from 'node:events';
function makeFakeWs(): EventEmitter & { close: ReturnType<typeof vi.fn> } {
const ws = new EventEmitter() as EventEmitter & { close: ReturnType<typeof vi.fn> };
ws.close = vi.fn();
return ws;
}
const podRestarted = {
...podRunning,
status: {
...podRunning.status,
containerStatuses: [{
state: { running: { startedAt: '2026-01-02T00:00:00Z' } },
restartCount: 3,
lastState: { terminated: { reason: 'OOMKilled', exitCode: 137 } },
}],
},
};
const podTerminated = {
...podRunning,
status: {
phase: 'Failed',
containerStatuses: [{
state: { terminated: { reason: 'Error', exitCode: 1 } },
restartCount: 1,
}],
},
};
describe('KubernetesOrchestrator interactive sessions (mcpctl#114)', () => {
let orch: KubernetesOrchestrator;
beforeEach(() => {
clearHandlers();
vi.clearAllMocks();
orch = new KubernetesOrchestrator({ serversNamespace: 'mcpctl-servers' });
});
it("execInteractive: ws 'close' ends stdout so the consumer sees the death", async () => {
setHandler('readNamespacedPod:my-server', podRunning);
const ws = makeFakeWs();
k8sMock.__testHelpers.instances.exec!.exec.mockResolvedValue(ws);
const iexec = await orch.execInteractive!('my-server', ['node', 'index.js']);
iexec.stdout.resume(); // PassThrough only emits 'end' once it is being read
const ended = new Promise<void>((resolve) => iexec.stdout.on('end', resolve));
ws.emit('close');
await expect(ended).resolves.toBeUndefined();
});
it("execInteractive: ws 'error' also ends stdout, and close() closes the ws", async () => {
setHandler('readNamespacedPod:my-server', podRunning);
const ws = makeFakeWs();
k8sMock.__testHelpers.instances.exec!.exec.mockResolvedValue(ws);
const iexec = await orch.execInteractive!('my-server', ['node']);
iexec.stdout.resume();
const ended = new Promise<void>((resolve) => iexec.stdout.on('end', resolve));
ws.emit('error', new Error('abnormal closure'));
await expect(ended).resolves.toBeUndefined();
iexec.close();
expect(ws.close).toHaveBeenCalled();
});
it("attachInteractive: ws 'close' ends stdout (first-ever attach coverage)", async () => {
setHandler('readNamespacedPod:my-server', podRunning);
const ws = makeFakeWs();
k8sMock.__testHelpers.instances.attach!.attach.mockResolvedValue(ws);
const iexec = await orch.attachInteractive!('my-server');
iexec.stdout.resume();
const ended = new Promise<void>((resolve) => iexec.stdout.on('end', resolve));
ws.emit('close');
await expect(ended).resolves.toBeUndefined();
expect(k8sMock.__testHelpers.instances.attach!.attach).toHaveBeenCalledOnce();
});
it('inspectContainer surfaces restartCount and startedAt', async () => {
setHandler('readNamespacedPod:my-server', podRestarted);
const info = await orch.inspectContainer('my-server');
expect(info.state).toBe('running');
expect(info.restartCount).toBe(3);
expect(info.startedAt).toEqual(new Date('2026-01-02T00:00:00Z'));
});
it('createContainer ADOPTS an alive pod on 409 instead of throwing', async () => {
setHandler('readNamespace:mcpctl-servers', {});
setHandler('createNamespacedPod', undefined, { code: 409 });
setHandler('readNamespacedPod:my-server', podRestarted);
const info = await orch.createContainer(testSpec);
expect(info.containerId).toBe('my-server');
expect(info.state).toBe('running');
expect(mockCore.deleteNamespacedPod).not.toHaveBeenCalled();
});
it('createContainer replaces a genuinely dead pod on 409', async () => {
setHandler('readNamespace:mcpctl-servers', {});
setHandler('createNamespacedPod', undefined, { code: 409 });
setHandler('readNamespacedPod:my-server', podTerminated);
setHandler('deleteNamespacedPod:my-server', {});
const promise = orch.createContainer(testSpec);
// Give it a tick to inspect + delete, then flip the world: pod is gone,
// create succeeds.
await new Promise((r) => setTimeout(r, 50));
// The gone-wait loop polls once per second: serve 404 so it breaks, then
// restore the pod before the post-create inspect (~500ms later) runs.
setHandler('readNamespacedPod:my-server', undefined, { code: 404 });
setHandler('createNamespacedPod', podRunning);
setTimeout(() => setHandler('readNamespacedPod:my-server', podRunning), 1200);
const info = await promise;
expect(info.state).toBe('running');
expect(mockCore.deleteNamespacedPod).toHaveBeenCalled();
expect(mockCore.createNamespacedPod).toHaveBeenCalledTimes(2);
});
it('createContainer still throws on non-409 errors', async () => {
setHandler('readNamespace:mcpctl-servers', {});
setHandler('createNamespacedPod', undefined, { code: 500 });
await expect(orch.createContainer(testSpec)).rejects.toBeTruthy();
});
});

View File

@@ -725,3 +725,103 @@ describe('MCP server full flow', () => {
});
});
});
// ── mcpctl#114: attach server survives an in-place container restart ──
import { PassThrough } from 'node:stream';
import type { InteractiveExec } from '../src/services/orchestrator.js';
/**
* A scripted MCP server on the other end of an attach session: answers
* initialize and every request with a result carrying its session number,
* so the test can prove which session served which call.
*/
function makeScriptedAttach(session: number): InteractiveExec {
const stdout = new PassThrough();
return {
stdout,
write(data: string) {
for (const line of data.split('\n')) {
if (!line.trim()) continue;
const msg = JSON.parse(line) as { id?: number; method?: string };
if (msg.id === undefined) continue; // notifications
const result = msg.method === 'initialize'
? { capabilities: {} }
: { ok: true, session };
stdout.write(JSON.stringify({ jsonrpc: '2.0', id: msg.id, result }) + '\n');
}
},
close() {
stdout.destroy();
},
};
}
describe('attach-mode STDIO restart recovery (mcpctl#114)', () => {
it('survives an in-place container restart: syncStatus invalidates, next call rides a new session', async () => {
const serverRepo = createInMemoryServerRepo();
const instanceRepo = createInMemoryInstanceRepo();
const server = await serverRepo.create({
name: 'gitea',
transport: 'STDIO',
dockerImage: 'ghcr.io/gitea-mcp:latest',
replicas: 1,
} as never);
const instance = await instanceRepo.create({
serverId: server.id,
containerId: 'pod-gitea',
status: 'RUNNING',
metadata: { lastRestartCount: 0 },
} as never);
let restartCount = 0;
const sessions: InteractiveExec[] = [];
const orchestrator = {
ping: vi.fn(async () => true),
pullImage: vi.fn(async () => {}),
createContainer: vi.fn(),
stopContainer: vi.fn(async () => {}),
removeContainer: vi.fn(async () => {}),
inspectContainer: vi.fn(async () => ({
containerId: 'pod-gitea',
name: 'pod-gitea',
state: 'running' as const,
createdAt: new Date(),
restartCount,
})),
getContainerLogs: vi.fn(async () => ({ stdout: '', stderr: '' })),
execInContainer: vi.fn(),
attachInteractive: vi.fn(async () => {
const s = makeScriptedAttach(sessions.length + 1);
sessions.push(s);
return s;
}),
} as unknown as McpOrchestrator;
const proxyService = new McpProxyService(instanceRepo, serverRepo, orchestrator);
const instanceService = new InstanceService(instanceRepo, serverRepo, orchestrator);
instanceService.setStdioInvalidator((cid) => proxyService.removeClient(cid));
// 1. A call round-trips on session 1.
const first = await proxyService.execute({ serverId: server.id, method: 'tools/list' });
expect((first.result as { session: number }).session).toBe(1);
// 2. The container crashes and kubelet restarts it in place: same pod
// name, restartCount bumps, the old session's pipe dies.
sessions[0]!.stdout.end();
restartCount = 1;
// 3. One reconcile tick detects the bump, keeps the row RUNNING, and
// evicts the stale client.
await instanceService.syncStatus();
const after = await instanceRepo.findById(instance.id);
expect(after!.status).toBe('RUNNING');
expect((after!.metadata as { lastRestartCount: number }).lastRestartCount).toBe(1);
// 4. The next call succeeds first-try on a fresh attach session.
const second = await proxyService.execute({ serverId: server.id, method: 'tools/list' });
expect((second.result as { session: number }).session).toBe(2);
expect(orchestrator.attachInteractive).toHaveBeenCalledTimes(2);
});
});

View File

@@ -109,3 +109,110 @@ describe('PersistentStdioClient', () => {
await expect(client.send('tools/list')).rejects.toThrow(/interactive exec/i);
});
});
// Drive a fake session through the init handshake so send() can round-trip.
async function completeHandshake(fake: ReturnType<typeof makeFakeExec>): Promise<void> {
await new Promise((r) => setTimeout(r, 10));
const init = JSON.parse(fake.written[0]!);
fake.emit({ jsonrpc: '2.0', id: init.id, result: { capabilities: {} } });
await new Promise((r) => setTimeout(r, 150));
}
describe('PersistentStdioClient teardown & reconnect (mcpctl#114)', () => {
it("stdout 'end' rejects in-flight requests and the next send() redials", async () => {
const fakes = [makeFakeExec(), makeFakeExec()];
let call = 0;
const execInteractive = vi.fn(async () => fakes[call++]!.iexec);
const client = new PersistentStdioClient(
makeOrchestrator({ execInteractive }),
'c1',
{ kind: 'exec', command: ['node'] },
);
const first = client.send('tools/list');
await completeHandshake(fakes[0]!);
// Kill the session while tools/list is in flight.
fakes[0]!.iexec.stdout.end();
await expect(first).rejects.toThrow('STDIO process exited');
expect(client.isConnected).toBe(false);
// Next send must redial through a fresh interactive session.
const second = client.send('tools/list');
await completeHandshake(fakes[1]!);
const req = JSON.parse(fakes[1]!.written[2]!);
fakes[1]!.emit({ jsonrpc: '2.0', id: req.id, result: { tools: [] } });
const res = await second;
expect(res.result).toEqual({ tools: [] });
expect(execInteractive).toHaveBeenCalledTimes(2);
client.close();
});
it("stdout 'error' tears down with the error message", async () => {
const fake = makeFakeExec();
const client = new PersistentStdioClient(
makeOrchestrator({ execInteractive: vi.fn(async () => fake.iexec) }),
'c1',
{ kind: 'exec', command: ['node'] },
);
const pending = client.send('tools/list');
await completeHandshake(fake);
fake.iexec.stdout.emit('error', new Error('socket hang up'));
await expect(pending).rejects.toThrow('STDIO stream error: socket hang up');
expect(client.isConnected).toBe(false);
client.close();
});
it("a LATE 'end' from the previous session does not clobber the reconnected one", async () => {
const fakes = [makeFakeExec(), makeFakeExec()];
let call = 0;
const client = new PersistentStdioClient(
makeOrchestrator({ attachInteractive: vi.fn(async () => fakes[call++]!.iexec) }),
'c1',
{ kind: 'attach' },
);
const first = client.send('tools/list');
await completeHandshake(fakes[0]!);
fakes[0]!.iexec.stdout.end();
await expect(first).rejects.toThrow('STDIO process exited');
// Reconnect on a fresh session.
const second = client.send('tools/list');
await completeHandshake(fakes[1]!);
// The old session's stream fires a stale 'close' AFTER the reconnect —
// the identity guard must ignore it.
fakes[0]!.iexec.stdout.emit('close');
expect(client.isConnected).toBe(true);
const req = JSON.parse(fakes[1]!.written[2]!);
fakes[1]!.emit({ jsonrpc: '2.0', id: req.id, result: { tools: [{ name: 't' }] } });
const res = await second;
expect((res.result as { tools: unknown[] }).tools).toHaveLength(1);
client.close();
});
it('attach mode also redials after teardown', async () => {
const fakes = [makeFakeExec(), makeFakeExec()];
let call = 0;
const attachInteractive = vi.fn(async () => fakes[call++]!.iexec);
const client = new PersistentStdioClient(
makeOrchestrator({ attachInteractive }),
'c-gitea',
{ kind: 'attach' },
);
const first = client.send('tools/list');
await completeHandshake(fakes[0]!);
fakes[0]!.iexec.stdout.emit('close');
await expect(first).rejects.toThrow('STDIO stream closed');
const second = client.send('tools/list');
await completeHandshake(fakes[1]!);
const req = JSON.parse(fakes[1]!.written[2]!);
fakes[1]!.emit({ jsonrpc: '2.0', id: req.id, result: { tools: [] } });
await expect(second).resolves.toMatchObject({ result: { tools: [] } });
expect(attachInteractive).toHaveBeenCalledTimes(2);
client.close();
});
});

View File

@@ -544,3 +544,32 @@ describe('HealthProbeRunner', () => {
expect(result.message).toBe('ECONNREFUSED 10.0.0.1:3000');
});
});
describe('HealthProbeRunner stale-pipe eviction (mcpctl#114)', () => {
it('evicts the cached stdio client exactly once, when failures cross the threshold', async () => {
const instanceRepo = mockInstanceRepo();
const serverRepo = mockServerRepo();
const orchestrator = mockOrchestrator();
const mcpProxyService = mockMcpProxyService();
const runner = new HealthProbeRunner(instanceRepo, serverRepo, orchestrator, undefined, mcpProxyService);
const instance = makeInstance();
// intervalSeconds: 0 so every tick actually probes; failureThreshold: 3.
const server = makeServer({
healthCheck: { tool: 'list_datasources', arguments: {}, intervalSeconds: 0, timeoutSeconds: 10, failureThreshold: 3 },
} as Partial<McpServer>);
vi.mocked(instanceRepo.findAll).mockResolvedValue([instance]);
vi.mocked(serverRepo.findById).mockResolvedValue(server);
vi.mocked(mcpProxyService.execute).mockRejectedValue(new Error('pipe is dead'));
await runner.tick(); // failure 1 — degraded
await runner.tick(); // failure 2 — degraded
expect(mcpProxyService.removeClient).not.toHaveBeenCalled();
await runner.tick(); // failure 3 — crosses threshold → evict once
expect(mcpProxyService.removeClient).toHaveBeenCalledExactlyOnceWith('container-abc');
await runner.tick(); // failure 4 — already past threshold, no re-evict
expect(mcpProxyService.removeClient).toHaveBeenCalledTimes(1);
});
});

View File

@@ -1,5 +1,5 @@
import type { McpdClient } from './http/mcpd-client.js';
import { DISCOVERY_TIMEOUT_MS } from './http/mcpd-client.js';
import { DISCOVERY_TIMEOUT_MS, TOOLCALL_TIMEOUT_MS } from './http/mcpd-client.js';
import type { McpRouter } from './router.js';
import { McpdUpstream } from './upstream/mcpd.js';
@@ -152,6 +152,10 @@ function syncUpstreams(router: McpRouter, mcpdClient: McpdClient, servers: McpdS
// unreachable upstream cannot stall session init for the full tool-call window.
const discoveryClient = mcpdClient.withTimeout(DISCOVERY_TIMEOUT_MS);
// Everything else an upstream receives is a proxied tools/call, which can
// legitimately run past the 30s default (browser solvers, PDF extraction).
const toolClient = mcpdClient.withTimeout(TOOLCALL_TIMEOUT_MS);
// Remove stale upstreams
const currentNames = new Set(router.getUpstreamNames());
const serverNames = new Set(servers.map((s) => s.name));
@@ -164,7 +168,7 @@ function syncUpstreams(router: McpRouter, mcpdClient: McpdClient, servers: McpdS
// Add/update upstreams for each server
for (const server of servers) {
if (!currentNames.has(server.name)) {
const upstream = new McpdUpstream(server.id, server.name, mcpdClient, server.description, discoveryClient);
const upstream = new McpdUpstream(server.id, server.name, toolClient, server.description, discoveryClient);
router.addUpstream(upstream);
}
registered.push(server.name);

View File

@@ -62,6 +62,17 @@ export const LONG_RUNNING_TIMEOUT_MS = Number(process.env['MCPLOCAL_LONG_TIMEOUT
*/
export const DISCOVERY_TIMEOUT_MS = Number(process.env['MCPLOCAL_DISCOVERY_TIMEOUT_MS']) || 8_000;
/**
* Budget for proxied tools/call. The 30s DEFAULT_TIMEOUT_MS is a guaranteed
* failure for tools that legitimately run long — web_url_read through a
* browser solver, a large PDF extraction, a slow retail site — all of which
* died as `mcpd proxy error: mcpd did not respond within 30000ms` while mcpd
* was still working. Distinct from LONG_RUNNING_TIMEOUT_MS (chat/inference,
* minutes) and DISCOVERY_TIMEOUT_MS (list calls, must stay short so a dead
* upstream cannot stall session init). Override via `MCPLOCAL_TOOLCALL_TIMEOUT_MS`.
*/
export const TOOLCALL_TIMEOUT_MS = Number(process.env['MCPLOCAL_TOOLCALL_TIMEOUT_MS']) || 120_000;
export class McpdClient {
private readonly baseUrl: string;
private readonly token: string;

View File

@@ -176,9 +176,13 @@ export function registerProjectMcpEndpoint(app: FastifyInstance, mcpdClient: Mcp
chain.push(createAgentsPlugin());
router.setPlugin(composePlugins(chain));
// Fetch project instructions and set on router
// Fetch project instructions and set on router. Must ride the CALLER's
// token (requestClient) like every other downstream call here: in HTTP
// mode mcpdClient's own token is empty, mcpd answers 401, and the catch
// below swallowed it — every session initialized with no instructions
// while nothing looked broken (#113).
try {
const instructions = await mcpdClient.get<{ prompt: string; servers: Array<{ name: string; description: string }> }>(
const instructions = await requestClient.get<{ prompt: string; servers: Array<{ name: string; description: string }> }>(
`/api/v1/projects/${encodeURIComponent(projectName)}/instructions`,
);
const parts: string[] = [];

View File

@@ -7,20 +7,37 @@
* - sectionStore management
*
* This plugin handles:
* 1. onToolCallBefore: intercept section drill-down requests (_resultId + _section params)
* 2. onToolCallAfter: run tool results through the proxymodel pipeline
* 1. onToolsList: declare the drill-down params on every tool it may paginate
* 2. onToolCallBefore: intercept section drill-down requests (_resultId + _section params)
* 3. onToolCallAfter: run tool results through the proxymodel pipeline
*/
import type { JsonRpcRequest, JsonRpcResponse } from '../../types.js';
import type { Section } from '../types.js';
import type { Section, ToolDefinition } from '../types.js';
import type { ProxyModelPlugin, PluginSessionContext } from '../plugin.js';
const SECTION_STORE_TTL_MS = 300_000; // 5 minutes
/**
* Tools owned by the gate plugin. Their calls are intercepted in
* onToolCallBefore and returned before onToolCallAfter ever runs, so their
* results never paginate and they must not advertise drill-down params.
*/
const GATE_TOOLS = new Set(['begin_session', 'read_prompts', 'propose_prompt', 'propose_skill']);
export function createContentPipelinePlugin(): ProxyModelPlugin {
return {
name: 'content-pipeline',
description: 'Content transformation pipeline: paginate, section-split, summarize tool results.',
async onToolsList(tools): Promise<ToolDefinition[]> {
// The drill-down params are part of this plugin's contract with the
// client, so they belong in the advertised schema. Without them a client
// that validates arguments against inputSchema (and upstreams that set
// `additionalProperties: false`, e.g. unifi-network) can never send
// _resultId/_section, which makes every paginated result unreadable.
return tools.map(withDrillDownParams);
},
async onToolCallBefore(_toolName, args, request, ctx) {
// Intercept section drill-down requests
const resultId = args['_resultId'] as string | undefined;
@@ -30,6 +47,13 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
return handleSectionDrillDown(request, resultId, section, ctx);
}
// _resultId without _section: the caller wants the cached result but did
// not name a section. Re-show the table of contents instead of
// forwarding _resultId upstream, where it is an unknown argument.
if (resultId !== undefined && resultId !== '') {
return handleSectionListing(request, resultId, ctx);
}
return null;
},
@@ -50,8 +74,20 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
if (response.error) return response;
// Extract text content from the response
const raw = extractTextContent(response);
if (!raw || raw.length <= 2000) return response;
const extracted = extractTextContent(response);
if (extracted === null) return response;
// Collapse nested MCP envelopes first. A server that proxies another MCP
// server (unifi-network does) returns the inner result wrapped in its own
// content/structuredContent pair, so the same payload arrives two or
// three times over. Unwrapping halves it, which keeps many results under
// the pagination threshold entirely.
const raw = unwrapNestedEnvelopes(extracted);
const unwrapped = raw !== extracted;
if (raw.length <= 2000) {
return unwrapped ? textResponse(response.id, raw) : response;
}
try {
const result = await ctx.processContent(toolName, raw, 'toolResult');
@@ -61,7 +97,7 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
const resultId = `pm-${Date.now().toString(36)}`;
storeSections(ctx, resultId, result.sections);
const text = `${result.content}\n\n_resultId: ${resultId} — use _resultId and _section parameters to drill into a section.`;
const text = `${result.content}\n\n${drillDownInstruction(toolName, resultId, result.sections)}`;
return {
jsonrpc: '2.0',
id: response.id,
@@ -71,21 +107,174 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
// Pipeline ran but no sections — return processed content if it changed
if (result.content !== raw) {
return {
jsonrpc: '2.0',
id: response.id,
result: { content: [{ type: 'text', text: result.content }] },
};
return textResponse(response.id, result.content);
}
} catch {
// Pipeline failed — return original response
// Pipeline failed — fall through, but keep the unwrap
}
if (unwrapped) return textResponse(response.id, raw);
return response;
},
};
}
/**
* Build the drill-down instruction appended to a sectioned result.
*
* Names both parameters exactly as the schema declares them and shows a real
* section id, so a model can copy the call rather than guess at it. Guessing
* used to mean sending a bare `section`, which fell through to the upstream
* and re-paginated with a fresh _resultId — an unbounded loop.
*/
function drillDownInstruction(toolName: string, resultId: string, sections: Section[]): string {
const example = sections[0]?.id ?? 'page-1';
return `To read one, call ${toolName} again with _resultId="${resultId}" and _section="${example}". `
+ 'Both parameters are required together; pass no other arguments.';
}
/**
* Declare the drill-down params on a tool's advertised input schema.
*
* Only `properties` is extended: `additionalProperties: false` stays as the
* upstream set it, because a property listed in `properties` is allowed by
* that keyword. Keeping it false preserves the upstream's typo protection.
*/
function withDrillDownParams(tool: ToolDefinition): ToolDefinition {
if (GATE_TOOLS.has(tool.name)) return tool;
const schema = tool.inputSchema;
if (typeof schema !== 'object' || schema === null || Array.isArray(schema)) return tool;
const s = schema as Record<string, unknown>;
if (s['type'] !== undefined && s['type'] !== 'object') return tool;
const existing = s['properties'];
const props: Record<string, unknown> =
typeof existing === 'object' && existing !== null && !Array.isArray(existing)
? { ...(existing as Record<string, unknown>) }
: {};
if ('_resultId' in props || '_section' in props) return tool;
props['_resultId'] = {
type: 'string',
description:
'Set only when re-reading a large result this tool already returned. '
+ 'Pass the _resultId from that result together with _section.',
};
props['_section'] = {
type: 'string',
description:
'Section id to read (e.g. "page-1"), from the table of contents of a '
+ 'previous large result. Requires _resultId.',
};
return { ...tool, inputSchema: { ...s, properties: props } };
}
/** Build a single-text-part tool result. */
function textResponse(id: JsonRpcResponse['id'], text: string): JsonRpcResponse {
return { jsonrpc: '2.0', id, result: { content: [{ type: 'text', text }] } };
}
/** Maximum envelope layers to peel. Guards against pathological nesting. */
const MAX_UNWRAP_DEPTH = 5;
/**
* Collapse nested MCP result envelopes.
*
* A server that fronts another MCP server hands back the inner result still
* wrapped: `{ ...meta, content: [{ type: 'text', text: "<inner JSON>" }],
* structuredContent: <same inner value> }`. The payload is then present twice
* — once escaped inside `content`, once parsed in `structuredContent` — and
* the whole thing is itself a string inside the outer result. unifi-network's
* get_clients arrives at 136K this way.
*
* A layer is peeled only when it carries nothing the inner value doesn't
* already have: every sibling key must reappear in the inner value with an
* equal value, and `structuredContent` (if present) must equal it too. That
* makes the collapse lossless; anything else is left alone.
*/
function unwrapNestedEnvelopes(text: string): string {
let current: unknown;
try {
current = JSON.parse(text);
} catch {
return text; // Not JSON — nothing to unwrap.
}
let peeled = false;
for (let depth = 0; depth < MAX_UNWRAP_DEPTH; depth++) {
const inner = peelEnvelope(current);
if (inner === null) break;
current = inner;
peeled = true;
}
if (!peeled) return text;
return typeof current === 'string' ? current : JSON.stringify(current, null, 2);
}
/** Peel one envelope layer, or return null when this value is not a redundant wrapper. */
function peelEnvelope(value: unknown): unknown | null {
if (typeof value !== 'object' || value === null || Array.isArray(value)) return null;
const obj = value as Record<string, unknown>;
const parts = obj['content'];
if (!Array.isArray(parts) || parts.length === 0) return null;
// Only text parts can be collapsed; images and resources must survive intact.
const texts: string[] = [];
for (const part of parts) {
if (typeof part !== 'object' || part === null) return null;
const p = part as Record<string, unknown>;
if (p['type'] !== 'text' || typeof p['text'] !== 'string') return null;
texts.push(p['text']);
}
const innerText = texts.join('\n');
let inner: unknown;
try {
inner = JSON.parse(innerText);
} catch {
// Inner payload is plain text. Collapsing to it is still lossless when the
// envelope adds nothing but a duplicate structuredContent.
inner = innerText;
}
// structuredContent must not carry anything the inner payload lacks.
if ('structuredContent' in obj && !deepEqual(obj['structuredContent'], inner)) return null;
// Every other sibling key must already be present, and equal, inside.
for (const [key, v] of Object.entries(obj)) {
if (key === 'content' || key === 'structuredContent') continue;
if (typeof inner !== 'object' || inner === null || Array.isArray(inner)) return null;
if (!deepEqual((inner as Record<string, unknown>)[key], v)) return null;
}
return inner;
}
/** Structural equality for JSON-shaped values. */
function deepEqual(a: unknown, b: unknown): boolean {
if (a === b) return true;
if (typeof a !== typeof b) return false;
if (typeof a !== 'object' || a === null || b === null) return false;
if (Array.isArray(a) || Array.isArray(b)) {
if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) return false;
return a.every((item, i) => deepEqual(item, b[i]));
}
const ao = a as Record<string, unknown>;
const bo = b as Record<string, unknown>;
const aKeys = Object.keys(ao);
if (aKeys.length !== Object.keys(bo).length) return false;
return aKeys.every((k) => k in bo && deepEqual(ao[k], bo[k]));
}
/** Extract text content from a tool result response. */
function extractTextContent(response: JsonRpcResponse): string | null {
if (!response.result || typeof response.result !== 'object') return null;
@@ -145,6 +334,39 @@ function handleSectionDrillDown(
};
}
/** Re-show the table of contents for a cached result (no _section given). */
function handleSectionListing(
request: JsonRpcRequest,
resultId: string,
ctx: PluginSessionContext,
): JsonRpcResponse {
const sections = getSections(ctx, resultId);
if (!sections) {
return {
jsonrpc: '2.0',
id: request.id,
result: {
content: [{
type: 'text',
text: 'Cached result not found (expired or invalid _resultId). Please re-call the tool without _resultId/_section to get a fresh result.',
}],
},
};
}
const toc = sections.map((s) => `[${s.id}] ${s.title}`).join('\n');
return {
jsonrpc: '2.0',
id: request.id,
result: {
content: [{
type: 'text',
text: `${sections.length} sections:\n${toc}\n\nAdd _section="<id>" alongside _resultId="${resultId}" to read one.`,
}],
},
};
}
/** Find a section by ID, searching recursively through children. */
function findSection(sections: Section[], id: string): Section | null {
for (const section of sections) {

View File

@@ -10,6 +10,7 @@
* overlap on hooks, no conflicts arise.
*/
import type { ProxyModelPlugin } from '../plugin.js';
import type { ToolDefinition } from '../types.js';
import { createGatePlugin, type GatePluginConfig } from './gate.js';
import { createContentPipelinePlugin } from './content-pipeline.js';
@@ -59,8 +60,15 @@ export function createDefaultPlugin(config: DefaultPluginConfig = {}): ProxyMode
if (gate.onInitialize) {
plugin.onInitialize = gate.onInitialize.bind(gate);
}
if (gate.onToolsList) {
plugin.onToolsList = gate.onToolsList.bind(gate);
// Tools list: gate first (it decides which tools are visible at all), then
// content-pipeline (it annotates whatever survived with drill-down params).
if (gate.onToolsList || pipeline.onToolsList) {
plugin.onToolsList = async (tools, ctx): Promise<ToolDefinition[]> => {
let acc = tools;
if (gate.onToolsList) acc = await gate.onToolsList(acc, ctx);
if (pipeline.onToolsList) acc = await pipeline.onToolsList(acc, ctx);
return acc;
};
}
if (pipeline.onToolCallAfter) {
plugin.onToolCallAfter = pipeline.onToolCallAfter.bind(pipeline);

View File

@@ -37,7 +37,9 @@ const handler: StageHandler = async (content, ctx) => {
).join('\n');
return {
content: `Content split into ${sections.length} pages (${content.length} total chars):\n${toc}\n\nUse section parameter to read a specific page.`,
// No navigation hint here — the caller (content-pipeline / router) appends
// the authoritative _resultId/_section instruction once sections are stored.
content: `Content split into ${sections.length} pages (${content.length} total chars):\n${toc}`,
sections,
};
};

View File

@@ -65,7 +65,7 @@ const handler: StageHandler = async (content, ctx) => {
}).join('\n');
return {
content: `${sections.length} sections (${contentType}):\n${toc}\n\nUse section parameter to read a specific section.`,
content: `${sections.length} sections (${contentType}):\n${toc}`,
sections,
};
};

View File

@@ -39,7 +39,7 @@ const handler: StageHandler = async (content, ctx) => {
const summary = await cachedSummarize(ctx, ctx.originalContent, maxTokens);
return {
content: summary + '\n\nUse section parameter with id "full" to read the complete content.',
content: summary + '\n\nSection "full" holds the complete content.',
sections: [{ id: 'full', title: 'Full Content', content: ctx.originalContent }],
};
}
@@ -56,7 +56,7 @@ const handler: StageHandler = async (content, ctx) => {
}).join('\n');
return {
content: `${tree.length} sections:\n${toc}\n\nUse section parameter to read details.`,
content: `${tree.length} sections:\n${toc}`,
sections: tree,
};
};

View File

@@ -1191,6 +1191,6 @@ function injectSectionsIntoPromptResponse(
if (now - entry.createdAt > SECTION_TTL_MS) store.delete(key);
}
const text = `${tocContent}\n\n_resultId: ${resultId} — use _resultId and _section parameters to drill into a section.`;
const text = `${tocContent}\n\nTo read one, request this prompt again with _resultId="${resultId}" and _section="<id>". Both parameters are required together.`;
return replacePromptText(response, text);
}

View File

@@ -0,0 +1,339 @@
/**
* content-pipeline drill-down contract tests.
*
* Regression cover for the defect that made UniFi tools unusable from
* non-Claude MCP clients: a large tool result was replaced with a stub that
* told the caller to pass _resultId/_section, but those parameters were never
* advertised on the tool's inputSchema. Upstreams that declare
* `additionalProperties: false` (unifi-network, my-grafana) therefore made the
* drill-down call invalid for any client that validates against the schema,
* and the stub's own wording ("Use section parameter") pointed at a parameter
* that does not exist — sending it re-paginated and minted a fresh _resultId,
* an unbounded loop.
*
* Driven through a real McpRouter with the default plugin (gate +
* content-pipeline), mirroring project-mcp-endpoint wiring.
*/
import { describe, it, expect, vi } from 'vitest';
import { McpRouter } from '../src/router.js';
import type { UpstreamConnection, JsonRpcRequest, JsonRpcResponse } from '../src/types.js';
import type { McpdClient } from '../src/http/mcpd-client.js';
import { createDefaultPlugin } from '../src/proxymodel/plugins/default.js';
import { LLMProviderAdapter } from '../src/proxymodel/llm-adapter.js';
import { MemoryCache } from '../src/proxymodel/cache.js';
/** Mirrors unifi-network: strict schema, and a payload big enough to paginate. */
const STRICT_SCHEMA = {
type: 'object',
properties: {
targetId: { type: 'string' },
site: { type: 'string' },
},
additionalProperties: false,
$schema: 'http://json-schema.org/draft-07/schema#',
};
const BIG_PAYLOAD = 'x'.repeat(20_000);
/**
* The shape unifi-network actually returns: the inner MCP result is wrapped in
* the server's own content/structuredContent pair, so the payload arrives
* twice — once escaped inside `content`, once parsed in `structuredContent`.
*/
function wrapLikeUnifi(tool: string, inner: unknown): string {
const innerEnvelope = { tool, targetId: 'home', result: inner };
return JSON.stringify(
{
tool,
targetId: 'home',
content: [{ type: 'text', text: JSON.stringify(innerEnvelope, null, 2) }],
structuredContent: innerEnvelope,
},
null,
2,
);
}
/** Inner payload stays under the 8000-char page size; wrapped, it does not. */
const DEVICE_ROWS = Array.from({ length: 30 }, (_, i) => ({
mac: `0c:ea:14:38:af:${i.toString(16).padStart(2, '0')}`,
name: `Switch ${i} — office floor plan position ${i}`,
model: 'USPM16',
ip: `192.168.1.${i + 10}`,
version: '7.4.1.16850',
}));
const PAYLOADS: Record<string, string> = {
get_devices: BIG_PAYLOAD,
get_clients: wrapLikeUnifi('get_clients', { data: DEVICE_ROWS }),
get_alarms: wrapLikeUnifi('get_alarms', { data: [] }),
};
interface Upstream {
conn: UpstreamConnection;
calls: Array<Record<string, unknown>>;
}
function mockUpstream(name: string, payloads: Record<string, string> = PAYLOADS): Upstream {
const calls: Array<Record<string, unknown>> = [];
const conn = {
name,
isAlive: vi.fn(() => true),
close: vi.fn(async () => {}),
onNotification: vi.fn(),
send: vi.fn(async (req: JsonRpcRequest): Promise<JsonRpcResponse> => {
if (req.method === 'tools/list') {
return {
jsonrpc: '2.0',
id: req.id,
result: {
tools: Object.keys(payloads).map((n) => ({
name: n,
description: `Retrieve ${n}`,
inputSchema: STRICT_SCHEMA,
})),
},
};
}
if (req.method === 'tools/call') {
const params = (req.params ?? {}) as Record<string, unknown>;
calls.push((params['arguments'] as Record<string, unknown>) ?? {});
const tool = String(params['name'] ?? '').split('/').pop() ?? '';
return { jsonrpc: '2.0', id: req.id, result: { content: [{ type: 'text', text: payloads[tool] ?? '' }] } };
}
if (req.method === 'resources/list') return { jsonrpc: '2.0', id: req.id, result: { resources: [] } };
if (req.method === 'prompts/list') return { jsonrpc: '2.0', id: req.id, result: { prompts: [] } };
return { jsonrpc: '2.0', id: req.id, error: { code: -32601, message: 'Not found' } };
}),
} as unknown as UpstreamConnection;
return { conn, calls };
}
function mockMcpdClient(): McpdClient {
return {
get: vi.fn(async () => []),
post: vi.fn(async () => ({})),
put: vi.fn(async () => ({})),
delete: vi.fn(async () => {}),
forward: vi.fn(async () => ({ status: 200, body: {} })),
withHeaders: vi.fn(function (this: McpdClient) { return this; }),
} as unknown as McpdClient;
}
function setup(opts: { gated?: boolean; payloads?: Record<string, string> } = {}) {
const router = new McpRouter();
router.setPromptConfig(mockMcpdClient(), 'test-project');
router.setPlugin(createDefaultPlugin({ gated: opts.gated ?? false, providerRegistry: null }));
router.setProxyModel(
'default',
{ complete: async () => '', available: () => false } as unknown as LLMProviderAdapter,
new MemoryCache(),
);
const upstream = mockUpstream('unifi-network', opts.payloads ?? PAYLOADS);
router.addUpstream(upstream.conn);
return { router, upstream };
}
async function listTools(router: McpRouter, sessionId = 's1') {
await router.route({ jsonrpc: '2.0', id: 1, method: 'initialize' }, { sessionId });
const res = await router.route({ jsonrpc: '2.0', id: 2, method: 'tools/list' }, { sessionId });
return (res.result as { tools: Array<{ name: string; inputSchema?: unknown }> }).tools;
}
function textOf(res: JsonRpcResponse): string {
return (res.result as { content: Array<{ text: string }> }).content[0]!.text;
}
async function callNamed(
router: McpRouter,
tool: string,
args: Record<string, unknown>,
id: number,
sessionId = 's1',
): Promise<JsonRpcResponse> {
return router.route(
{ jsonrpc: '2.0', id, method: 'tools/call', params: { name: `unifi-network/${tool}`, arguments: args } },
{ sessionId },
);
}
async function callTool(
router: McpRouter,
args: Record<string, unknown>,
id: number,
sessionId = 's1',
): Promise<JsonRpcResponse> {
return router.route(
{ jsonrpc: '2.0', id, method: 'tools/call', params: { name: 'unifi-network/get_devices', arguments: args } },
{ sessionId },
);
}
describe('content-pipeline drill-down contract', () => {
it('advertises _resultId/_section on a strict-schema upstream tool', async () => {
const { router } = setup();
const tool = (await listTools(router)).find((t) => t.name.endsWith('get_devices'));
expect(tool).toBeDefined();
const schema = tool!.inputSchema as Record<string, unknown>;
const props = schema['properties'] as Record<string, unknown>;
expect(props['_resultId']).toMatchObject({ type: 'string' });
expect(props['_section']).toMatchObject({ type: 'string' });
// The upstream's own params survive untouched.
expect(props['targetId']).toMatchObject({ type: 'string' });
// Declaring the params in `properties` is what makes them legal; the
// upstream's strictness is preserved rather than loosened.
expect(schema['additionalProperties']).toBe(false);
});
it('leaves gate tools alone (they never reach the pipeline)', async () => {
const { router } = setup({ gated: true });
const tools = await listTools(router);
const beginSession = tools.find((t) => t.name === 'begin_session');
expect(beginSession).toBeDefined();
const props = (beginSession!.inputSchema as Record<string, unknown>)['properties'] as Record<string, unknown>;
expect(props['_resultId']).toBeUndefined();
expect(props['_section']).toBeUndefined();
});
it('stubs a large result with an instruction naming the real parameters', async () => {
const { router } = setup();
await listTools(router);
const text = textOf(await callTool(router, {}, 3));
expect(text).toContain('_resultId=');
expect(text).toContain('_section=');
// The old wording pointed at a parameter that does not exist.
expect(text).not.toContain('Use section parameter');
});
it('serves the page from cache without a second upstream call', async () => {
const { router, upstream } = setup();
await listTools(router);
const stub = textOf(await callTool(router, {}, 3));
const resultId = /_resultId="([^"]+)"/.exec(stub)?.[1];
expect(resultId).toBeTruthy();
const callsAfterFirst = upstream.calls.length;
const page = textOf(await callTool(router, { _resultId: resultId!, _section: 'page-1' }, 4));
expect(page).toContain('x'.repeat(100));
// Drill-down is answered locally, so the strict upstream never sees the
// params it would reject.
expect(upstream.calls.length).toBe(callsAfterFirst);
});
it('re-shows the table of contents when _resultId arrives without _section', async () => {
const { router, upstream } = setup();
await listTools(router);
const stub = textOf(await callTool(router, {}, 3));
const resultId = /_resultId="([^"]+)"/.exec(stub)?.[1];
const callsAfterFirst = upstream.calls.length;
const res = textOf(await callTool(router, { _resultId: resultId! }, 4));
expect(res).toContain('page-1');
expect(res).toContain('_section=');
expect(upstream.calls.length).toBe(callsAfterFirst);
});
it('reports an expired or unknown _resultId instead of forwarding it', async () => {
const { router, upstream } = setup();
await listTools(router);
const callsBefore = upstream.calls.length;
const res = textOf(await callTool(router, { _resultId: 'pm-nope', _section: 'page-1' }, 3));
expect(res).toContain('Cached result not found');
expect(upstream.calls.length).toBe(callsBefore);
});
});
describe('nested MCP envelope collapse', () => {
it('collapses a doubly-wrapped result to its inner payload', async () => {
const { router } = setup();
await listTools(router);
const text = textOf(await callNamed(router, 'get_alarms', {}, 3));
const parsed = JSON.parse(text) as Record<string, unknown>;
// The wrapper's content/structuredContent pair is gone; the payload it
// carried twice now appears once.
expect(parsed['content']).toBeUndefined();
expect(parsed['structuredContent']).toBeUndefined();
expect(parsed['tool']).toBe('get_alarms');
expect(parsed['targetId']).toBe('home');
expect(parsed['result']).toEqual({ data: [] });
});
it('drops a formerly-paginated result below the pagination threshold', async () => {
const { router } = setup();
await listTools(router);
const wrapped = PAYLOADS['get_clients']!;
expect(wrapped.length).toBeGreaterThan(8000); // would paginate as-is
const text = textOf(await callNamed(router, 'get_clients', {}, 3));
// Delivered whole, not as a table of contents.
expect(text).not.toContain('Content split into');
expect(text).not.toContain('_resultId=');
expect(text.length).toBeLessThan(wrapped.length);
const parsed = JSON.parse(text) as { result: { data: unknown[] } };
expect(parsed.result.data).toHaveLength(30);
});
it('leaves a wrapper alone when structuredContent disagrees with the text', async () => {
const divergent = JSON.stringify({
tool: 'get_devices',
content: [{ type: 'text', text: JSON.stringify({ tool: 'get_devices', result: { data: [1] } }) }],
structuredContent: { tool: 'get_devices', result: { data: [1, 2, 3] } },
});
const { router } = setup({ payloads: { get_devices: divergent } });
await listTools(router);
const text = textOf(await callTool(router, {}, 3));
// Collapsing here would silently drop rows, so the envelope survives.
expect(text).toBe(divergent);
});
it('leaves a wrapper alone when it carries a key the payload lacks', async () => {
const extraKey = JSON.stringify({
tool: 'get_devices',
warning: 'partial results — controller unreachable',
content: [{ type: 'text', text: JSON.stringify({ tool: 'get_devices', result: { data: [] } }) }],
});
const { router } = setup({ payloads: { get_devices: extraKey } });
await listTools(router);
expect(textOf(await callTool(router, {}, 3))).toBe(extraKey);
});
it('leaves non-text content parts intact', async () => {
const withImage = JSON.stringify({
tool: 'get_devices',
content: [
{ type: 'text', text: '{"tool":"get_devices"}' },
{ type: 'image', data: 'iVBORw0KGgo=', mimeType: 'image/png' },
],
});
const { router } = setup({ payloads: { get_devices: withImage } });
await listTools(router);
expect(textOf(await callTool(router, {}, 3))).toBe(withImage);
});
it('leaves plain non-JSON results untouched', async () => {
const { router } = setup({ payloads: { get_devices: 'plain text, no envelope' } });
await listTools(router);
expect(textOf(await callTool(router, {}, 3))).toBe('plain text, no envelope');
});
});

View File

@@ -261,9 +261,11 @@ describe('Prompt section drill-down', () => {
expect(result.sections).toBeDefined();
expect(result.sections!.length).toBeGreaterThanOrEqual(3);
// TOC should list sections
// TOC should list sections. The stage must NOT name a navigation
// parameter: the caller appends the authoritative _resultId/_section
// instruction, and a bare `section` hint here sends models into a loop.
expect(result.content).toContain('sections');
expect(result.content).toContain('Use section parameter');
expect(result.content).not.toContain('Use section parameter');
// Original was ~16K, TOC should be much shorter
expect(result.content.length).toBeLessThan(largePrompt.length);

View File

@@ -4,7 +4,7 @@
* Verifies that large prompts served via prompts/get are section-split
* and that subsequent calls with _resultId + _section return cached sections.
*
* Requires: mcplocal running on localhost:3200, mcpd on 10.0.0.194:3100
* Requires: mcplocal running on localhost:3200, mcpd at https://mcpctl.ad.itaz.eu
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';

View File

@@ -6,7 +6,7 @@
*
* Prerequisites:
* - mcplocal running on localhost:3200
* - mcpd running on 10.0.0.194:3100
* - mcpd reachable at https://mcpctl.ad.itaz.eu
* - smoke-aws-docs server deployed (runtime: python)
*
* The test suite uses the fixture at fixtures/smoke-data.yaml which

View File

@@ -0,0 +1,109 @@
/**
* Smoke tests: tool drill-down contract.
*
* A large tool result is replaced with a table of contents and re-read by
* calling the same tool with _resultId + _section. Those two parameters must
* appear in the tool's advertised inputSchema, because upstreams such as
* unifi-network and my-grafana declare `additionalProperties: false` — a
* client validating against the schema cannot send an undeclared parameter,
* which used to make every paginated UniFi result unreadable.
*
* Requires: mcplocal running (localhost:3200) with a project whose servers are
* reachable. Set SMOKE_PROJECT to target a specific project.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';
const PROJECT_NAME = process.env['SMOKE_PROJECT'] ?? 'smoke-data';
/** HTTP-mode mcplocal authenticates every request with an McpToken. */
const TOKEN = process.env['SMOKE_MCPTOKEN'];
interface Tool {
name: string;
inputSchema?: { type?: string; properties?: Record<string, unknown>; additionalProperties?: unknown };
}
/** Tools served by the gate plugin — intercepted before the pipeline, so exempt. */
const GATE_TOOLS = new Set(['begin_session', 'read_prompts', 'propose_prompt', 'propose_skill']);
describe('Smoke: tool drill-down contract', () => {
let available = false;
let session: SmokeMcpSession;
let tools: Tool[] = [];
beforeAll(async () => {
available = await isMcplocalRunning();
if (!available) return;
session = new SmokeMcpSession(PROJECT_NAME, TOKEN);
await session.initialize();
await session.sendNotification('notifications/initialized');
// Open the gate if the project is gated, so the real catalog is visible.
const gated = await session.send('tools/list') as { tools: Tool[] };
if (gated.tools.some((t) => t.name === 'begin_session')) {
await session.send('tools/call', {
name: 'begin_session',
arguments: { description: 'Verify the paginated tool-result drill-down contract' },
}, 180_000);
}
tools = ((await session.send('tools/list')) as { tools: Tool[] }).tools;
}, 240_000);
afterAll(async () => {
if (session) await session.close();
});
it('every upstream tool advertises _resultId and _section', async () => {
if (!available) return;
const upstreamTools = tools.filter((t) => !GATE_TOOLS.has(t.name));
if (upstreamTools.length === 0) {
console.log(` No upstream tools in project "${PROJECT_NAME}" — skipping`);
return;
}
const missing = upstreamTools.filter((t) => {
const props = t.inputSchema?.properties;
return props === undefined || props['_resultId'] === undefined || props['_section'] === undefined;
});
if (missing.length > 0) {
console.log(` Missing drill-down params: ${missing.map((t) => t.name).join(', ')}`);
}
expect(missing).toEqual([]);
console.log(` ${upstreamTools.length} tools carry the drill-down contract`);
});
it('strict upstream schemas stay strict (params are declared, not permitted)', async () => {
if (!available) return;
// Declaring the params in `properties` is what makes them legal under
// `additionalProperties: false`; loosening it instead would drop the
// upstream's own typo protection.
const strict = tools.filter(
(t) => !GATE_TOOLS.has(t.name) && t.inputSchema?.additionalProperties === false,
);
if (strict.length === 0) {
console.log(' No strict-schema tools in this project — skipping');
return;
}
for (const t of strict) {
expect(t.inputSchema!.properties!['_resultId']).toBeDefined();
expect(t.inputSchema!.properties!['_section']).toBeDefined();
}
console.log(` ${strict.length} strict-schema tools keep additionalProperties: false`);
});
it('gate tools do not advertise drill-down params', async () => {
if (!available) return;
for (const t of tools.filter((x) => GATE_TOOLS.has(x.name))) {
const props = t.inputSchema?.properties ?? {};
expect(props['_resultId']).toBeUndefined();
expect(props['_section']).toBeUndefined();
}
});
});