🐛 Test and document MCP plugin terminal-close handling (#11906)

Extract the WebSocket close-code decision into a pure ReconnectPolicy
module and cover it with node:test. The plugin now names the 1008
policy-violation code instead of using a magic number, and the terminal
behavior of a 1008 close is pinned by a regression test so a future
refactor cannot reintroduce the reconnect storm.

Document the contract in mem:mcp/core: 1008 is terminal, it comes from a
duplicate user token or a missing token in multi-user mode, and recovery
is manual via "Connect here".

Closes #11510

AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
Andrey Antukh 2026-09-24 15:57:23 +02:00 committed by GitHub
parent 9b3594748c
commit dad6026132
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 50 additions and 1 deletions

View File

@ -94,3 +94,9 @@ In the normal Penpot devenv MCP path, the browser plugin does not discover or ro
The live plugin connection registry is in-memory inside each MCP server process (`PluginBridge.connectedClients` / `clientsByToken`). The database only stores MCP access tokens and profile props such as `mcp-enabled`; it does not manage which plugin is connected to which MCP server. The live plugin connection registry is in-memory inside each MCP server process (`PluginBridge.connectedClients` / `clientsByToken`). The database only stores MCP access tokens and profile props such as `mcp-enabled`; it does not manage which plugin is connected to which MCP server.
For parallel devenvs, prefer same-origin MCP routing: each Penpot instance should expose `/mcp/ws` through its own nginx/Caddy path to the MCP server running inside the same main container. Keep container-internal ports fixed (MCP defaults `4401/4402/4403`, backend/exporter/frontend defaults, etc.) and only offset host-side published ports per instance. If internal ports are offset, hardcoded local proxy config such as `docker/devenv/files/nginx.conf` will misroute unless templated too. For parallel devenvs, prefer same-origin MCP routing: each Penpot instance should expose `/mcp/ws` through its own nginx/Caddy path to the MCP server running inside the same main container. Keep container-internal ports fixed (MCP defaults `4401/4402/4403`, backend/exporter/frontend defaults, etc.) and only offset host-side published ports per instance. If internal ports are offset, hardcoded local proxy config such as `docker/devenv/files/nginx.conf` will misroute unless templated too.
## Plugin reconnect policy
- The plugin treats WebSocket close code `1008` (policy violation) as terminal: it stops auto-reconnecting and stays disconnected until the user explicitly reconnects. Other close codes keep the capped-backoff retry. The decision lives in `ReconnectPolicy.ts` (`shouldReconnectAfterClose`), kept as a pure module so it is unit-testable without DOM/CSS.
- The MCP server emits `1008` for a duplicate connection on the same user token (`PluginBridge`) and for a missing `userToken` in multi-user mode.
- A tab rejected with `1008` never reaches `connected`, so the frontend's 60s reconnect watcher (`start-reconnect-watcher` in `app.main.data.workspace.mcp`, started only on `connected`) does not engage; recovery is manual via "Connect here".

View File

@ -0,0 +1,24 @@
import assert from "node:assert/strict";
import test from "node:test";
import { WS_CLOSE_POLICY_VIOLATION, shouldReconnectAfterClose } from "./ReconnectPolicy.ts";
test("names the policy violation close code used by the MCP server", () => {
assert.equal(WS_CLOSE_POLICY_VIOLATION, 1008);
});
test("does not reconnect after a policy violation close", () => {
assert.equal(shouldReconnectAfterClose(WS_CLOSE_POLICY_VIOLATION), false);
});
test("reconnects after transient close codes", () => {
const transientCodes = [
{ code: 1000, meaning: "normal closure" },
{ code: 1001, meaning: "going away" },
{ code: 1005, meaning: "no status received" },
{ code: 1006, meaning: "abnormal closure" },
];
for (const { code, meaning } of transientCodes) {
assert.equal(shouldReconnectAfterClose(code), true, `expected reconnect for ${code} (${meaning})`);
}
});

View File

@ -0,0 +1,18 @@
/**
* WebSocket close code the MCP server uses for policy violations.
*
* The server rejects a second plugin connection for the same user token, and
* connections missing a token in multi-user mode, with this code.
*/
export const WS_CLOSE_POLICY_VIOLATION = 1008;
/**
* Returns whether a WebSocket close should trigger an automatic reconnect.
*
* Policy violations are terminal: the server refuses the connection again
* immediately, so retrying only repeats the rejection. Other closes (for
* example a dropped or restarted server) are retried with backoff.
*/
export function shouldReconnectAfterClose(code: number): boolean {
return code !== WS_CLOSE_POLICY_VIOLATION;
}

View File

@ -1,4 +1,5 @@
import "./style.css"; import "./style.css";
import { shouldReconnectAfterClose } from "./ReconnectPolicy";
/** /**
* the maximum allowed size for task responses sent back to the MCP server in the integrated remote MCP mode. * the maximum allowed size for task responses sent back to the MCP server in the integrated remote MCP mode.
@ -255,7 +256,7 @@ function connectToMcpServer(baseUrl?: string, token?: string): void {
updateCurrentTask(null); updateCurrentTask(null);
} }
ws = null; ws = null;
if (event.code === 1008) { if (!shouldReconnectAfterClose(event.code)) {
// Policy violation (e.g. duplicate connection for the same user // Policy violation (e.g. duplicate connection for the same user
// token - another tab already holds the connection). Retrying // token - another tab already holds the connection). Retrying
// would be refused again immediately, so stay disconnected // would be refused again immediately, so stay disconnected