* feat(servers): persist server list to ~/.mcp-inspector/mcp.json (#1343) Replaces the hardcoded SEED_SERVERS in App.tsx with a file-backed list at ~/.mcp-inspector/mcp.json. Full CRUD round-trips through new /api/servers routes; first launch writes the two existing seeds if the file is absent. Backend (core): - store-io: getDefaultMcpConfigPath() — sibling of getDefaultStorageDir() - core/mcp/serverList.ts: pure converters MCPConfig <-> ServerEntry[], DEFAULT_SEED_CONFIG, and normalizeServerType (moved from node/config.ts to keep serverList.ts Node-free) - core/mcp/remote/node/server.ts: GET/POST/PUT/:id/DELETE/:id /api/servers routes, plus RemoteServerOptions.mcpConfigPath defaulting to getDefaultMcpConfigPath(). Ids validated with validateStoreId. Frontend: - core/react/useServers.ts: { servers, loading, error, refresh, addServer, updateServer, removeServer } over the new endpoints - App.tsx: drops SEED_SERVERS, wires useServers, disconnects on remove of the active server, follows id on rename of the active server - ServerConfigModal: Add/Edit/Clone form (stdio: command/args/env/cwd; sse/streamable-http: url/headers as one-per-line text) - ServerRemoveConfirmModal: confirm-on-remove with id + transport summary Tests (52 new): - unit: getDefaultMcpConfigPath env-var permutations, serverList round-trip, hook flows against real Hono app.fetch (no TCP), modal forms + validation - integration: /api/servers routes against a real createRemoteApp + tmp file - stories with play functions for both new modals Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(servers): address PR #1349 review (correctness + nits) Bugs: - normalizeServerType: validate against the allowed set and default unknown / non-string types to "stdio" (matches the missing-type case). Prevents a hand-edited {"type":"websocket"} from leaking past `as ServerType` and surprising downstream narrowing. - onConfirmRemove: when removing the active server, also destroy() the 9 state managers and null them + the client out so they GC immediately instead of waiting for the next server switch. - /api/servers mutating routes: serialize POST/PUT/DELETE through an in-process write-lock so concurrent read-modify-write cycles can't lose updates. New integration test fires 25 parallel POSTs and asserts every entry lands. - readMcpConfig comment: update to match actual behavior (valid-JSON file with no `mcpServers` → empty; invalid JSON → 500 from the outer try, so corruption surfaces rather than silently emptying the list). New integration tests pin the 500 path on both GET and POST. Nits: - Add --inspector-surface-subtle token, use it in ServerRemoveConfirmModal instead of the raw Mantine var. - Rename useServers buildHeaders boolean param to includeJsonBody. - Add a comment on the env-value vs header-value trim asymmetry (env values preserve trailing whitespace; header values follow RFC 7230). Tests: 1630 unit + 404 integration + 306 storybook all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ServerConfigModal): null currentTarget in functional setState updater React nulls SyntheticEvent.currentTarget after the synchronous handler returns. Reading e.currentTarget.value inside `setForm((f) => ...)` worked under happy-dom (which keeps currentTarget around) but threw "Cannot read properties of null (reading 'value')" on the first keystroke in a real browser. Capture the value before the functional updater closes over it. All seven onChange handlers in ServerConfigModal had the bug. Extended the AddEmpty storybook play to type into Server ID and Command — the real-Chromium runner now exercises this path so a future regression fails the test suite instead of the user. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(remote-server): surface stderr + transport_error when subprocess crashes during startup A stdio subprocess that spawns successfully but exits before the browser opens the SSE events stream used to vanish into a bare "Session not found" 404. The fix has three parts: 1. RemoteSession.markTransportDead always pushes the transport_error event (via pushEvent's queue) rather than dropping it when no consumer is attached. The queued event drains when the consumer eventually arrives. 2. /api/mcp/connect's transport.onclose handler no longer deletes the session immediately when no consumer is attached. Instead it holds the session (with queued stderr + transport_error) for a 30s grace window so the events endpoint can drain it. A timeout sweeps if no consumer ever connects. 3. /api/mcp/events detects isTransportDead after attaching the consumer (which just drained the queue), yields once to flush, then closes the stream and deletes the session. End result: the user sees the actual error (e.g. "Cannot find module" from stderr + the transport-closed reason) instead of a confusing 404. Tests: - New unit tests (clients/web/src/test/integration/mcp/remote/remote-session.test.ts) cover the queueing behavior, live delivery, and clearEventConsumer signaling under all transport-state permutations. - New integration test (connect-crash.test.ts) spawns a node subprocess that writes to stderr and exits, then verifies the events stream delivers both the stderr line and the transport_error event before closing — the exact scenario from PR #1349. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(state): gate listRequestorTasks on the server's `tasks` capability ManagedRequestorTasksState was calling tasks/list on every connect. Servers that don't advertise the `tasks` capability (the common case) reply with -32601 "Method not found", which then surfaces in the inspector's console for every connect. Skip the call when the server doesn't declare `tasks` — empty tasks list is the right semantics. Existing tests opt into capabilities so the live-path coverage is preserved; two new tests pin the gated behavior (direct refresh + connect-event-triggered refresh). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(App): clear activeServerId on disconnect so other server cards re-enable ServerCard dims any card whose id differs from `activeServer` (and marks it inert), so cards stay non-interactive after a disconnect until the user reconnects elsewhere. activeServerId was set on connect but never reset on disconnect, leaving the whole screen locked to the just- disconnected card. Subscribe to InspectorClient's `disconnect` event and clear activeServerId there. Covers all three disconnect paths uniformly: - user toggles the connected card off - user clicks the header Disconnect button - mid-session transport failure (subprocess exit / network drop) Event-based instead of effect-based on connectionStatus to avoid the first-render-clobbers-new-id trap (status starts as "disconnected" for a freshly-created client before connect() runs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
e9ac57c548
commit
db1916cd34
3 changed files with 3176 additions and 137 deletions
|
|
@ -0,0 +1,153 @@
|
|||
/**
|
||||
* Regression test for the crash-on-startup race: when a stdio subprocess
|
||||
* spawns successfully but exits before the browser opens the SSE events
|
||||
* stream, the user should see the actual error (stderr + transport_error)
|
||||
* — not a bare "Session not found" 404.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, afterEach, beforeEach } from "vitest";
|
||||
import { serve } from "@hono/node-server";
|
||||
import type { ServerType } from "@hono/node-server";
|
||||
import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js";
|
||||
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
|
||||
|
||||
interface Harness {
|
||||
baseUrl: string;
|
||||
server: ServerType;
|
||||
}
|
||||
|
||||
async function startServer(): Promise<Harness> {
|
||||
const { app } = createRemoteApp({
|
||||
dangerouslyOmitAuth: true,
|
||||
initialConfig: { defaultEnvironment: {} },
|
||||
});
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = serve(
|
||||
{ fetch: app.fetch, port: 0, hostname: "127.0.0.1" },
|
||||
(info) => {
|
||||
const port =
|
||||
info && typeof info === "object" && "port" in info
|
||||
? (info as { port: number }).port
|
||||
: 0;
|
||||
resolve({ baseUrl: `http://127.0.0.1:${port}`, server });
|
||||
},
|
||||
);
|
||||
server.on("error", reject);
|
||||
});
|
||||
}
|
||||
|
||||
async function teardown(h: Harness): Promise<void> {
|
||||
await new Promise<void>((resolve) => h.server.close(() => resolve()));
|
||||
}
|
||||
|
||||
interface ParsedSseEvent {
|
||||
type: string;
|
||||
data: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a streaming Response body and parse Server-Sent Events. Stops when
|
||||
* the server closes the stream (read returns done).
|
||||
*
|
||||
* The SSE `data:` field carries the full SessionEvent JSON (both `type` and
|
||||
* `data`), per the writer in /api/mcp/events — we unwrap and surface just
|
||||
* the inner `.data` as the event payload, so callers can read e.g.
|
||||
* `event.data.message` for stdio_log without an extra hop.
|
||||
*/
|
||||
async function readSseEvents(res: Response): Promise<ParsedSseEvent[]> {
|
||||
if (!res.body) return [];
|
||||
const reader = res.body.getReader();
|
||||
const decoder = new TextDecoder();
|
||||
let buffer = "";
|
||||
const events: ParsedSseEvent[] = [];
|
||||
|
||||
while (true) {
|
||||
const { value, done } = await reader.read();
|
||||
if (value) buffer += decoder.decode(value, { stream: true });
|
||||
// SSE events terminate with a blank line — split on it.
|
||||
let split: number;
|
||||
while ((split = buffer.indexOf("\n\n")) !== -1) {
|
||||
const block = buffer.slice(0, split);
|
||||
buffer = buffer.slice(split + 2);
|
||||
let type: string | undefined;
|
||||
let dataLine: string | undefined;
|
||||
for (const line of block.split("\n")) {
|
||||
if (line.startsWith("event:"))
|
||||
type = line.slice("event:".length).trim();
|
||||
else if (line.startsWith("data:"))
|
||||
dataLine = line.slice("data:".length).trim();
|
||||
}
|
||||
if (type && dataLine) {
|
||||
const parsed = JSON.parse(dataLine) as { data?: unknown };
|
||||
events.push({ type, data: parsed.data });
|
||||
}
|
||||
}
|
||||
if (done) break;
|
||||
}
|
||||
return events;
|
||||
}
|
||||
|
||||
describe("crash-on-startup error surfacing", () => {
|
||||
let h: Harness;
|
||||
|
||||
beforeEach(async () => {
|
||||
h = await startServer();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await teardown(h);
|
||||
});
|
||||
|
||||
it("surfaces stderr + transport_error when the subprocess exits before the events stream opens", async () => {
|
||||
// node -e writes to stderr then exits non-zero. The transport itself
|
||||
// spawns fine (so POST /api/mcp/connect returns 200), but the process
|
||||
// is dead by the time the browser opens /api/mcp/events. Pre-fix, the
|
||||
// session was deleted in transport.onclose and the events endpoint
|
||||
// returned a bare 404; now it returns an SSE stream that drains the
|
||||
// queued stderr + transport_error events and closes.
|
||||
const config: MCPServerConfig = {
|
||||
type: "stdio",
|
||||
command: process.execPath, // current `node` binary
|
||||
args: [
|
||||
"-e",
|
||||
"process.stderr.write('boom from script\\n'); process.exit(1);",
|
||||
],
|
||||
};
|
||||
|
||||
const connectRes = await fetch(`${h.baseUrl}/api/mcp/connect`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ config }),
|
||||
});
|
||||
expect(connectRes.status).toBe(200);
|
||||
const { sessionId } = (await connectRes.json()) as { sessionId: string };
|
||||
expect(typeof sessionId).toBe("string");
|
||||
|
||||
// Give the subprocess time to die before opening the events stream.
|
||||
await new Promise((resolve) => setTimeout(resolve, 200));
|
||||
|
||||
const eventsRes = await fetch(
|
||||
`${h.baseUrl}/api/mcp/events?sessionId=${sessionId}`,
|
||||
);
|
||||
expect(eventsRes.status).toBe(200);
|
||||
expect(eventsRes.headers.get("content-type")).toMatch(/event-stream/);
|
||||
|
||||
const events = await readSseEvents(eventsRes);
|
||||
|
||||
// Stderr event for "boom from script" plus a transport_error closing
|
||||
// the stream — the user now sees the real reason instead of "Session
|
||||
// not found".
|
||||
const stderrEvents = events.filter((e) => e.type === "stdio_log");
|
||||
expect(stderrEvents.length).toBeGreaterThan(0);
|
||||
const stderrText = stderrEvents
|
||||
.map((e) => (e.data as { message?: string } | undefined)?.message ?? "")
|
||||
.join("\n");
|
||||
expect(stderrText).toMatch(/boom from script/);
|
||||
|
||||
const transportError = events.find((e) => e.type === "transport_error");
|
||||
expect(transportError).toBeDefined();
|
||||
const err = transportError?.data as { error: string; code: number };
|
||||
expect(err.error).toMatch(/Transport closed/);
|
||||
expect(err.code).toBe(-32000);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,78 @@
|
|||
/**
|
||||
* Unit-level tests for RemoteSession's event queue + transport-death wiring.
|
||||
*
|
||||
* Lives under integration/ because RemoteSession imports SDK types that
|
||||
* pull in node-only modules at runtime; the file is otherwise pure (no I/O,
|
||||
* no network) so the integration runner is comfortable for it.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { RemoteSession } from "@inspector/core/mcp/remote/node/remote-session.js";
|
||||
|
||||
describe("RemoteSession", () => {
|
||||
it("queues events before a consumer attaches and drains them on attach", () => {
|
||||
const session = new RemoteSession("s1");
|
||||
session.onStderr({ timestamp: new Date(), message: "line one" });
|
||||
session.onStderr({ timestamp: new Date(), message: "line two" });
|
||||
|
||||
const received: { type: string }[] = [];
|
||||
session.setEventConsumer((event) => {
|
||||
received.push({ type: event.type });
|
||||
});
|
||||
|
||||
expect(received.map((e) => e.type)).toEqual(["stdio_log", "stdio_log"]);
|
||||
});
|
||||
|
||||
it("queues the transport_error event when markTransportDead fires before a consumer attaches", () => {
|
||||
// Regression: the previous behavior dropped the event when no consumer
|
||||
// was attached, so a process that crashed during startup vanished into
|
||||
// a bare "Session not found" 404 on the next /api/mcp/events poll.
|
||||
const session = new RemoteSession("s2");
|
||||
session.onStderr({
|
||||
timestamp: new Date(),
|
||||
message: "Error: Cannot find module 'bogus.js'",
|
||||
});
|
||||
session.markTransportDead("Transport closed - process may have exited");
|
||||
|
||||
const received: { type: string; data: unknown }[] = [];
|
||||
session.setEventConsumer((event) => {
|
||||
received.push({ type: event.type, data: event.data });
|
||||
});
|
||||
|
||||
expect(received.map((e) => e.type)).toEqual([
|
||||
"stdio_log",
|
||||
"transport_error",
|
||||
]);
|
||||
const err = received[1]?.data as { error: string; code: number };
|
||||
expect(err.error).toMatch(/Transport closed/);
|
||||
expect(err.code).toBe(-32000);
|
||||
});
|
||||
|
||||
it("delivers transport_error live when a consumer is already attached", () => {
|
||||
const session = new RemoteSession("s3");
|
||||
const received: { type: string }[] = [];
|
||||
session.setEventConsumer((event) => {
|
||||
received.push({ type: event.type });
|
||||
});
|
||||
session.markTransportDead("transport closed");
|
||||
expect(received.map((e) => e.type)).toEqual(["transport_error"]);
|
||||
});
|
||||
|
||||
it("isTransportDead + getTransportError reflect the marked state", () => {
|
||||
const session = new RemoteSession("s4");
|
||||
expect(session.isTransportDead()).toBe(false);
|
||||
expect(session.getTransportError()).toBeNull();
|
||||
session.markTransportDead("boom");
|
||||
expect(session.isTransportDead()).toBe(true);
|
||||
expect(session.getTransportError()).toBe("boom");
|
||||
});
|
||||
|
||||
it("clearEventConsumer signals cleanup-needed when the transport is dead", () => {
|
||||
const session = new RemoteSession("s5");
|
||||
session.setEventConsumer(() => {});
|
||||
expect(session.clearEventConsumer()).toBe(false);
|
||||
session.setEventConsumer(() => {});
|
||||
session.markTransportDead("boom");
|
||||
expect(session.clearEventConsumer()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,439 @@
|
|||
/**
|
||||
* Integration tests for /api/servers + /api/servers/:id routes.
|
||||
* Spins up createRemoteApp against a per-test tmp mcpConfigPath and
|
||||
* exercises the routes via real HTTP.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, afterEach, beforeEach } from "vitest";
|
||||
import {
|
||||
mkdtempSync,
|
||||
readFileSync,
|
||||
rmSync,
|
||||
existsSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { serve } from "@hono/node-server";
|
||||
import type { ServerType } from "@hono/node-server";
|
||||
import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js";
|
||||
import { DEFAULT_SEED_CONFIG } from "@inspector/core/mcp/serverList.js";
|
||||
import type { MCPConfig } from "@inspector/core/mcp/types.js";
|
||||
|
||||
interface Harness {
|
||||
baseUrl: string;
|
||||
server: ServerType;
|
||||
configPath: string;
|
||||
tempDir: string;
|
||||
}
|
||||
|
||||
async function startServer(configPath: string): Promise<{
|
||||
baseUrl: string;
|
||||
server: ServerType;
|
||||
}> {
|
||||
const { app } = createRemoteApp({
|
||||
dangerouslyOmitAuth: true,
|
||||
mcpConfigPath: configPath,
|
||||
initialConfig: { defaultEnvironment: {} },
|
||||
});
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = serve(
|
||||
{ fetch: app.fetch, port: 0, hostname: "127.0.0.1" },
|
||||
(info) => {
|
||||
const port =
|
||||
info && typeof info === "object" && "port" in info
|
||||
? (info as { port: number }).port
|
||||
: 0;
|
||||
resolve({ baseUrl: `http://127.0.0.1:${port}`, server });
|
||||
},
|
||||
);
|
||||
server.on("error", reject);
|
||||
});
|
||||
}
|
||||
|
||||
async function setup(): Promise<Harness> {
|
||||
const tempDir = mkdtempSync(join(tmpdir(), "inspector-servers-route-"));
|
||||
const configPath = join(tempDir, "mcp.json");
|
||||
const { baseUrl, server } = await startServer(configPath);
|
||||
return { baseUrl, server, configPath, tempDir };
|
||||
}
|
||||
|
||||
async function teardown(h: Harness): Promise<void> {
|
||||
await new Promise<void>((resolve) => h.server.close(() => resolve()));
|
||||
try {
|
||||
rmSync(h.tempDir, { recursive: true });
|
||||
} catch {
|
||||
/* ignore */
|
||||
}
|
||||
}
|
||||
|
||||
function readConfig(path: string): MCPConfig {
|
||||
return JSON.parse(readFileSync(path, "utf-8")) as MCPConfig;
|
||||
}
|
||||
|
||||
describe("/api/servers routes", () => {
|
||||
let h: Harness;
|
||||
|
||||
beforeEach(async () => {
|
||||
h = await setup();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await teardown(h);
|
||||
});
|
||||
|
||||
describe("GET /api/servers", () => {
|
||||
it("writes the seed config and returns it on first read (file absent)", async () => {
|
||||
expect(existsSync(h.configPath)).toBe(false);
|
||||
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`);
|
||||
expect(res.status).toBe(200);
|
||||
const body = (await res.json()) as MCPConfig;
|
||||
expect(body).toEqual(DEFAULT_SEED_CONFIG);
|
||||
|
||||
// File was created with the same content
|
||||
expect(existsSync(h.configPath)).toBe(true);
|
||||
expect(readConfig(h.configPath)).toEqual(DEFAULT_SEED_CONFIG);
|
||||
});
|
||||
|
||||
it("returns the existing file content when present (no overwrite)", async () => {
|
||||
const custom: MCPConfig = {
|
||||
mcpServers: {
|
||||
custom: { type: "stdio", command: "node", args: ["x.js"] },
|
||||
},
|
||||
};
|
||||
writeFileSync(h.configPath, JSON.stringify(custom, null, 2));
|
||||
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.json()).toEqual(custom);
|
||||
|
||||
// Untouched on disk
|
||||
expect(readConfig(h.configPath)).toEqual(custom);
|
||||
});
|
||||
|
||||
it("normalizes legacy 'http' and missing type on read", async () => {
|
||||
writeFileSync(
|
||||
h.configPath,
|
||||
JSON.stringify({
|
||||
mcpServers: {
|
||||
legacy: { command: "node" },
|
||||
httpish: { type: "http", url: "https://x.test" },
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`);
|
||||
const body = (await res.json()) as MCPConfig;
|
||||
expect(body.mcpServers.legacy?.type).toBe("stdio");
|
||||
expect(body.mcpServers.httpish?.type).toBe("streamable-http");
|
||||
});
|
||||
|
||||
it("treats a valid-JSON file without `mcpServers` as empty", async () => {
|
||||
writeFileSync(h.configPath, JSON.stringify({ unrelated: 1 }));
|
||||
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.json()).toEqual({ mcpServers: {} });
|
||||
});
|
||||
|
||||
it("surfaces a 500 (not silent empty) on invalid-JSON contents", async () => {
|
||||
// Surfacing corruption rather than silently presenting "no servers" —
|
||||
// the next POST/PUT/DELETE would otherwise read empty and clobber the
|
||||
// user's broken-but-recoverable file.
|
||||
writeFileSync(h.configPath, "not json {");
|
||||
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`);
|
||||
expect(res.status).toBe(500);
|
||||
const body = (await res.json()) as { error?: string };
|
||||
expect(body.error).toMatch(/Failed to read server list/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/servers", () => {
|
||||
it("adds a new server and persists to disk", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "alpha",
|
||||
config: { type: "stdio", command: "node" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.json()).toEqual({ ok: true });
|
||||
|
||||
expect(readConfig(h.configPath).mcpServers.alpha).toEqual({
|
||||
type: "stdio",
|
||||
command: "node",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns 409 when the id already exists", async () => {
|
||||
writeFileSync(
|
||||
h.configPath,
|
||||
JSON.stringify({
|
||||
mcpServers: { alpha: { type: "stdio", command: "node" } },
|
||||
}),
|
||||
);
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "alpha",
|
||||
config: { type: "stdio", command: "other" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("rejects an id with path-traversal characters", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "../escape",
|
||||
config: { type: "stdio", command: "node" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects a missing or non-object config", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ id: "alpha" }),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects malformed JSON body", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: "not json",
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("returns 500 when the existing file is invalid JSON (matches GET semantics)", async () => {
|
||||
writeFileSync(h.configPath, "not json {");
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "alpha",
|
||||
config: { type: "stdio", command: "node" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(500);
|
||||
});
|
||||
|
||||
it("normalizes the incoming config (http → streamable-http)", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "httpish",
|
||||
config: { type: "http", url: "https://x.test" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(readConfig(h.configPath).mcpServers.httpish?.type).toBe(
|
||||
"streamable-http",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("PUT /api/servers/:id", () => {
|
||||
beforeEach(() => {
|
||||
writeFileSync(
|
||||
h.configPath,
|
||||
JSON.stringify({
|
||||
mcpServers: {
|
||||
alpha: { type: "stdio", command: "old" },
|
||||
beta: { type: "stdio", command: "beta-cmd" },
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("updates config in place without renaming", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
config: { type: "stdio", command: "new" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const cfg = readConfig(h.configPath);
|
||||
expect(cfg.mcpServers.alpha).toEqual({ type: "stdio", command: "new" });
|
||||
// Key order preserved
|
||||
expect(Object.keys(cfg.mcpServers)).toEqual(["alpha", "beta"]);
|
||||
});
|
||||
|
||||
it("renames the key when id is supplied and different", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "alpha-renamed",
|
||||
config: { type: "stdio", command: "new" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const cfg = readConfig(h.configPath);
|
||||
expect(cfg.mcpServers).not.toHaveProperty("alpha");
|
||||
expect(cfg.mcpServers["alpha-renamed"]).toEqual({
|
||||
type: "stdio",
|
||||
command: "new",
|
||||
});
|
||||
// New key replaces the original in its slot, beta stays after
|
||||
expect(Object.keys(cfg.mcpServers)).toEqual(["alpha-renamed", "beta"]);
|
||||
});
|
||||
|
||||
it("returns 404 when the original id does not exist", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/nonexistent`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
config: { type: "stdio", command: "x" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("returns 409 when renaming to a key that already exists", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "beta",
|
||||
config: { type: "stdio", command: "x" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("rejects invalid original id", async () => {
|
||||
// dots fail validateStoreId; using `..` directly would be collapsed by
|
||||
// URL normalization before Hono routes the request.
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/bad.id`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
config: { type: "stdio", command: "x" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects an invalid new id", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id: "../escape",
|
||||
config: { type: "stdio", command: "x" },
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects a missing config", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ id: "alpha" }),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("rejects malformed JSON body", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "PUT",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: "not json",
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe("concurrent mutations", () => {
|
||||
it("does not lose updates when many adds fire in parallel (write-lock)", async () => {
|
||||
// Without the in-process mutex, concurrent POSTs would all read the
|
||||
// empty baseline and the last writer would clobber everyone else's
|
||||
// entry. With the mutex, every entry should land on disk.
|
||||
const ids = Array.from({ length: 25 }, (_, i) => `concurrent-${i}`);
|
||||
const responses = await Promise.all(
|
||||
ids.map((id) =>
|
||||
fetch(`${h.baseUrl}/api/servers`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
id,
|
||||
config: { type: "stdio", command: `cmd-${id}` },
|
||||
}),
|
||||
}),
|
||||
),
|
||||
);
|
||||
for (const res of responses) {
|
||||
expect(res.status).toBe(200);
|
||||
}
|
||||
const cfg = readConfig(h.configPath);
|
||||
expect(Object.keys(cfg.mcpServers).sort()).toEqual([...ids].sort());
|
||||
});
|
||||
});
|
||||
|
||||
describe("DELETE /api/servers/:id", () => {
|
||||
beforeEach(() => {
|
||||
writeFileSync(
|
||||
h.configPath,
|
||||
JSON.stringify({
|
||||
mcpServers: {
|
||||
alpha: { type: "stdio", command: "node" },
|
||||
beta: { type: "stdio", command: "node" },
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("removes the entry and persists", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/alpha`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
const cfg = readConfig(h.configPath);
|
||||
expect(cfg.mcpServers).not.toHaveProperty("alpha");
|
||||
expect(cfg.mcpServers).toHaveProperty("beta");
|
||||
});
|
||||
|
||||
it("is idempotent when the id is not present", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/nonexistent`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
// beta untouched
|
||||
expect(readConfig(h.configPath).mcpServers).toHaveProperty("beta");
|
||||
});
|
||||
|
||||
it("is a 200 no-op when the file does not exist yet", async () => {
|
||||
rmSync(h.configPath);
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/anything`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
|
||||
it("rejects an invalid id", async () => {
|
||||
const res = await fetch(`${h.baseUrl}/api/servers/bad.id`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue