🐛 Keep reporting rollbacks the MCP agent has not heard about

The MCP plugin sends the rollbacks it saw with its next task response
and then forgets them. When that response came after the server had
given up on the task, the server dropped it and the agent never heard
about the rollback. A reload of the tab or of the plugin lost them too.

The server now sends the rollbacks of such a late response back to the
plugin, which reports them again with its next response. The plugin
keeps the rollbacks not reported yet in its local storage, one entry
per file, so a reload does not lose them. Both MCP plugin manifests ask
for the allow:localstorage permission for that.
This commit is contained in:
alonso.torres 2026-11-03 11:22:30 +00:00
parent 642f35cf90
commit d09bc9c3ee
10 changed files with 382 additions and 16 deletions

View File

@ -136,3 +136,7 @@ For parallel devenvs, prefer same-origin MCP routing: each Penpot instance shoul
- The MCP server emits `1008` for a missing `userToken` in multi-user mode, for missing/invalid connection metadata, and to close a connection displaced by a newer one with the same user and session ID.
- Session IDs are deterministic per tab and file, so a reconnect reuses its ID while the old socket may be half-open: the newest connection wins (`PluginBridge.displaceConnection`). Across instances, each new connection publishes a session claim on a per-user Redis channel; an instance displaces its local connection only on a foreign claim that arrives after its own claim (channel order, no clocks). Discovery deduplicates by session ID.
- 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 the MCP menu.
## Rollback warnings
- The plugin's `RollbackLog` (`packages/plugin/src/RollbackLog.ts`) holds the `contentrollback` events not yet sent with a task response; `Task.sendResponse` attaches them and acknowledges them once sent, and the server's `unclaimed-rollbacks` message (a response that came after its task timed out) puts them back. With a `RollbackStore` (`LocalStorageRollbackStore` over `penpot.localStorage`, one entry per file) every change is written through, so a reload of the tab or plugin keeps them; storage failures fall back to memory. Plugin tests run with `node --experimental-strip-types`, which rejects TypeScript parameter properties: declare fields explicitly.

View File

@ -25,7 +25,9 @@
(def mcp-permissions
#{"user:read" "library:read" "library:write"
"comment:read" "comment:write" "content:read" "content:write"
"allow:global" "manage:projects" "manage:teams" "manage:delete"})
"allow:global" "manage:projects" "manage:teams" "manage:delete"
;; Keeps the rollbacks not reported yet across reloads
"allow:localstorage"})
;; Promise that resolves when plugins runtime is initialized.
;; Lives here to avoid circular dependency: workspace.mcp -> app.plugins -> app.plugins.api -> workspace

View File

