github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-06 01:50:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Let plugins write in Inspect mode and stop them as viewers

Browse Source 
Plugin writes failed whenever the workspace read-only flag was set. The
Inspect tab sets it too, although it only hides the editing tools, so an
MCP agent could not edit while the user was inspecting. A user who
became a viewer had the flag cleared, so plugin writes did not throw and
silently did nothing.

Writes now fail on what makes them unsafe, not on the flag: failing
saves, a user who cannot edit, a version preview or a lost render
context. The error says which one, and the MCP instructions tell the
agent which of them pass by waiting.
This commit is contained in:
alonso.torres 2026-11-03 11:22:30 +00:00
parent d8a232b8c8
commit 3e4cdaa5da
6 changed files with 61 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.serena/memories/frontend/plugin-api-to-cljs-binding.md
View File

@ -24,7 +24,7 @@
- Public Plugin API objects are lightweight handles, not durable snapshots. Most getters locate fresh state from `app.main.store/state` using hidden `$id`, `$file`, `$page`, etc.
- `not-valid` logs by default but throws when the plugin flag `throwValidationErrors` is enabled. The MCP execute-code handler deliberately enables that flag while running code.
- A write to a removed shape always throws `Shape not found`, whatever that flag: setters through the shape proxy's `:on-set u/check-shape-writable` (runs outside the error handling), and methods that must not vanish silently (`resize`, `rotate`) call `u/check-shape-located` first. Methods run inside `:on-error (u/handle-error ...)`, which logs instead of throwing when the flag is off, so it rethrows errors whose `ex-data :type` is `::u/shape-not-found` or `::r/read-only`.
- A write in a read-only workspace (`[:workspace-global :read-only?]`: save-failure lock, user without edit rights, version preview) always throws, whatever that flag. `r/check-permission` with `content:write`/`library:write` calls `r/check-writable` first, which covers most writers; writers without a permission check (most top-level context methods in `api.cljs`: `create*`, `group`, `align*`, …; z-order methods; text range `applyTypography`; ruler guide `remove`) call `r/check-writable` themselves, and shape setters get it through `u/check-shape-writable`. A new writer without a write-permission check must call it too. `r/has-permission?` is the check without it, used by the undo blocks so a block opened before a lock still closes. Comments (`comment:write`) stay writable: they do not go through file saves. An async writer calls it again after each server await, before the next server write or commit (`rx/tap` on the response stream; `dwm/create-svg-shape-with-images` takes it as `:check-writable`), so a lock that lands mid-way rejects the promise with the read-only error. `r/has-permission?` is the check without it, used by the undo blocks so a block opened before a lock still closes. Comments (`comment:write`) stay writable: they do not go through file saves.
- A write while the workspace cannot take it (`r/read-only-reason`: save-failure lock `[:workspace-global :persistence-lock]`, `[:permissions :can-edit]` false, version preview `:preview-id`, lost render context `[:render-state :lost]`) always throws, whatever that flag. The `[:workspace-global :read-only?]` flag alone does not block: the Inspect tab sets it and plugins may still write. `r/check-permission` with `content:write`/`library:write` calls `r/check-writable` first, which covers most writers; writers without a permission check (most top-level context methods in `api.cljs`: `create*`, `group`, `align*`, …; z-order methods; text range `applyTypography`; ruler guide `remove`) call `r/check-writable` themselves, and shape setters get it through `u/check-shape-writable`. A new writer without a write-permission check must call it too. An async writer calls it again after each server await, before the next server write or commit (`rx/tap` on the response stream; `dwm/create-svg-shape-with-images` takes it as `:check-writable`), so a lock that lands mid-way rejects the promise with the read-only error. `r/has-permission?` is the check without it, used by the undo blocks so a block opened before a lock still closes. Comments (`comment:write`) stay writable: they do not go through file saves.
- `penpotMgmt` (`app.plugins.management`) checks permissions with `r/has-permission?` (`granted?`), never `r/check-permission`: its calls act on files through the backend, often from the dashboard with no file open, so the workspace read-only check (`check-writable`) must not block them.
- `naturalChildOrdering` and `throwValidationErrors` are stored per plugin under `[:plugins :flags plugin-id ...]`; changing default behavior affects automation and MCP diagnostics.
- Plugin data is stored under keyword namespaces: private data uses `(keyword "plugin" plugin-id)`, shared data uses `(keyword "shared" namespace)`.

