diff --git a/mcp/README.md b/mcp/README.md index 702eec3d60..3189a4dea3 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -265,7 +265,8 @@ The Penpot MCP server can be configured using environment variables. | `PENPOT_MCP_SERVER_PORT` | Port for the HTTP/SSE server | `4401` | | `PENPOT_MCP_WEBSOCKET_PORT` | Port for the WebSocket server (plugin connection) | `4402` | | `PENPOT_MCP_REPL_PORT` | Port for the REPL server (development/debugging) | `4403` | -| `PENPOT_MCP_REPL_ENABLE` | Explicitly enable/disable the REPL server. Set to `true` to enable. When unset, defaults to the value of `PENPOT_MCP_DEVENV`. | (unset) | +| `PENPOT_MCP_REPL_HOST` | Address on which the REPL server listens (binds to) | `localhost` | +| `PENPOT_MCP_REPL_ENABLE` | Explicitly enable/disable the REPL server. Set to `true` to enable. When unset, defaults to the value of `PENPOT_MCP_DEVENV`. The REPL server never starts in multi-user mode. | (unset) | | `PENPOT_MCP_REMOTE_MODE` | Enable remote mode (disables file system access). Set to `true` to enable. | `false` | | `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools in local single-user mode. Set to `true` to enable. | `false` | | `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` | diff --git a/mcp/packages/server/src/PenpotMcpServer.test.ts b/mcp/packages/server/src/PenpotMcpServer.test.ts index 68665359f5..2d35e70589 100644 --- a/mcp/packages/server/src/PenpotMcpServer.test.ts +++ b/mcp/packages/server/src/PenpotMcpServer.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { PenpotMcpServer, shouldRegisterDeveloperTools } from "./PenpotMcpServer"; +import { PenpotMcpServer, shouldRegisterDeveloperTools, shouldStartReplServer } from "./PenpotMcpServer"; test("registers developer tools in local devenv mode", () => { assert.equal(shouldRegisterDeveloperTools(true, false), true); @@ -14,6 +14,22 @@ test("does not register developer tools when devenv mode is disabled", () => { assert.equal(shouldRegisterDeveloperTools(false, false), false); }); +test("starts REPL server in single-user mode when enabled", () => { + assert.equal(shouldStartReplServer(true, false), true); +}); + +test("does not start REPL server in multi-user mode even when enabled", () => { + assert.equal(shouldStartReplServer(true, true), false); +}); + +test("does not start REPL server when disabled in single-user mode", () => { + assert.equal(shouldStartReplServer(false, false), false); +}); + +test("does not start REPL server when disabled in multi-user mode", () => { + assert.equal(shouldStartReplServer(false, true), false); +}); + // ── Pure function tests ──────────────────────────────────────── test("isDevEnvEnabled returns false when PENPOT_MCP_DEVENV is not set", () => { @@ -125,6 +141,79 @@ test("constructor does not create ReplServer when PENPOT_MCP_REPL_ENABLE is 'fal } }); +test("constructor does not create ReplServer in multi-user mode even with DEVENV", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + delete process.env.PENPOT_MCP_REPL_ENABLE; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(true); + assert.equal(server.hasReplServer(), false); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable); + } +}); + +test("constructor does not create ReplServer in multi-user mode even with explicit REPL_ENABLE", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplEnable = process.env.PENPOT_MCP_REPL_ENABLE; + const prevPorts = setUniqueEnv(); + delete process.env.PENPOT_MCP_DEVENV; + process.env.PENPOT_MCP_REPL_ENABLE = "true"; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(true); + assert.equal(server.hasReplServer(), false); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_ENABLE", prevReplEnable); + } +}); + +test("replHost defaults to localhost and ignores SERVER_HOST", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevServerHost = process.env.PENPOT_MCP_SERVER_HOST; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + process.env.PENPOT_MCP_SERVER_HOST = "0.0.0.0"; + delete process.env.PENPOT_MCP_REPL_HOST; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), true); + assert.equal(server.replHost, "localhost"); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_SERVER_HOST", prevServerHost); + restoreOrDelete("PENPOT_MCP_REPL_HOST", prevReplHost); + } +}); + +test("replHost respects PENPOT_MCP_REPL_HOST", async () => { + const prevDevEnv = process.env.PENPOT_MCP_DEVENV; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; + const prevPorts = setUniqueEnv(); + process.env.PENPOT_MCP_DEVENV = "true"; + process.env.PENPOT_MCP_REPL_HOST = "0.0.0.0"; + let server: PenpotMcpServer | undefined; + try { + server = new PenpotMcpServer(false); + assert.equal(server.hasReplServer(), true); + assert.equal(server.replHost, "0.0.0.0"); + } finally { + await server?.stop(); + restoreEnv(prevDevEnv, prevPorts); + restoreOrDelete("PENPOT_MCP_REPL_HOST", prevReplHost); + } +}); + // ── Helpers ──────────────────────────────────────────────────── function setUniqueEnv() { @@ -132,15 +221,22 @@ function setUniqueEnv() { const prevServer = process.env.PENPOT_MCP_SERVER_PORT; const prevWs = process.env.PENPOT_MCP_WEBSOCKET_PORT; const prevRepl = process.env.PENPOT_MCP_REPL_PORT; + const prevReplHost = process.env.PENPOT_MCP_REPL_HOST; 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 }; + delete process.env.PENPOT_MCP_REPL_HOST; + return { prevServer, prevWs, prevRepl, prevReplHost }; } function restoreEnv( devEnv: string | undefined, - ports: { prevServer: string | undefined; prevWs: string | undefined; prevRepl: string | undefined } + ports: { + prevServer: string | undefined; + prevWs: string | undefined; + prevRepl: string | undefined; + prevReplHost: string | undefined; + } ) { if (devEnv !== undefined) { process.env.PENPOT_MCP_DEVENV = devEnv; @@ -150,6 +246,7 @@ function restoreEnv( restoreOrDelete("PENPOT_MCP_SERVER_PORT", ports.prevServer); restoreOrDelete("PENPOT_MCP_WEBSOCKET_PORT", ports.prevWs); restoreOrDelete("PENPOT_MCP_REPL_PORT", ports.prevRepl); + restoreOrDelete("PENPOT_MCP_REPL_HOST", ports.prevReplHost); } function restoreOrDelete(key: string, value: string | undefined) { diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index c620aca3fc..4691c0541d 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -57,6 +57,16 @@ export function shouldRegisterDeveloperTools(isDevEnv: boolean, isMultiUserMode: return isDevEnv && !isMultiUserMode; } +/** + * Indicates whether the REPL server may be started for the current server mode. + * + * The REPL server never starts in multi-user mode, even when explicitly + * enabled, mirroring the developer tools policy. + */ +export function shouldStartReplServer(isReplEnabled: boolean, isMultiUserMode: boolean): boolean { + return isReplEnabled && !isMultiUserMode; +} + export class PenpotMcpServer { /** * Timeout, in minutes, for idle sessions (Streamable HTTP and SSE) before they are automatically closed and removed. @@ -133,6 +143,7 @@ export class PenpotMcpServer { public readonly host: string; public readonly port: number; public readonly webSocketPort: number; + public readonly replHost: string; public readonly replPort: number; private sessionTimeoutInterval: ReturnType | undefined; @@ -156,6 +167,7 @@ export class PenpotMcpServer { this.host = process.env.PENPOT_MCP_SERVER_HOST ?? "localhost"; this.port = parseInt(process.env.PENPOT_MCP_SERVER_PORT ?? "4401", 10); this.webSocketPort = parseInt(process.env.PENPOT_MCP_WEBSOCKET_PORT ?? "4402", 10); + this.replHost = process.env.PENPOT_MCP_REPL_HOST ?? "localhost"; this.replPort = parseInt(process.env.PENPOT_MCP_REPL_PORT ?? "4403", 10); this.tenant = process.env.PENPOT_TENANT ?? "default"; const toolTimeoutSecs = parseInt(process.env.PENPOT_MCP_TOOL_TIMEOUT_S ?? "120", 10); @@ -181,8 +193,8 @@ export class PenpotMcpServer { this.pluginBridge = new PluginBridge(this, this.webSocketPort, toolTimeoutSecs, this.redisBridge); - if (PenpotMcpServer.isReplEnabled(process.env)) { - this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.host); + if (shouldStartReplServer(PenpotMcpServer.isReplEnabled(process.env), this.isMultiUserMode())) { + this.replServer = new ReplServer(this.pluginBridge, this.replPort, this.replHost); } else { this.replServer = null; } @@ -232,9 +244,10 @@ export class PenpotMcpServer { /** * Indicates whether the REPL server was created. * - * The REPL server is created when {@link isReplEnabled} returns true, - * which means either ``PENPOT_MCP_REPL_ENABLE=true`` or, when that - * variable is unset, ``PENPOT_MCP_DEVENV=true``. + * The REPL server is created when {@link isReplEnabled} returns true and + * the server is not running in multi-user mode, which means either + * ``PENPOT_MCP_REPL_ENABLE=true`` or, when that variable is unset, + * ``PENPOT_MCP_DEVENV=true``, in single-user mode. */ public hasReplServer(): boolean { return this.replServer !== null; @@ -471,6 +484,8 @@ export class PenpotMcpServer { // start the REPL server (devenv only) and session timeout checker if (this.replServer) { await this.replServer.start(); + } else if (this.isMultiUserMode()) { + this.logger.info("REPL server disabled in multi-user mode (never started with --multi-user)"); } else { this.logger.info( "REPL server disabled (set PENPOT_MCP_REPL_ENABLE=true or PENPOT_MCP_DEVENV=true to enable)" diff --git a/mcp/scripts/start-mcp-devenv b/mcp/scripts/start-mcp-devenv index 62e957a9eb..9098a8b254 100755 --- a/mcp/scripts/start-mcp-devenv +++ b/mcp/scripts/start-mcp-devenv @@ -3,4 +3,4 @@ # This starts the MCP server in a configuration for Penpot development # (assuming devenv) -PENPOT_MCP_SERVER_HOST=0.0.0.0 PENPOT_MCP_REMOTE_MODE=true PENPOT_MCP_DEVENV=true pnpm run bootstrap +PENPOT_MCP_SERVER_HOST=0.0.0.0 PENPOT_MCP_REPL_HOST=0.0.0.0 PENPOT_MCP_REMOTE_MODE=true PENPOT_MCP_DEVENV=true pnpm run bootstrap