@ -29,6 +29,19 @@ export interface PluginTaskResult<T> {
rollbacks?: ContentRollbackEvent[];
}
/**
* Message from the MCP server giving the plugin back the rollbacks of a task response that came
* after its task timed out, so that the plugin reports them with its next response.
*/
export interface UnclaimedRollbacksMessage {
type: "unclaimed-rollbacks";
/**
* The rollbacks the late response carried, oldest first.
*/
rollbacks: ContentRollbackEvent[];
}
/**
* Rollback of rejected changes in the Penpot workspace.
*

View File

@ -5,5 +5,5 @@
"version": 2,
"scope": "global",
"description": "This plugin enables interaction with the Penpot MCP server",
"permissions": ["user:read", "content:read", "content:write", "library:read", "library:write", "comment:read", "comment:write"]
"permissions": ["user:read", "content:read", "content:write", "library:read", "library:write", "comment:read", "comment:write", "allow:localstorage"]
}

View File

@ -32,6 +32,7 @@ function pluginRequire(name: string) {
if (name === "./VersionCompatibility") return VersionCompatibility;
if (name === "./RollbackLog")
return {
LocalStorageRollbackStore: class {},
RollbackLog: class {
listen() {}
},

View File

@ -1,6 +1,6 @@
import assert from "node:assert/strict";
import test from "node:test";
import { RollbackLog } from "./RollbackLog.ts";
import { LocalStorageRollbackStore, RollbackLog, type RollbackStore } from "./RollbackLog.ts";
const rollback = (id: string) => ({ fileIds: [`file-${id}`], pageIds: [`page-${id}`], shapeIds: [`shape-${id}`] });
@ -45,3 +45,125 @@ test("keeps the rollbacks recorded after the acknowledged ones", () => {
assert.deepEqual(log.pending(), [rollback("b")]);
});
test("puts returned rollbacks back ahead of the ones recorded since", () => {
const log = new RollbackLog();
log.record(rollback("a"));
const sent = log.pending();
log.acknowledge(sent.length);
log.record(rollback("b"));
log.restore(sent);
assert.deepEqual(log.pending(), [rollback("a"), rollback("b")]);
});
/**
* Rollback store that keeps its events in memory, standing in for the storage that outlives a reload.
*/
class MemoryRollbackStore implements RollbackStore {
events: unknown = [];
load() {
return this.events as any;
}
save(events: any) {
this.events = JSON.parse(JSON.stringify(events));
}
}
/**
* Plugin local storage held in a map.
*/
function memoryStorage(): {
items: Map<string, string>;
getItem(key: string): string;
setItem(key: string, value: unknown): void;
removeItem(key: string): void;
} {
const items = new Map<string, string>();
return {
items,
getItem: (key) => items.get(key) ?? (null as any),
setItem: (key, value) => void items.set(key, String(value)),
removeItem: (key) => void items.delete(key),
};
}
test("keeps unsent rollbacks across a reload", () => {
const store = new MemoryRollbackStore();
new RollbackLog(store).record(rollback("a"));
const reloaded = new RollbackLog(store);
assert.deepEqual(reloaded.pending(), [rollback("a")]);
});
test("forgets the stored rollbacks once a response carried them", () => {
const store = new MemoryRollbackStore();
const log = new RollbackLog(store);
log.record(rollback("a"));
log.record(rollback("b"));
log.acknowledge(1);
assert.deepEqual(new RollbackLog(store).pending(), [rollback("b")]);
log.acknowledge(1);
assert.deepEqual(new RollbackLog(store).pending(), []);
});
test("stores the rollbacks the server returns", () => {
const store = new MemoryRollbackStore();
new RollbackLog(store).restore([rollback("a")]);
assert.deepEqual(new RollbackLog(store).pending(), [rollback("a")]);
});
test("keeps rollbacks in memory when the storage fails", () => {
const failing: RollbackStore = {
load: () => {
throw new Error("storage unavailable");
},
save: () => {
throw new Error("storage unavailable");
},
};
const log = new RollbackLog(failing);
assert.doesNotThrow(() => log.record(rollback("a")));
assert.deepEqual(log.pending(), [rollback("a")]);
});
test("stores rollbacks per file in the plugin local storage", () => {
const storage = memoryStorage();
const fileA = new LocalStorageRollbackStore(storage, "file-a");
const fileB = new LocalStorageRollbackStore(storage, "file-b");
fileA.save([rollback("a")]);
assert.deepEqual(new LocalStorageRollbackStore(storage, "file-a").load(), [rollback("a")]);
assert.deepEqual(fileB.load(), []);
});
test("removes the entry of a file with no rollbacks left", () => {
const storage = memoryStorage();
const store = new LocalStorageRollbackStore(storage, "file-a");
store.save([rollback("a")]);
store.save([]);
assert.equal(storage.items.size, 0);
});
test("ignores a stored entry that is not a rollback list", () => {
const storage = memoryStorage();
const store = new LocalStorageRollbackStore(storage, "file-a");
for (const raw of ["not json", '{"fileIds": []}', '[{"fileIds": "x"}]']) {
storage.items.clear();
store.save([rollback("a")]);
storage.items.set([...storage.items.keys()][0], raw);
assert.deepEqual(store.load(), [], raw);
}
});

