diff --git a/mcp/packages/server/src/PenpotMcpServer.test.ts b/mcp/packages/server/src/PenpotMcpServer.test.ts new file mode 100644 index 0000000000..99765b5927 --- /dev/null +++ b/mcp/packages/server/src/PenpotMcpServer.test.ts @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { PenpotMcpServer } from "./PenpotMcpServer"; + +// ── Pure function tests ──────────────────────────────────────── + +test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is not set", () => { + assert.equal(PenpotMcpServer.isDevEnvEnabled({}), false); +}); + +test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is 'false'", () => { + assert.equal(PenpotMcpServer.isDevEnvEnabled({ PENPOT_MCP_DEVENV: "false" }), false); +}); + +test("isDevEnvEnabled returns true when PENPOT_MCP_DEVENV is 'true'", () => { + assert.equal(PenpotMcpServer.isDevEnvEnabled({ PENPOT_MCP_DEVENV: "true" }), true); +}); + +// ── Integration tests: constructor gating ────────────────────── +// +// Each test uses unique ports to avoid conflicts when tests run +// in the same process. The server is stopped in the finally block +// to release the WebSocket port. + +let portCounter = 14_500; +function uniquePorts() { + const base = portCounter; + portCounter += 10; + return { server: base, ws: base + 1, repl: base + 2 }; +} + +test("constructor does not create ReplServer when PENPOT_MCP_DEVENV is unset", async () => { + const prev = process.env.PENPOT_MCP_DEVENV; + const prevPorts = setUniqueEnv(); + delete process.env.PENPOT_MCP_DEVENV; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), false); + } finally { + await server?.stop(); + restoreEnv(prev, prevPorts); + } +}); + +test("constructor creates ReplServer when PENPOT_MCP_DEVENV is 'true'", async () => { + const prev = process.env.PENPOT_MCP_DEVENV; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), true); + } finally { + await server?.stop(); + restoreEnv(prev, prevPorts); + } +}); + +// ── Helpers ──────────────────────────────────────────────────── + +function setUniqueEnv() { + const ports = uniquePorts(); + const prevServer = process.env.PENPOT_MCP_SERVER_PORT; + const prevWs = process.env.PENPOT_MCP_WEBSOCKET_PORT; + const prevRepl = process.env.PENPOT_MCP_REPL_PORT; + process.env.PENPOT_MCP_SERVER_PORT = String(ports.server); + process.env.PENPOT_MCP_WEBSOCKET_PORT = String(ports.ws); + process.env.PENPOT_MCP_REPL_PORT = String(ports.repl); + return { prevServer, prevWs, prevRepl }; +} + +function restoreEnv( + devEnv: string | undefined, + ports: { prevServer: string | undefined; prevWs: string | undefined; prevRepl: string | undefined } +) { + if (devEnv !== undefined) { + process.env.PENPOT_MCP_DEVENV = devEnv; + } else { + delete process.env.PENPOT_MCP_DEVENV; + } + restoreOrDelete("PENPOT_MCP_SERVER_PORT", ports.prevServer); + restoreOrDelete("PENPOT_MCP_WEBSOCKET_PORT", ports.prevWs); + restoreOrDelete("PENPOT_MCP_REPL_PORT", ports.prevRepl); +} + +function restoreOrDelete(key: string, value: string | undefined) { + if (value !== undefined) { + process.env[key] = value; + } else { + delete process.env[key]; + } +} diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index bd992ec108..49137b48b7 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -56,6 +56,16 @@ export class PenpotMcpServer { */ private static readonly SESSION_TIMEOUT_MINUTES = 60; + /** + * Determines whether the server is running in a Penpot development + * environment, based on the given environment variables. + * + * Returns ``true`` only when ``PENPOT_MCP_DEVENV`` is ``"true"``. + */ + public static isDevEnvEnabled(env: Record): boolean { + return env.PENPOT_MCP_DEVENV === "true"; + } + /** * Returns a short, non-reversible fingerprint of a user token, suitable for * correlating log lines without exposing the full credential. @@ -83,7 +93,7 @@ export class PenpotMcpServer { public readonly configLoader: ConfigurationLoader; private app: any; public readonly pluginBridge: PluginBridge; - private readonly replServer: ReplServer; + private readonly replServer: ReplServer | null; private apiDocs: ApiDocs; private readonly penpotHighLevelOverview: string; private readonly connectionInstructions: string; @@ -149,7 +159,12 @@ export class PenpotMcpServer { } this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge); - this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); + + if (PenpotMcpServer.isDevEnvEnabled(process.env)) { + this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); + } else { + this.replServer = null; + } } /** @@ -190,7 +205,16 @@ export class PenpotMcpServer { * additional developer tools such as ClojureScript expression evaluation are exposed. */ public isDevEnv(): boolean { - return process.env.PENPOT_MCP_DEVENV === "true"; + return PenpotMcpServer.isDevEnvEnabled(process.env); + } + + /** + * Indicates whether the REPL server was created. + * + * The REPL server is created only in development environment mode. + */ + public hasReplServer(): boolean { + return this.replServer !== null; } /** @@ -421,8 +445,12 @@ export class PenpotMcpServer { this.logger.info(`Legacy SSE endpoint: http://${this.host}:${this.port}/sse`); this.logger.info(`WebSocket server URL: ws://${this.host}:${this.webSocketPort}`); - // start the REPL server and session timeout checker - await this.replServer.start(); + // start the REPL server (devenv only) and session timeout checker + if (this.replServer) { + await this.replServer.start(); + } else { + this.logger.info("REPL server disabled (set PENPOT_MCP_DEVENV=true to enable)"); + } this.startSessionTimeoutChecker(); resolve(); @@ -438,8 +466,11 @@ export class PenpotMcpServer { public async stop(): Promise { this.logger.info("Stopping Penpot MCP Server..."); clearInterval(this.sessionTimeoutInterval); + await this.pluginBridge.close(); await this.redisBridge?.close(); - await this.replServer.stop(); + if (this.replServer) { + await this.replServer.stop(); + } this.logger.info("Penpot MCP Server stopped"); } } diff --git a/mcp/packages/server/src/PluginBridge.ts b/mcp/packages/server/src/PluginBridge.ts index a362faef36..6a1e07f9c5 100644 --- a/mcp/packages/server/src/PluginBridge.ts +++ b/mcp/packages/server/src/PluginBridge.ts @@ -467,4 +467,16 @@ export class PluginBridge { task.rejectWithError(error instanceof Error ? error : new Error(String(error))); } } + + /** + * Closes the WebSocket server and all connected client sockets. + */ + public async close(): Promise { + return new Promise((resolve) => { + this.wsServer.close(() => { + this.logger.info("WebSocket server closed"); + resolve(); + }); + }); + } }