35
frontend/src/app/plugins/register.cljs
View File

@ -279,17 +279,34 @@
:else (let [{:keys [permissions]} (dm/get-in @registry [:data plugin-id])]
(contains? permissions permission))))
(defn- read-only-reason
"Why plugin writes must fail in `state`, or nil when they can go ahead.
The read-only flag alone is not a reason: the Inspect tab sets it too,
and it only hides the editing tools."
[state]
(let [wglobal (:workspace-global state)]
(cond
(some? (:persistence-lock wglobal))
"because changes cannot be saved right now"
(not (dm/get-in state [:permissions :can-edit]))
"because you cannot edit this file"
(some? (:preview-id wglobal))
"while a version is being previewed"
(dm/get-in state [:render-state :lost])
"because the canvas stopped rendering")))
(defn check-writable
"Throws while the workspace is read-only: saves are failing, the user
cannot edit or a version is being previewed. A write made then would be
lost, so it fails whatever the plugin's validation flag."
"Throws while saves are failing, the user cannot edit, a version is
being previewed or the render context is lost. A write made then would
be lost or edit the wrong data, so it fails whatever the plugin's
validation flag."
[]
(let [wglobal (:workspace-global @st/state)]
(when (true? (:read-only? wglobal))
(throw (ex-info (if (some? (:persistence-lock wglobal))
"[PENPOT PLUGIN] The workspace is read-only because changes cannot be saved right now"
"[PENPOT PLUGIN] The workspace is read-only")
{:type ::read-only})))))
(when-let [reason (read-only-reason @st/state)]
(throw (ex-info (str "[PENPOT PLUGIN] The workspace is read-only " reason)
{:type ::read-only}))))
(def ^:private write-permissions
#{"content:write" "library:write"})

36
frontend/test/frontend_tests/plugins/read_only_test.cljs
View File

@ -25,9 +25,9 @@
;; Its validation flag is off, so only errors that always throw reach it
(def ^:private plugin-id "00000000-0000-0000-0000-000000000000")
(defn- set-read-only
[store read-only?]
(swap! store assoc-in [:workspace-global :read-only?] read-only?))
(defn- set-can-edit
[store can-edit?]
(swap! store assoc-in [:permissions :can-edit] can-edit?))
(defn- shape-count
[store]
@ -46,7 +46,7 @@
(with-context
(fn [store ^js context]
(let [^js board (.createBoard context)]
(set-read-only store true)
(set-can-edit store false)
(t/testing "a shape setter"
(t/is (thrown-with-msg? js/Error #"read-only" (set! (.-name board) "Renamed")))
@ -67,11 +67,31 @@
(with-context
(fn [store ^js context]
(let [^js board (.createBoard context)]
(set-read-only store true)
(set-read-only store false)
(set-can-edit store false)
(set-can-edit store true)
(set! (.-name board) "Renamed")
(t/is (= "Renamed" (.-name board)))))))
(t/deftest the-inspect-tab-does-not-block-writes
(with-context
(fn [store ^js context]
(let [^js board (.createBoard context)]
(swap! store update :workspace-global assoc
:read-only? true
:options-mode :inspect)
(set! (.-name board) "Renamed")
(t/is (= "Renamed" (.-name board)))))))
(t/deftest each-read-only-source-says-why
(doseq [[path value reason] [[[:permissions :can-edit] false #"cannot edit this file"]
[[:workspace-global :preview-id] (random-uuid) #"version is being previewed"]
[[:render-state :lost] true #"canvas stopped rendering"]]]
(with-context
(fn [store ^js context]
(swap! store assoc-in path value)
(t/testing (str path)
(t/is (thrown-with-msg? js/Error reason (.createBoard context))))))))
(t/deftest a-save-failure-lock-says-why-the-workspace-is-read-only
(with-context
(fn [store ^js context]
@ -84,7 +104,7 @@
(with-context
(fn [store ^js context]
(let [^js board (.createBoard context)]
(set-read-only store true)
(set-can-edit store false)
(t/is (string? (.-name board)))
(let [block (.undoBlockBegin (.-history context))]
(t/is (some? block))
@ -93,7 +113,7 @@
(t/deftest read-only-keeps-permission-checks-of-reads
(with-context
(fn [store _]
(set-read-only store true)
(set-can-edit store false)
(t/is (true? (r/check-permission plugin-id "content:read")))
(t/is (true? (r/has-permission? plugin-id "content:write")))
(t/is (thrown? js/Error (r/check-permission plugin-id "library:write"))))))

3
frontend/test/frontend_tests/plugins/shape_bugfixes_test.cljs
View File

@ -102,7 +102,8 @@
emitted (atom nil)
context (api/create-context plugin-id)]
(set! st/state (atom {:current-file-id file-id
:current-page-id page-id}))
:current-page-id page-id
:permissions {:can-edit true}}))
(with-redefs [dw/convert-selected-to-path
(mock/stub (fn [ids]
(reset! emitted ids)

4
mcp/packages/server/data/initial_instructions.md
View File

@ -36,7 +36,9 @@ When you get this warning:
* Redo the missing changes, then check again; if the same changes are rejected again, stop and tell the user.
While saves are failing, the workspace becomes read-only: every write through the API throws an error saying that the
workspace is read-only. Do not retry in a loop; wait and try again later, or tell the user.
workspace is read-only because changes cannot be saved. Do not retry in a loop; wait and try again later, or tell the user.
Writes also throw when the user cannot edit the file, a version is being previewed or the canvas stopped rendering;
the error says which. These do not pass by waiting: tell the user.
# The Structure of Penpot Designs

2
plugins/CHANGELOG.md
View File

@ -18,7 +18,7 @@
- **plugin-types, plugin-runtime:** Dashboard property setters now update values after backend success and log failures without interrupting the workspace. File moves reject projects in other teams, backend rejections include their type and code, and `project.fileCount` stays at its listing snapshot. Duplicated projects include their file count and pinned state.
- **plugin-runtime:** Cancel plugin loads before they execute after unloading or logout, and allow reopening plugins that close during startup.
- **plugins-runtime**: `library.connectLibrary()` now resolves once the library's assets are loaded; it used to resolve with an empty library the first time. `library.connected` and `library.availableLibraries()` ignore files opened earlier in the same tab, which used to show up as connected and be left out of the available libraries.
- **plugins-runtime:** Writes now fail with an error while the workspace is read-only (saves failing, a user who cannot edit, a version preview), whatever the `throwValidationErrors` flag. They used to change the workspace with edits that would never be saved. Undo blocks still open and close.
- **plugins-runtime:** Writes now fail with an error while the workspace cannot take them (saves failing, a user who cannot edit, a version preview, a canvas that stopped rendering), whatever the `throwValidationErrors` flag. They used to change the workspace with edits that would never be saved. The error says why. The Inspect tab does not block writes, and undo blocks still open and close.
- **plugins-runtime:** Writes that wait for the server (`createShapeFromSvgWithImages`, `uploadMediaData`, `uploadMediaUrl`, `connectLibrary`) check again when the server answers. If the workspace turned read-only meanwhile (a save failed), the promise rejects with the read-only error: no shape is added and no library is linked. An image the server already stored stays in the file's media, unused.
- **plugins-runtime:** `LibraryComponent.remove()` now rejects linked-library and missing component handles, and does nothing for an already deleted component.
- **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.