View File

@ -1,13 +1,107 @@
import type { LocalStorage } from "@penpot/plugin-types";
import type { ContentRollbackEvent } from "../../common/src";
/**
* Storage of the rollbacks a {@link RollbackLog} has not handed over yet, which outlives a reload
* of the plugin or of the browser tab.
*/
export interface RollbackStore {
/**
* Returns the stored rollbacks, oldest first.
*/
load(): ContentRollbackEvent[];
/**
* Replaces the stored rollbacks.
*
* @param events - the rollbacks not handed over yet, oldest first
*/
save(events: ContentRollbackEvent[]): void;
}
/**
* Rollback store in Penpot's plugin local storage, with one entry per file.
*
* The plugin runs for one open file, and a reload opens the same file again, so the entry of that
* file holds what the reload would otherwise lose. The storage is the browser's, so it is kept
* across reloads but not shared with other browsers.
*/
export class LocalStorageRollbackStore implements RollbackStore {
private static readonly KEY_PREFIX = "rollbacks/";
private readonly storage: Pick<LocalStorage, "getItem" | "setItem" | "removeItem">;
private readonly fileId: string;
/**
* @param storage - the plugin local storage (`penpot.localStorage`)
* @param fileId - the id of the file the plugin runs for
*/
constructor(storage: Pick<LocalStorage, "getItem" | "setItem" | "removeItem">, fileId: string) {
this.storage = storage;
this.fileId = fileId;
}
private get key(): string {
return LocalStorageRollbackStore.KEY_PREFIX + this.fileId;
}
load(): ContentRollbackEvent[] {
const raw = this.storage.getItem(this.key);
if (!raw) {
return [];
}
try {
const events = JSON.parse(raw);
return Array.isArray(events) && events.every(isRollbackEvent) ? events : [];
} catch {
return [];
}
}
save(events: ContentRollbackEvent[]): void {
if (events.length === 0) {
this.storage.removeItem(this.key);
} else {
this.storage.setItem(this.key, JSON.stringify(events));
}
}
}
/**
* Tells whether the given value has the shape of a rollback event.
*
* @param value - a value read from storage
*/
function isRollbackEvent(value: any): value is ContentRollbackEvent {
const isIdList = (ids: unknown) => Array.isArray(ids) && ids.every((id) => typeof id === "string");
return (
typeof value === "object" &&
value !== null &&
isIdList(value.fileIds) &&
isIdList(value.pageIds) &&
isIdList(value.shapeIds)
);
}
/**
* Log of the rollbacks of rejected changes that the MCP server has not been told about yet.
*
* A rollback happens after the task that made the rejected changes has returned (the backend
* rejects a save seconds later), so the log hands its events to the next task response.
* rejects a save seconds later), so the log hands its events to the next task response. With a
* store, the log also keeps its events across a reload, until a response carries them.
*/
export class RollbackLog {
private events: ContentRollbackEvent[] = [];
private readonly store?: RollbackStore;
private events: ContentRollbackEvent[];
/**
* @param store - the storage that keeps the events across reloads; without one, or when it
* fails, the events live in memory only
*/
constructor(store?: RollbackStore) {
this.store = store;
this.events = this.load();
}
/**
* Starts recording the rollbacks Penpot reports.
@ -30,11 +124,19 @@ export class RollbackLog {
* @param event - the rollback as reported by Penpot
*/
record(event: ContentRollbackEvent): void {
this.events.push({
fileIds: [...event.fileIds],
pageIds: [...event.pageIds],
shapeIds: [...event.shapeIds],
});
this.events.push(copyEvent(event));
this.save();
}
/**
* Puts back rollbacks that a sent response carried but no tool execution received (the
* server had given up on that task), ahead of the ones recorded since.
*
* @param events - the returned rollbacks, oldest first
*/
restore(events: ContentRollbackEvent[]): void {
this.events = [...events.map(copyEvent), ...this.events];
this.save();
}
/**
@ -50,6 +152,40 @@ export class RollbackLog {
* @param count - the number of rollbacks the sent response carried
*/
acknowledge(count: number): void {
if (count === 0) {
return;
}
this.events = this.events.slice(count);
this.save();
}
private load(): ContentRollbackEvent[] {
try {
return this.store?.load() ?? [];
} catch (error) {
console.warn("Unable to read the stored rollbacks:", error);
return [];
}
}
private save(): void {
try {
this.store?.save(this.events);
} catch (error) {
console.warn("Unable to store the rollbacks:", error);
}
}
}
/**
* Copies a rollback event into a plain object.
*
* @param event - the rollback, possibly a proxy or array-like from Penpot
*/
function copyEvent(event: ContentRollbackEvent): ContentRollbackEvent {
return {
fileIds: [...event.fileIds],
pageIds: [...event.pageIds],
shapeIds: [...event.shapeIds],
};
}

