Merge remote-tracking branch 'origin/staging' into develop

This commit is contained in:
Andrey Antukh 2026-09-22 13:44:56 +02:00
commit 476e25e371
13 changed files with 298 additions and 34 deletions

View File

@ -368,9 +368,17 @@
ptk/WatchEvent
(watch [_ state _]
(let [zoom-type (get-in state [:viewer-local :zoom-type])
params (rt/get-params state)]
(rx/of (rt/nav :viewer (assoc params :zoom zoom-type)))))))
params (rt/get-params state)
current (rt/get-query-param params :zoom)
expected (some-> zoom-type name)]
;; Zoom is view state: mirror it into the URL, replacing the history
;; entry, only when the query string does not already describe it.
(when (not= current expected)
(rx/of (rt/nav :viewer
(if (some? zoom-type)
(assoc params :zoom zoom-type)
(dissoc params :zoom))
{::rt/replace true})))))))
(def increase-zoom
(ptk/reify ::increase-zoom

View File

@ -210,7 +210,7 @@
ptk/WatchEvent
(watch [_ _ _]
(let [interactions (ctsi/update-interaction (:interactions shape) index update-fn)
interaction (nth interactions index)]
interaction (get interactions index)]
(rx/of
(dwsh/update-shapes
[(:id shape)]

View File

@ -105,7 +105,8 @@
:get #(-> % u/proxy->interaction :event-type format/format-key)
:set
(fn [_ value]
(let [value (parser/parse-keyword value)]
(let [value (parser/parse-keyword value)
index (locate-index)]
(cond
(not (contains? ctsi/event-types value))
(u/not-valid plugin-id :trigger value)
@ -113,11 +114,14 @@
(not (r/check-permission plugin-id "content:write"))
(u/not-valid plugin-id :trigger "Plugin doesn't have 'content:write' permission")
(nil? index)
(u/not-valid plugin-id :trigger "The interaction is not part of the shape anymore")
:else
(do
(st/emit! (dwi/update-interaction
(u/locate-shape file-id page-id shape-id)
(locate-index)
index
#(assoc % :event-type value)
{:page-id page-id}))
(swap! current assoc :event-type value)))))}
@ -127,21 +131,25 @@
:get #(-> % u/proxy->interaction :delay)
:set
(fn [_ value]
(cond
(or (not (sm/valid-safe-int? value)) (neg? value))
(u/not-valid plugin-id :delay value)
(let [index (locate-index)]
(cond
(or (not (sm/valid-safe-int? value)) (neg? value))
(u/not-valid plugin-id :delay value)
(not (r/check-permission plugin-id "content:write"))
(u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission")
(not (r/check-permission plugin-id "content:write"))
(u/not-valid plugin-id :delay "Plugin doesn't have 'content:write' permission")
:else
(do
(st/emit! (dwi/update-interaction
(u/locate-shape file-id page-id shape-id)
(locate-index)
#(assoc % :delay value)
{:page-id page-id}))
(swap! current assoc :delay value))))}
(nil? index)
(u/not-valid plugin-id :delay "The interaction is not part of the shape anymore")
:else
(do
(st/emit! (dwi/update-interaction
(u/locate-shape file-id page-id shape-id)
index
#(assoc % :delay value)
{:page-id page-id}))
(swap! current assoc :delay value)))))}
:action
{:this true
@ -149,21 +157,27 @@
:set
(fn [self value]
(let [params (parser/parse-action value)
index (locate-index)
interaction
(-> (u/proxy->interaction self)
(d/patch-object params))]
(cond
(not (sm/validate ctsi/schema:interaction interaction))
(u/not-valid plugin-id :action interaction)
(not (r/check-permission plugin-id "content:write"))
(u/not-valid plugin-id :action "Plugin doesn't have 'content:write' permission")
;; Precedes the schema check, which sees only the partial map that
;; patching a missing interaction produces.
(nil? index)
(u/not-valid plugin-id :action "The interaction is not part of the shape anymore")
(not (sm/validate ctsi/schema:interaction interaction))
(u/not-valid plugin-id :action interaction)
:else
(do
(st/emit! (dwi/update-interaction
(u/locate-shape file-id page-id shape-id)
(locate-index)
index
#(d/patch-object % params)
{:page-id page-id}))
(reset! current interaction)))))}

View File

@ -8,6 +8,8 @@
(:require
[app.common.uuid :as uuid]
[app.main.data.viewer :as dv]
[app.main.router :as rt]
[beicon.v2.core :as rx]
[cljs.test :as t]
[potok.v2.core :as ptk]))
@ -67,3 +69,56 @@
result (ptk/update dv/zoom-to-fill state)]
(t/is (= (get-in result [:viewer-local :zoom-type]) :fill))
(t/is (number? (get-in result [:viewer-local :zoom]))))))
(defn- watch-events
"Collect the events an event's watch emits synchronously."
[event state]
(let [out (atom [])]
(some-> (ptk/watch event state nil)
(rx/subscribe #(swap! out conj %)))
@out))
(defn- zoom-state
"Build a viewer state with the given `:zoom` query param and zoom type."
[zoom-param zoom-type]
{:route {:params {:query (cond-> {:page-id (str page-id) :index "0"}
(some? zoom-param)
(assoc :zoom zoom-param))}}
:viewer-local (cond-> {}
(some? zoom-type)
(assoc :zoom-type zoom-type))})
(t/deftest update-zoom-querystring-does-not-navigate-when-url-already-matches
(t/testing "zoom type already described by the query string"
(t/is (empty? (watch-events dv/update-zoom-querystring
(zoom-state "fit" :fit)))))
(t/testing "no zoom type and no zoom query param"
(t/is (empty? (watch-events dv/update-zoom-querystring
(zoom-state nil nil))))))
(t/deftest update-zoom-querystring-navigates-when-zoom-changes
(t/testing "zoom type differs from the query string"
(let [events (watch-events dv/update-zoom-querystring
(zoom-state "fit" :fill))
{:keys [id params options]} (some-> (first events) deref)]
(t/is (= 1 (count events)))
(t/is (= :viewer id))
(t/is (= :fill (:zoom params)))
(t/is (true? (::rt/replace options)))))
(t/testing "zoom query param absent, other params preserved"
(let [events (watch-events dv/update-zoom-querystring
(zoom-state nil :fit))
{:keys [params]} (some-> (first events) deref)]
(t/is (= 1 (count events)))
(t/is (= :fit (:zoom params)))
(t/is (= (str page-id) (:page-id params)))
(t/is (= "0" (:index params)))))
(t/testing "zoom type cleared drops the query param"
(let [events (watch-events dv/update-zoom-querystring
(zoom-state "fit" nil))
{:keys [params]} (some-> (first events) deref)]
(t/is (= 1 (count events)))
(t/is (not (contains? params :zoom))))))

View File

@ -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` |

View File

@ -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) {

View File

@ -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<typeof setInterval> | 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)"

View File

@ -94,7 +94,7 @@ export class PluginBridge {
private readonly taskTimeoutSecs: number,
private readonly redisBridge?: RedisBridge
) {
this.wsServer = new WebSocketServer({ port: port });
this.wsServer = new WebSocketServer({ port: port, host: mcpServer.host });
this.setupWebSocketHandlers();
}

View File

@ -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

View File

@ -10,6 +10,7 @@
- **plugins-runtime**: An interaction obtained from `Shape.interactions` now keeps addressing that interaction instead of the position it held when the array was read. Removing every interaction of a shape from a single read removes all of them rather than leaving some behind, and writing through a held interaction after an earlier one is removed no longer lands on a different interaction.
- **plugins-runtime**: `Shape.removeInteraction()` now rejects an interaction belonging to a different shape with a validation error, instead of removing whichever interaction sat at the same position on the target shape.
- **plugins-runtime**: Writing `trigger`, `delay` or `action` on an interaction the shape no longer has now raises a validation error. The write used to be sent to the workspace with no position to apply it at, where it failed out of the plugin's reach: nothing was written and nothing was reported.
- **plugins-runtime**: `Library.createComponent()` now rejects invalid input (an empty shape list, or a shape inside a component copy) with a validation error instead of returning a component proxy pointing at nothing.
- **plugins-runtime**: Setting an individual padding/margin side (`leftPadding`, `topMargin`, …) now re-derives the padding/margin type, switching to `multiple` when the four sides stop being symmetric (so the value is actually painted) and back to `simple` once top/bottom and left/right are mirrored again.
- **plugins-runtime**: Removed the premature deep-hardening of the host plugin context, which froze shared host functions (including `Function.prototype`) before SES override taming, causing `TypeError: Cannot assign to read only property 'toString'` on later host-side function extension. Related to #11001.

View File

@ -69,6 +69,27 @@ E2E_LOGIN_EMAIL=… E2E_LOGIN_PASSWORD=… \
- `PRINT_UNCOVERED=1` dumps the uncovered targets per interface; `PRINT_STATIC=1`
dumps the statically-covered ones (see [Coverage](#how-coverage-works-and-how-to-write-tests-that-move-it)).
### Errors the app reports on its own
An API call can leave the plugin happy and still break Penpot: an exception
raised inside an event handler is caught by the store, which reports it and
carries on, so nothing is thrown back across the sandbox. The CI runner watches
the page console for the prefixes Penpot's error handler prints (`Internal
Error`, `Unexpected Error`, `Assertion Error`, `Uncaught Exception`, `Uncaught
Rejection`) and for uncaught page errors, and fails the test that was running.
Two consequences when writing a test for such a case:
- Await the API (`await ctx.penpot.waitForLayoutUpdate()`) after the operation,
so the message reaches the console before the test ends and is attributed to
it rather than to the next one.
- This runs in the CI runner only. The plugin UI cannot read the page console,
so the same test shows green there — check it with `test:ci` or
`test:ci:mocked`.
`Plugin Error` and `Network Error` are not watched: tests provoke both on
purpose.
CI entry points reuse the exact same test files (`src/ci/headless.ts` discovers
them the same way the plugin does).

View File

@ -34,6 +34,13 @@ const frontendDir = resolve(repoRoot, 'frontend');
const staticRoot = resolve(frontendDir, 'resources/public');
const e2eDataDir = resolve(frontendDir, 'playwright/data');
// Console prefixes Penpot's error handler prints for failures the app did not
// expect (`frontend/src/app/main/errors.cljs`). The store swallows these, so
// the console is the only place a test can observe them. "Plugin Error" and
// "Network Error" are left out: tests provoke both on purpose.
const APP_ERROR_RE =
/^(Internal Error|Unexpected Error|Assertion Error|Uncaught Exception|Uncaught Rejection):/;
const MOCKED = !!process.env['MOCK_BACKEND'];
const MOCK_BASE_URL = 'http://localhost:3000';
const apiUrl = MOCKED
@ -393,13 +400,36 @@ async function main() {
let fatal: string | null = null;
console.log('\nRunning tests:');
// Errors the app reported since the previous test result.
let appErrors: string[] = [];
const takeAppErrors = (): string => {
const detail = appErrors.join('; ');
appErrors = [];
return detail;
};
const done = new Promise<void>((resolvePromise) => {
page.on('pageerror', (err) => {
appErrors.push(`Uncaught ${err.message}`);
});
page.on('console', (msg) => {
const text = msg.text();
if (APP_ERROR_RE.test(text)) {
appErrors.push(text.split('\n')[0]!.trim());
}
if (text.startsWith('__TEST_RESULT__ ')) {
const result: TestResult = JSON.parse(
text.slice('__TEST_RESULT__ '.length),
);
// Errors buffered so far belong to the test this result closes.
const reported = takeAppErrors();
if (reported) {
result.error =
result.status === 'fail' && result.error
? `${result.error} — Penpot also reported: ${reported}`
: `Penpot reported an error during the test: ${reported}`;
result.status = 'fail';
}
results.push(result);
// Print each result as it streams in so the run shows live progress
// instead of staying silent until it finishes.

View File

@ -413,6 +413,28 @@ describe('Interactions', () => {
expect(r.interactions.map((i) => i.delay)).toEqual([null, 500]);
});
// A proxy whose own interaction is gone resolves to no position, so the write
// has nothing to address and the API rejects it.
test('a write through a removed interaction is rejected', async (ctx) => {
const r = rect(ctx);
r.addInteraction('click', {
type: 'open-url',
url: 'https://example.com',
});
await ctx.penpot.waitForLayoutUpdate();
const [only] = r.interactions;
only.remove();
await ctx.penpot.waitForLayoutUpdate();
expect(r.interactions).toHaveLength(0);
expect(() => {
only.delay = 500;
}).toThrow('The interaction is not part of the shape anymore');
await ctx.penpot.waitForLayoutUpdate();
expect(r.interactions).toHaveLength(0);
});
test('interaction trigger can be changed', (ctx) => {
const dest = board(ctx);
const r = rect(ctx);