mirror of
https://github.com/penpot/penpot.git
synced 2026-10-30 22:40:33 +00:00
🐛 Ask MCP plugins for heartbeats so hidden tabs do not look frozen
The server refuses to send a task to a plugin whose last heartbeat is older than 30 s, taking its tab for frozen. The plugin sent heartbeats only from a 10 s timer, and Chrome runs the timers of a hidden page as rarely as once a minute: without Energy Saver after about a minute hidden, and at once with it. A Penpot tab hidden for a minute therefore failed that check about half the time, although it could run tasks. Chrome handles socket messages of a hidden page at once, and a frozen page cannot handle them at all. The server now sends each plugin a heartbeat request every 10 s, and the plugin answers it with a heartbeat, so the heartbeat age again tells a frozen tab from a hidden one. The plugin keeps its timer, so it still sends heartbeats to a server that does not ask. A plugin that does not know the request passes it to the plugin code, which ignores it.
This commit is contained in:
parent
05f598246e
commit
19e5f072b5
@ -147,6 +147,17 @@ test("resume and freeze events cannot send liveness messages before initializati
|
||||
assert.deepEqual(ui.sockets[0].sent, [{ type: "heartbeat" }]);
|
||||
});
|
||||
|
||||
test("answers heartbeat requests once initialized, without forwarding them to the plugin", () => {
|
||||
const ui = pluginUi();
|
||||
ui.connect();
|
||||
ui.sockets[0].receive({ type: "heartbeat-request" });
|
||||
assert.deepEqual(ui.sockets[0].sent, []);
|
||||
ui.sockets[0].receive({ type: "initialized" });
|
||||
ui.sockets[0].receive({ type: "heartbeat-request" });
|
||||
assert.deepEqual(ui.sockets[0].sent, [{ type: "heartbeat" }]);
|
||||
assert.equal(ui.messages.filter((message) => message.type === "heartbeat-request").length, 0);
|
||||
});
|
||||
|
||||
test("plugin reads the current file for each connection metadata request", () => {
|
||||
let onMessage!: (message: any) => void;
|
||||
const messages: any[] = [];
|
||||
|
||||
@ -145,7 +145,9 @@ function sendTaskResponse(response: any): void {
|
||||
* Emits a liveness signal from the plugin event loop.
|
||||
*
|
||||
* WebSocket ping/pong is not enough here: browsers can answer protocol pings while
|
||||
* page JavaScript is frozen and unable to run MCP tasks.
|
||||
* page JavaScript is frozen and unable to run MCP tasks. The plugin sends one on a timer and
|
||||
* in answer to each heartbeat request of the server, since a browser runs the timers of a
|
||||
* hidden page as rarely as once a minute but handles socket messages at once.
|
||||
*/
|
||||
function sendHeartbeat(): boolean {
|
||||
if (connectionReady && ws?.readyState === WebSocket.OPEN) {
|
||||
@ -252,6 +254,10 @@ function connectToMcpServer(baseUrl?: string, token?: string): void {
|
||||
parent.postMessage({ type: "context-request" }, "*");
|
||||
return;
|
||||
}
|
||||
if (request.type === "heartbeat-request") {
|
||||
sendHeartbeat();
|
||||
return;
|
||||
}
|
||||
// Track the current task received from the MCP server
|
||||
if (request.task) {
|
||||
updateCurrentTask(request.task);
|
||||
|
||||
@ -17,6 +17,8 @@ export interface PenpotConnection extends PluginLivenessState {
|
||||
readonly userToken: string | null;
|
||||
readonly session: PenpotSession;
|
||||
readonly pingInterval: NodeJS.Timeout;
|
||||
/** the interval sending heartbeat requests to the plugin. */
|
||||
readonly heartbeatRequestInterval: NodeJS.Timeout;
|
||||
/** whether initialization and dispatch subscriptions have completed. */
|
||||
ready: boolean;
|
||||
}
|
||||
|
||||
@ -1,8 +1,8 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { once } from "node:events";
|
||||
import { afterEach, beforeEach, test } from "node:test";
|
||||
import { afterEach, beforeEach, mock, test } from "node:test";
|
||||
import { WebSocket } from "ws";
|
||||
import { PluginBridge } from "./PluginBridge";
|
||||
import { HEARTBEAT_REQUEST_INTERVAL_MS, PluginBridge } from "./PluginBridge";
|
||||
import { PluginTask } from "./PluginTask";
|
||||
import type { PenpotMcpServer } from "./PenpotMcpServer";
|
||||
|
||||
@ -37,17 +37,33 @@ async function connect(sessionId: string, token = "alice", fileName = "Design",
|
||||
const socket = new WebSocket(`ws://127.0.0.1:${port}?userToken=${token}`);
|
||||
sockets.push(socket);
|
||||
await once(socket, "open");
|
||||
// answer tasks from the start, since one may arrive right behind the initialization
|
||||
socket.on("message", (raw) => {
|
||||
const request = JSON.parse(raw.toString());
|
||||
if (request.type !== "initialized" && request.type !== "heartbeat-request") {
|
||||
socket.send(JSON.stringify({ id: request.id, success: true, data: reply }));
|
||||
}
|
||||
});
|
||||
const initialized = once(socket, "message", { signal: AbortSignal.timeout(1500) });
|
||||
socket.send(JSON.stringify({ type: "initialize", session: { sessionId, fileId: "file-1", fileName } }));
|
||||
const [message] = await initialized;
|
||||
assert.equal(JSON.parse(message.toString()).type, "initialized");
|
||||
socket.on("message", (raw) => {
|
||||
const request = JSON.parse(raw.toString());
|
||||
socket.send(JSON.stringify({ id: request.id, success: true, data: reply }));
|
||||
});
|
||||
return socket;
|
||||
}
|
||||
|
||||
test("asks each plugin for a heartbeat at a fixed interval", async () => {
|
||||
mock.timers.enable({ apis: ["setInterval"] });
|
||||
try {
|
||||
const socket = await connect("tab-1");
|
||||
const requested = once(socket, "message", { signal: AbortSignal.timeout(1500) });
|
||||
mock.timers.tick(HEARTBEAT_REQUEST_INTERVAL_MS);
|
||||
const [message] = await requested;
|
||||
assert.deepEqual(JSON.parse(message.toString()), { type: "heartbeat-request" });
|
||||
} finally {
|
||||
mock.timers.reset();
|
||||
}
|
||||
});
|
||||
|
||||
test("keeps two tabs for the same file and dispatches to the selected session", async () => {
|
||||
await connect("tab-1");
|
||||
await connect("tab-2");
|
||||
|
||||
@ -15,6 +15,14 @@ import { MultiInstanceTaskDispatcher } from "./dispatch/MultiInstanceTaskDispatc
|
||||
const KEEP_ALIVE_TIME = 30000; // 30 seconds
|
||||
const INITIALIZATION_TIMEOUT_MS = 10_000;
|
||||
|
||||
/**
|
||||
* The interval, in ms, between the heartbeat requests sent to each plugin.
|
||||
*
|
||||
* A browser runs the timers of a hidden page as rarely as once a minute, which would leave the
|
||||
* plugin's own heartbeat older than the stale threshold, but it handles socket messages at once.
|
||||
*/
|
||||
export const HEARTBEAT_REQUEST_INTERVAL_MS = 10_000;
|
||||
|
||||
/**
|
||||
* Maximum plugin heartbeat age before a connection is stale.
|
||||
*
|
||||
@ -218,12 +226,17 @@ export class PluginBridge implements TaskDispatchHost {
|
||||
const pingInterval = setInterval(() => {
|
||||
ws.ping();
|
||||
}, KEEP_ALIVE_TIME);
|
||||
// the plugin answers with a heartbeat, which a frozen page cannot send
|
||||
const heartbeatRequestInterval = setInterval(() => {
|
||||
ws.send(JSON.stringify({ type: "heartbeat-request" }));
|
||||
}, HEARTBEAT_REQUEST_INTERVAL_MS);
|
||||
|
||||
// register the client connection with both indexes
|
||||
const connection: PenpotConnection = {
|
||||
socket: ws,
|
||||
userToken,
|
||||
pingInterval,
|
||||
heartbeatRequestInterval,
|
||||
session,
|
||||
ready: false,
|
||||
lastHeartbeat: Date.now(),
|
||||
@ -260,11 +273,11 @@ export class PluginBridge implements TaskDispatchHost {
|
||||
/**
|
||||
* Removes a client connection and releases all resources associated with it.
|
||||
*
|
||||
* Clears the per-connection keep-alive interval and removes the connection from the
|
||||
* socket-keyed index and the user's session collection. The token-keyed index entry
|
||||
* is removed only when the user's last local connection closes. Tasks sent over the
|
||||
* connection and still awaiting a response fail at once. Safe to call with a socket
|
||||
* that is not (or no longer) registered.
|
||||
* Clears the per-connection keep-alive and heartbeat-request intervals and removes the
|
||||
* connection from the socket-keyed index and the user's session collection. The
|
||||
* token-keyed index entry is removed only when the user's last local connection closes.
|
||||
* Tasks sent over the connection and still awaiting a response fail at once. Safe to call
|
||||
* with a socket that is not (or no longer) registered.
|
||||
*
|
||||
* @param ws - The WebSocket whose connection state should be removed
|
||||
*/
|
||||
@ -275,6 +288,7 @@ export class PluginBridge implements TaskDispatchHost {
|
||||
}
|
||||
connection.ready = false;
|
||||
clearInterval(connection.pingInterval);
|
||||
clearInterval(connection.heartbeatRequestInterval);
|
||||
this.connectionsBySocket.delete(ws);
|
||||
const connections = this.getUserConnections(connection.userToken)!;
|
||||
connections.remove(connection);
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user