View File

@ -2,9 +2,9 @@ import { ExecuteCodeTaskHandler } from "./task-handlers/ExecuteCodeTaskHandler";
import { Task, TaskHandler } from "./TaskHandler";
import { formatTaskError } from "./ErrorUtils";
import { extractMinorVersion, isCompatibleVersion } from "./VersionCompatibility";
import type { PluginConnectionInit } from "../../common/src";
import type { ContentRollbackEvent, PluginConnectionInit } from "../../common/src";
import type { WorkspaceContext } from "@penpot/plugin-types";
import { RollbackLog } from "./RollbackLog";
import { LocalStorageRollbackStore, RollbackLog, type RollbackStore } from "./RollbackLog";
/**
* indicates whether the plugin is running in an environment with the Penpot-integrated remote MCP server
@ -27,10 +27,27 @@ const taskHandlers: TaskHandler[] = [new ExecuteCodeTaskHandler()];
/**
* Rollbacks of rejected changes, reported to the MCP server with the next task response.
* The ones not reported yet are kept in the plugin local storage of the open file, so a reload
* of the tab or of the plugin does not lose them.
*/
const rollbackLog = new RollbackLog();
const rollbackLog = new RollbackLog(createRollbackStore());
rollbackLog.listen();
/**
* Creates the store of the rollbacks not reported yet, for the open file.
*
* @returns the store, or undefined when no file is open or the local storage is unavailable
*/
function createRollbackStore(): RollbackStore | undefined {
try {
const fileId = penpot.currentFile?.id;
return fileId ? new LocalStorageRollbackStore(penpot.localStorage, fileId) : undefined;
} catch (error) {
console.warn("Rollbacks are kept in memory only:", error);
return undefined;
}
}
// Open the plugin UI (main.ts)
penpot.ui.open("Penpot MCP Plugin", `?theme=${penpot.theme}`, {
width: 236,
@ -40,7 +57,16 @@ penpot.ui.open("Penpot MCP Plugin", `?theme=${penpot.theme}`, {
// Register message handlers
penpot.ui.onMessage<
string | { id: string; type?: string; status?: string; sessionId?: string; task: string; params: any }
| string
| {
id: string;
type?: string;
status?: string;
sessionId?: string;
task: string;
params: any;
rollbacks?: ContentRollbackEvent[];
}
>((message) => {
if (typeof message === "object" && message.type === "ui-initialized") {
uiInitialized = true;
@ -85,6 +111,9 @@ penpot.ui.onMessage<
}
} else if (typeof message === "object" && message.type === "context-request") {
sendWorkspaceContext();
} else if (typeof message === "object" && message.type === "unclaimed-rollbacks") {
// the server got these with a response that came too late: report them again
rollbackLog.restore(message.rollbacks ?? []);
} else if (typeof message === "object" && message.type === "update-connection-status") {
mcp?.setMcpStatus(message.status || "unknown", message.sessionId);
} else if (typeof message === "object" && message.task && message.id) {

View File

@ -230,3 +230,32 @@ test("puts a rollback warning first in a failed tool response too", async (t) =>
assert.equal(failure.type, "text");
assert.match(failure.text, /Tool execution failed: .*Shape not found/);
});
/**
* A plugin connection whose socket keeps the messages sent to it.
*/
function fakeConnection(): { socket: { readyState: number; send: (message: string) => void }; sent: unknown[] } {
const sent: unknown[] = [];
return { socket: { readyState: 1, send: (message: string) => sent.push(JSON.parse(message)) }, sent };
}
test("gives the rollbacks of a response that came after its task timed out back to the plugin", () => {
const bridge = server.pluginBridge as any;
const connection = fakeConnection();
bridge.handlePluginTaskResponse(
{ id: "timed-out", success: true, data: { result: 1, log: "" }, rollbacks: [rollback] },
connection
);
assert.deepEqual(connection.sent, [{ type: "unclaimed-rollbacks", rollbacks: [rollback] }]);
});
test("sends nothing back for a late response without rollbacks", () => {
const bridge = server.pluginBridge as any;
const connection = fakeConnection();
bridge.handlePluginTaskResponse({ id: "timed-out", success: true, data: { result: 1, log: "" } }, connection);
assert.deepEqual(connection.sent, []);
});

View File

@ -3,7 +3,12 @@ import * as http from "http";
import { z } from "zod";
import { AbstractPluginTask, PluginTask, PluginTaskError } from "./PluginTask";
import { RollbackNotices } from "./RollbackNotices";
import { PluginTaskResponse, PluginTaskResult } from "@penpot/mcp-common";
import {
ContentRollbackEvent,
PluginTaskResponse,
PluginTaskResult,
UnclaimedRollbacksMessage,
} from "@penpot/mcp-common";
import { createLogger } from "./logger";
import type { PenpotMcpServer } from "./PenpotMcpServer";
import type { RedisBridge } from "./RedisBridge";
@ -317,7 +322,9 @@ export class PluginBridge implements TaskDispatchHost {
* Handles responses from the plugin for completed tasks.
*
* Finds the pending task by ID and resolves or rejects its promise
* based on the execution result.
* based on the execution result. A response that comes after its task
* timed out has no task to report its rollbacks to, so they go back to
* the plugin, which sends them with its next response.
*
* @param response - The plugin task response containing ID and result
* @param connection - The responding local connection, or undefined for Redis responses
@ -326,6 +333,7 @@ export class PluginBridge implements TaskDispatchHost {
const task = this.pendingTasks.get(response.id);
if (!task) {
this.logger.info(`Received response for unknown task ID: ${response.id}`);
this.returnRollbacks(connection, response.rollbacks);
return;
}
@ -356,6 +364,28 @@ export class PluginBridge implements TaskDispatchHost {
this.logger.info(`Task ${response.id} completed: success=${response.success}`);
}
/**
* Sends rollbacks that no tool execution received back to the plugin.
*
* @param connection - The plugin connection that reported them, if known
* @param rollbacks - The rollbacks of a response whose task is gone
*/
private returnRollbacks(
connection: PenpotConnection | undefined,
rollbacks: ContentRollbackEvent[] | undefined
): void {
if (!connection || !rollbacks || rollbacks.length === 0) {
return;
}
if (connection.socket.readyState !== 1) {
this.logger.warn("Plugin disconnected; %d rollback(s) of a late response are lost", rollbacks.length);
return;
}
const message: UnclaimedRollbacksMessage = { type: "unclaimed-rollbacks", rollbacks };
connection.socket.send(JSON.stringify(message));
this.logger.info("Returned %d rollback(s) of a late response to the plugin", rollbacks.length);
}
/**
* Rejects a still-pending task with the given error, releasing its correlation state.
*