Compare commits
8 Commits
8c359902c7
...
fix/stdio-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
5fb1154190 | ||
|
|
b022f322f0 | ||
|
|
c66502e590 | ||
|
|
740ce31469 | ||
| b6983f036d | |||
|
|
21aadf6d82 | ||
| 35d506df77 | |||
|
|
03350856ea |
@@ -7,7 +7,11 @@ Type=simple
|
|||||||
ExecStart=/usr/bin/mcpctl-local
|
ExecStart=/usr/bin/mcpctl-local
|
||||||
Restart=on-failure
|
Restart=on-failure
|
||||||
RestartSec=5
|
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_PORT=3200
|
||||||
Environment=MCPLOCAL_HTTP_HOST=127.0.0.1
|
Environment=MCPLOCAL_HTTP_HOST=127.0.0.1
|
||||||
|
|
||||||
|
|||||||
@@ -187,14 +187,16 @@ trap - ERR
|
|||||||
|
|
||||||
# ── 7. RPM + smoke ──
|
# ── 7. RPM + smoke ──
|
||||||
say "7/7 Build/install CLI RPM + smoke tests"
|
say "7/7 Build/install CLI RPM + smoke tests"
|
||||||
bash scripts/release.sh
|
# release.sh already restarts mcplocal and runs the smoke suite against the
|
||||||
systemctl --user restart mcplocal && sleep 2
|
# binary it just installed. This step used to restart and re-run it a second
|
||||||
if pnpm test:smoke > /tmp/deploy-smoke.log 2>&1; then
|
# time, which put two full suites inside a minute and tripped mcpd's rate
|
||||||
grep -E "Tests |passed" /tmp/deploy-smoke.log | tail -2
|
# 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"
|
say "Deploy complete — $TAG live. Rollback tag: $ROLLBACK_TAG"
|
||||||
else
|
else
|
||||||
tail -40 /tmp/deploy-smoke.log
|
warn "RELEASE OR SMOKE TESTS FAILED — system may be unhealthy. Consider rollback:"
|
||||||
warn "SMOKE TESTS FAILED — system may be unhealthy. Consider rollback:"
|
|
||||||
rollback_recipe
|
rollback_recipe
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -511,6 +511,10 @@ async function main(): Promise<void> {
|
|||||||
const authService = new AuthService(prisma);
|
const authService = new AuthService(prisma);
|
||||||
const templateService = new TemplateService(templateRepo);
|
const templateService = new TemplateService(templateRepo);
|
||||||
const mcpProxyService = new McpProxyService(instanceRepo, serverRepo, orchestrator);
|
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 rbacDefinitionService = new RbacDefinitionService(rbacDefinitionRepo);
|
||||||
const rbacService = new RbacService(rbacDefinitionRepo, prisma);
|
const rbacService = new RbacService(rbacDefinitionRepo, prisma);
|
||||||
const mcpTokenService = new McpTokenService(mcpTokenRepo, projectRepo, rbacDefinitionRepo, rbacService);
|
const mcpTokenService = new McpTokenService(mcpTokenRepo, projectRepo, rbacDefinitionRepo, rbacService);
|
||||||
@@ -969,6 +973,7 @@ async function main(): Promise<void> {
|
|||||||
healthProbeRunner.stop();
|
healthProbeRunner.stop();
|
||||||
secretBackendRotatorLoop.stop();
|
secretBackendRotatorLoop.stop();
|
||||||
gitBackup.stop();
|
gitBackup.stop();
|
||||||
|
mcpProxyService.closeAll();
|
||||||
await prisma.$disconnect();
|
await prisma.$disconnect();
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -265,6 +265,16 @@ export class DockerContainerManager implements McpOrchestrator {
|
|||||||
const stderr = new PassThrough();
|
const stderr = new PassThrough();
|
||||||
this.docker.modem.demuxStream(stream, stdout, stderr);
|
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 {
|
return {
|
||||||
stdout,
|
stdout,
|
||||||
write(data: string) {
|
write(data: string) {
|
||||||
|
|||||||
@@ -193,6 +193,13 @@ export class HealthProbeRunner {
|
|||||||
? 'unhealthy'
|
? 'unhealthy'
|
||||||
: 'degraded';
|
: '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
|
// Build event
|
||||||
const probeLabel = probeKind === 'readiness'
|
const probeLabel = probeKind === 'readiness'
|
||||||
? `Readiness check (${healthCheck.tool})`
|
? `Readiness check (${healthCheck.tool})`
|
||||||
|
|||||||
@@ -32,6 +32,9 @@ interface RetryMetadata {
|
|||||||
attemptCount?: number;
|
attemptCount?: number;
|
||||||
lastAttemptAt?: string;
|
lastAttemptAt?: string;
|
||||||
nextRetryAt?: 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;
|
[k: string]: unknown;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -52,6 +55,8 @@ export class InvalidStateError extends Error {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export class InstanceService {
|
export class InstanceService {
|
||||||
|
private stdioInvalidator?: (containerId: string) => void;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
private instanceRepo: IMcpInstanceRepository,
|
private instanceRepo: IMcpInstanceRepository,
|
||||||
private serverRepo: IMcpServerRepository,
|
private serverRepo: IMcpServerRepository,
|
||||||
@@ -59,6 +64,25 @@ export class InstanceService {
|
|||||||
private secretResolver?: SecretResolver,
|
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[]> {
|
async list(serverId?: string): Promise<McpInstance[]> {
|
||||||
return this.instanceRepo.findAll(serverId);
|
return this.instanceRepo.findAll(serverId);
|
||||||
}
|
}
|
||||||
@@ -71,37 +95,96 @@ export class InstanceService {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Sync instance statuses with actual container state.
|
* 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> {
|
async syncStatus(): Promise<void> {
|
||||||
const instances = await this.instanceRepo.findAll();
|
const instances = await this.instanceRepo.findAll();
|
||||||
for (const inst of instances) {
|
for (const inst of instances) {
|
||||||
if ((inst.status === 'RUNNING' || inst.status === 'STARTING') && inst.containerId) {
|
if (!inst.containerId) continue;
|
||||||
try {
|
if (inst.status !== 'RUNNING' && inst.status !== 'STARTING' && inst.status !== 'ERROR') {
|
||||||
const info = await this.orchestrator.inspectContainer(inst.containerId);
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if (info.state === 'stopped' || info.state === 'error') {
|
let info: ContainerInfo;
|
||||||
// Container died — get last logs for error context
|
try {
|
||||||
let errorMsg = `Container ${info.state}`;
|
info = await this.orchestrator.inspectContainer(inst.containerId);
|
||||||
try {
|
} catch {
|
||||||
const logs = await this.orchestrator.getContainerLogs(inst.containerId, { tail: 5 });
|
// Container gone entirely. ERROR rows with a missing pod stay as they
|
||||||
const lastLog = (logs.stdout || logs.stderr).trim().split('\n').pop();
|
// are — the retry/backoff path owns recreating them.
|
||||||
if (lastLog) errorMsg = lastLog;
|
if (inst.status !== 'ERROR') {
|
||||||
} catch { /* best-effort */ }
|
|
||||||
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
|
|
||||||
metadata: { error: errorMsg },
|
|
||||||
});
|
|
||||||
} else if (info.state === 'starting' && inst.status === 'RUNNING') {
|
|
||||||
// Pod went back to starting (e.g. CrashLoopBackOff restart)
|
|
||||||
await this.instanceRepo.updateStatus(inst.id, 'STARTING', {});
|
|
||||||
} 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', {
|
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
|
||||||
metadata: { error: 'Container not found' },
|
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
|
||||||
|
let errorMsg = `Container ${info.state}`;
|
||||||
|
try {
|
||||||
|
const logs = await this.orchestrator.getContainerLogs(inst.containerId, { tail: 5 });
|
||||||
|
const lastLog = (logs.stdout || logs.stderr).trim().split('\n').pop();
|
||||||
|
if (lastLog) errorMsg = lastLog;
|
||||||
|
} catch { /* best-effort */ }
|
||||||
|
await this.instanceRepo.updateStatus(inst.id, 'ERROR', {
|
||||||
|
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', { metadata: meta });
|
||||||
|
this.invalidateStdio(inst.containerId);
|
||||||
|
} else if (info.state === 'running' && inst.status === 'STARTING') {
|
||||||
|
// 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 {
|
} catch {
|
||||||
// Container may already be gone
|
// Container may already be gone
|
||||||
}
|
}
|
||||||
|
this.invalidateStdio(instance.containerId);
|
||||||
}
|
}
|
||||||
|
|
||||||
await this.instanceRepo.delete(id);
|
await this.instanceRepo.delete(id);
|
||||||
@@ -256,6 +340,7 @@ export class InstanceService {
|
|||||||
} catch {
|
} catch {
|
||||||
// best-effort
|
// best-effort
|
||||||
}
|
}
|
||||||
|
this.invalidateStdio(inst.containerId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -489,6 +574,7 @@ export class InstanceService {
|
|||||||
try {
|
try {
|
||||||
await this.orchestrator.removeContainer(instance.containerId, true);
|
await this.orchestrator.removeContainer(instance.containerId, true);
|
||||||
} catch { /* best-effort */ }
|
} catch { /* best-effort */ }
|
||||||
|
this.invalidateStdio(instance.containerId);
|
||||||
}
|
}
|
||||||
await this.instanceRepo.delete(instance.id);
|
await this.instanceRepo.delete(instance.id);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -81,6 +81,18 @@ function podToContainerInfo(pod: V1Pod): ContainerInfo {
|
|||||||
info.port = ports[0].containerPort;
|
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;
|
return info;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -116,10 +128,39 @@ export class KubernetesOrchestrator implements McpOrchestrator {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const manifest = generatePodSpec(spec, this.namespace);
|
const manifest = generatePodSpec(spec, this.namespace);
|
||||||
const pod = await this.client.core.createNamespacedPod({
|
let pod;
|
||||||
namespace: this.namespace,
|
try {
|
||||||
body: manifest as V1Pod,
|
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
|
// Wait briefly for pod to start scheduling
|
||||||
await new Promise((resolve) => setTimeout(resolve, 500));
|
await new Promise((resolve) => setTimeout(resolve, 500));
|
||||||
@@ -276,6 +317,17 @@ export class KubernetesOrchestrator implements McpOrchestrator {
|
|||||||
// Wait for WebSocket connection to establish
|
// Wait for WebSocket connection to establish
|
||||||
const ws = await wsPromise;
|
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 {
|
return {
|
||||||
stdout,
|
stdout,
|
||||||
write(data: string) {
|
write(data: string) {
|
||||||
@@ -316,6 +368,13 @@ export class KubernetesOrchestrator implements McpOrchestrator {
|
|||||||
false, // tty
|
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 {
|
return {
|
||||||
stdout,
|
stdout,
|
||||||
write(data: string) {
|
write(data: string) {
|
||||||
|
|||||||
@@ -180,20 +180,28 @@ export class McpProxyService {
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
this.removeClient(instance.containerId);
|
this.removeClient(instance.containerId);
|
||||||
// Fall back to one-shot exec when we have a command to run.
|
// 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') {
|
if (mode.kind === 'exec') {
|
||||||
return sendViaStdio(this.orchestrator, instance.containerId, packageName, method, params, 120_000, command, runtime);
|
return sendViaStdio(this.orchestrator, instance.containerId, packageName, method, params, 120_000, command, runtime);
|
||||||
}
|
}
|
||||||
const detail = formatError(err);
|
// Attach mode has no one-shot equivalent, but the failure is usually
|
||||||
console.error(`[mcp-proxy] attach to ${instance.containerId} failed:`, err);
|
// a stale pipe from an in-place container restart — retry once
|
||||||
return {
|
// through a fresh client (which redials) before surfacing the error,
|
||||||
jsonrpc: '2.0',
|
// so the FIRST call after a detected death succeeds (mcpctl#114).
|
||||||
id: 1,
|
try {
|
||||||
error: {
|
return await this.sendViaPersistentStdio(instance.containerId, mode, method, params);
|
||||||
code: -32000,
|
} catch (retryErr) {
|
||||||
message: `STDIO attach to '${instance.containerId}' failed: ${detail}`,
|
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,
|
||||||
|
error: {
|
||||||
|
code: -32000,
|
||||||
|
message: `STDIO attach to '${instance.containerId}' failed: ${detail}`,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -69,6 +69,14 @@ export interface ContainerInfo {
|
|||||||
/** Container IP on the first non-default network (for internal communication) */
|
/** Container IP on the first non-default network (for internal communication) */
|
||||||
ip?: string;
|
ip?: string;
|
||||||
createdAt: Date;
|
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 {
|
export interface ContainerLogs {
|
||||||
|
|||||||
@@ -121,15 +121,15 @@ export class PersistentStdioClient {
|
|||||||
this.processBuffer();
|
this.processBuffer();
|
||||||
});
|
});
|
||||||
|
|
||||||
exec.stdout.on('end', () => {
|
// All three events funnel into the same teardown: 'end' is the graceful
|
||||||
this.initialized = false;
|
// path, but an abnormal websocket death may only surface as 'close' or
|
||||||
this.exec = null;
|
// 'error' on the stream — before this, isConnected stayed true and every
|
||||||
for (const [, pending] of this.pendingRequests) {
|
// request rode out the full timeout against a dead pipe (mcpctl#114).
|
||||||
clearTimeout(pending.timer);
|
exec.stdout.on('end', () => this.teardown(exec, 'STDIO process exited'));
|
||||||
pending.reject(new Error('STDIO process exited'));
|
exec.stdout.on('close', () => this.teardown(exec, 'STDIO stream closed'));
|
||||||
}
|
exec.stdout.on('error', (err: Error) =>
|
||||||
this.pendingRequests.clear();
|
this.teardown(exec, `STDIO stream error: ${err.message}`),
|
||||||
});
|
);
|
||||||
|
|
||||||
// Run MCP init handshake
|
// Run MCP init handshake
|
||||||
const initId = this.nextId++;
|
const initId = this.nextId++;
|
||||||
@@ -174,6 +174,23 @@ export class PersistentStdioClient {
|
|||||||
this.initialized = true;
|
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 {
|
private write(msg: Record<string, unknown>): void {
|
||||||
if (!this.exec) throw new Error('Not connected');
|
if (!this.exec) throw new Error('Not connected');
|
||||||
this.exec.write(JSON.stringify(msg) + '\n');
|
this.exec.write(JSON.stringify(msg) + '\n');
|
||||||
|
|||||||
@@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -63,12 +63,22 @@ vi.mock('@kubernetes/client-node', () => {
|
|||||||
makeApiClient = vi.fn(() => mockCore);
|
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 {
|
class MockExec {
|
||||||
exec = vi.fn();
|
exec = vi.fn();
|
||||||
|
constructor() {
|
||||||
|
instances.exec = this;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
class MockAttach {
|
class MockAttach {
|
||||||
attach = vi.fn();
|
attach = vi.fn();
|
||||||
|
constructor() {
|
||||||
|
instances.attach = this;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
class MockLog {
|
class MockLog {
|
||||||
@@ -82,7 +92,7 @@ vi.mock('@kubernetes/client-node', () => {
|
|||||||
Attach: MockAttach,
|
Attach: MockAttach,
|
||||||
Log: MockLog,
|
Log: MockLog,
|
||||||
// Export test helpers
|
// 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();
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -109,3 +109,110 @@ describe('PersistentStdioClient', () => {
|
|||||||
await expect(client.send('tools/list')).rejects.toThrow(/interactive exec/i);
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -544,3 +544,32 @@ describe('HealthProbeRunner', () => {
|
|||||||
expect(result.message).toBe('ECONNREFUSED 10.0.0.1:3000');
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { McpdClient } from './http/mcpd-client.js';
|
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 type { McpRouter } from './router.js';
|
||||||
import { McpdUpstream } from './upstream/mcpd.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.
|
// unreachable upstream cannot stall session init for the full tool-call window.
|
||||||
const discoveryClient = mcpdClient.withTimeout(DISCOVERY_TIMEOUT_MS);
|
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
|
// Remove stale upstreams
|
||||||
const currentNames = new Set(router.getUpstreamNames());
|
const currentNames = new Set(router.getUpstreamNames());
|
||||||
const serverNames = new Set(servers.map((s) => s.name));
|
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
|
// Add/update upstreams for each server
|
||||||
for (const server of servers) {
|
for (const server of servers) {
|
||||||
if (!currentNames.has(server.name)) {
|
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);
|
router.addUpstream(upstream);
|
||||||
}
|
}
|
||||||
registered.push(server.name);
|
registered.push(server.name);
|
||||||
|
|||||||
@@ -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;
|
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 {
|
export class McpdClient {
|
||||||
private readonly baseUrl: string;
|
private readonly baseUrl: string;
|
||||||
private readonly token: string;
|
private readonly token: string;
|
||||||
|
|||||||
@@ -176,9 +176,13 @@ export function registerProjectMcpEndpoint(app: FastifyInstance, mcpdClient: Mcp
|
|||||||
chain.push(createAgentsPlugin());
|
chain.push(createAgentsPlugin());
|
||||||
router.setPlugin(composePlugins(chain));
|
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 {
|
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`,
|
`/api/v1/projects/${encodeURIComponent(projectName)}/instructions`,
|
||||||
);
|
);
|
||||||
const parts: string[] = [];
|
const parts: string[] = [];
|
||||||
|
|||||||
@@ -7,20 +7,37 @@
|
|||||||
* - sectionStore management
|
* - sectionStore management
|
||||||
*
|
*
|
||||||
* This plugin handles:
|
* This plugin handles:
|
||||||
* 1. onToolCallBefore: intercept section drill-down requests (_resultId + _section params)
|
* 1. onToolsList: declare the drill-down params on every tool it may paginate
|
||||||
* 2. onToolCallAfter: run tool results through the proxymodel pipeline
|
* 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 { 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';
|
import type { ProxyModelPlugin, PluginSessionContext } from '../plugin.js';
|
||||||
|
|
||||||
const SECTION_STORE_TTL_MS = 300_000; // 5 minutes
|
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 {
|
export function createContentPipelinePlugin(): ProxyModelPlugin {
|
||||||
return {
|
return {
|
||||||
name: 'content-pipeline',
|
name: 'content-pipeline',
|
||||||
description: 'Content transformation pipeline: paginate, section-split, summarize tool results.',
|
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) {
|
async onToolCallBefore(_toolName, args, request, ctx) {
|
||||||
// Intercept section drill-down requests
|
// Intercept section drill-down requests
|
||||||
const resultId = args['_resultId'] as string | undefined;
|
const resultId = args['_resultId'] as string | undefined;
|
||||||
@@ -30,6 +47,13 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
|
|||||||
return handleSectionDrillDown(request, resultId, section, ctx);
|
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;
|
return null;
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -50,8 +74,20 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
|
|||||||
if (response.error) return response;
|
if (response.error) return response;
|
||||||
|
|
||||||
// Extract text content from the response
|
// Extract text content from the response
|
||||||
const raw = extractTextContent(response);
|
const extracted = extractTextContent(response);
|
||||||
if (!raw || raw.length <= 2000) return 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 {
|
try {
|
||||||
const result = await ctx.processContent(toolName, raw, 'toolResult');
|
const result = await ctx.processContent(toolName, raw, 'toolResult');
|
||||||
@@ -61,7 +97,7 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
|
|||||||
const resultId = `pm-${Date.now().toString(36)}`;
|
const resultId = `pm-${Date.now().toString(36)}`;
|
||||||
storeSections(ctx, resultId, result.sections);
|
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 {
|
return {
|
||||||
jsonrpc: '2.0',
|
jsonrpc: '2.0',
|
||||||
id: response.id,
|
id: response.id,
|
||||||
@@ -71,21 +107,174 @@ export function createContentPipelinePlugin(): ProxyModelPlugin {
|
|||||||
|
|
||||||
// Pipeline ran but no sections — return processed content if it changed
|
// Pipeline ran but no sections — return processed content if it changed
|
||||||
if (result.content !== raw) {
|
if (result.content !== raw) {
|
||||||
return {
|
return textResponse(response.id, result.content);
|
||||||
jsonrpc: '2.0',
|
|
||||||
id: response.id,
|
|
||||||
result: { content: [{ type: 'text', text: result.content }] },
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Pipeline failed — return original response
|
// Pipeline failed — fall through, but keep the unwrap
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (unwrapped) return textResponse(response.id, raw);
|
||||||
|
|
||||||
return response;
|
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. */
|
/** Extract text content from a tool result response. */
|
||||||
function extractTextContent(response: JsonRpcResponse): string | null {
|
function extractTextContent(response: JsonRpcResponse): string | null {
|
||||||
if (!response.result || typeof response.result !== 'object') return 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. */
|
/** Find a section by ID, searching recursively through children. */
|
||||||
function findSection(sections: Section[], id: string): Section | null {
|
function findSection(sections: Section[], id: string): Section | null {
|
||||||
for (const section of sections) {
|
for (const section of sections) {
|
||||||
|
|||||||
@@ -10,6 +10,7 @@
|
|||||||
* overlap on hooks, no conflicts arise.
|
* overlap on hooks, no conflicts arise.
|
||||||
*/
|
*/
|
||||||
import type { ProxyModelPlugin } from '../plugin.js';
|
import type { ProxyModelPlugin } from '../plugin.js';
|
||||||
|
import type { ToolDefinition } from '../types.js';
|
||||||
import { createGatePlugin, type GatePluginConfig } from './gate.js';
|
import { createGatePlugin, type GatePluginConfig } from './gate.js';
|
||||||
import { createContentPipelinePlugin } from './content-pipeline.js';
|
import { createContentPipelinePlugin } from './content-pipeline.js';
|
||||||
|
|
||||||
@@ -59,8 +60,15 @@ export function createDefaultPlugin(config: DefaultPluginConfig = {}): ProxyMode
|
|||||||
if (gate.onInitialize) {
|
if (gate.onInitialize) {
|
||||||
plugin.onInitialize = gate.onInitialize.bind(gate);
|
plugin.onInitialize = gate.onInitialize.bind(gate);
|
||||||
}
|
}
|
||||||
if (gate.onToolsList) {
|
// Tools list: gate first (it decides which tools are visible at all), then
|
||||||
plugin.onToolsList = gate.onToolsList.bind(gate);
|
// 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) {
|
if (pipeline.onToolCallAfter) {
|
||||||
plugin.onToolCallAfter = pipeline.onToolCallAfter.bind(pipeline);
|
plugin.onToolCallAfter = pipeline.onToolCallAfter.bind(pipeline);
|
||||||
|
|||||||
@@ -37,7 +37,9 @@ const handler: StageHandler = async (content, ctx) => {
|
|||||||
).join('\n');
|
).join('\n');
|
||||||
|
|
||||||
return {
|
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,
|
sections,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -65,7 +65,7 @@ const handler: StageHandler = async (content, ctx) => {
|
|||||||
}).join('\n');
|
}).join('\n');
|
||||||
|
|
||||||
return {
|
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,
|
sections,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ const handler: StageHandler = async (content, ctx) => {
|
|||||||
|
|
||||||
const summary = await cachedSummarize(ctx, ctx.originalContent, maxTokens);
|
const summary = await cachedSummarize(ctx, ctx.originalContent, maxTokens);
|
||||||
return {
|
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 }],
|
sections: [{ id: 'full', title: 'Full Content', content: ctx.originalContent }],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -56,7 +56,7 @@ const handler: StageHandler = async (content, ctx) => {
|
|||||||
}).join('\n');
|
}).join('\n');
|
||||||
|
|
||||||
return {
|
return {
|
||||||
content: `${tree.length} sections:\n${toc}\n\nUse section parameter to read details.`,
|
content: `${tree.length} sections:\n${toc}`,
|
||||||
sections: tree,
|
sections: tree,
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1191,6 +1191,6 @@ function injectSectionsIntoPromptResponse(
|
|||||||
if (now - entry.createdAt > SECTION_TTL_MS) store.delete(key);
|
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);
|
return replacePromptText(response, text);
|
||||||
}
|
}
|
||||||
|
|||||||
339
src/mcplocal/tests/plugin-content-pipeline-drilldown.test.ts
Normal file
339
src/mcplocal/tests/plugin-content-pipeline-drilldown.test.ts
Normal 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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -261,9 +261,11 @@ describe('Prompt section drill-down', () => {
|
|||||||
expect(result.sections).toBeDefined();
|
expect(result.sections).toBeDefined();
|
||||||
expect(result.sections!.length).toBeGreaterThanOrEqual(3);
|
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('sections');
|
||||||
expect(result.content).toContain('Use section parameter');
|
expect(result.content).not.toContain('Use section parameter');
|
||||||
|
|
||||||
// Original was ~16K, TOC should be much shorter
|
// Original was ~16K, TOC should be much shorter
|
||||||
expect(result.content.length).toBeLessThan(largePrompt.length);
|
expect(result.content.length).toBeLessThan(largePrompt.length);
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
* Verifies that large prompts served via prompts/get are section-split
|
* Verifies that large prompts served via prompts/get are section-split
|
||||||
* and that subsequent calls with _resultId + _section return cached sections.
|
* 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 { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||||
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';
|
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';
|
||||||
|
|||||||
@@ -6,7 +6,7 @@
|
|||||||
*
|
*
|
||||||
* Prerequisites:
|
* Prerequisites:
|
||||||
* - mcplocal running on localhost:3200
|
* - 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)
|
* - smoke-aws-docs server deployed (runtime: python)
|
||||||
*
|
*
|
||||||
* The test suite uses the fixture at fixtures/smoke-data.yaml which
|
* The test suite uses the fixture at fixtures/smoke-data.yaml which
|
||||||
|
|||||||
109
src/mcplocal/tests/smoke/tool-drilldown.test.ts
Normal file
109
src/mcplocal/tests/smoke/tool-drilldown.test.ts
Normal 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();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user