🐛 Skip no-op plugin removal and clarify size comments

Return early from remove-profile-plugin when the id is absent:
no wasted write, no size check, and no manufactured :plugins key
that could spuriously fail on oversized profiles.

Clarify that per-field string caps count chars while the byte
budget is enforced by profile-props-max-size.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

backend/resources/climit.edn

@@ -47,8 +47,14 @@
  :send-user-feedback/by-profile
  {:permits 1 :queue 3}
 
- :import-binfile/global
+  :import-binfile/global
  {:permits 4}
 
  :import-binfile/by-profile
+ {:permits 1 :queue 2}
+
+ :profile-plugin-ops/global
+ {:permits 4}
+
+ :profile-plugin-ops/by-profile
  {:permits 1 :queue 2}}

backend/resources/rlimit.edn

@@ -216,8 +216,10 @@
  ;; ═══════════════════════════════════════════════
  ;; Profile operations
  ;; ═══════════════════════════════════════════════
- #{:main/update-profile
-   :main/update-profile-props
+  #{:main/update-profile
+    :main/update-profile-props
+    :main/add-profile-plugin
+    :main/remove-profile-plugin
    :main/update-profile-photo
    :main/update-profile-password
    :main/update-profile-notifications

backend/src/app/config.clj

@@ -99,7 +99,10 @@
 
    ;; Binfile import limits
    :binfile-import-max-object-size (* 1024 1024 100) ;; 100 MiB
-   :binfile-import-max-zip-entries (* 500 1000)})    ;; 500,000
+   :binfile-import-max-zip-entries (* 500 1000)      ;; 500,000
+
+   ;; Profile props total size limit (serialized, after merge)
+   :profile-props-max-size (* 1024 1024 2)})         ;; 2 MiB
 
 (def schema:config
   (do #_sm/optional-keys
@@ -160,6 +163,9 @@
     [:binfile-import-max-object-size {:optional true} ::sm/int]
     [:binfile-import-max-zip-entries {:optional true} ::sm/int]
 
+    ;; Profile props total size limit (PENPOT_PROFILE_PROPS_MAX_SIZE)
+    [:profile-props-max-size {:optional true} ::sm/int]
+
     [:deletion-delay {:optional true} ::ct/duration]
     [:file-clean-delay {:optional true} ::ct/duration]
     [:telemetry-enabled {:optional true} ::sm/boolean]

backend/src/app/rpc/commands/plugins.clj

@@ -11,6 +11,7 @@
    [app.common.types.plugins :as ctp]
    [app.db :as db]
    [app.rpc :as-alias rpc]
+   [app.rpc.climit :as-alias climit]
    [app.rpc.commands.profile :as profile]
    [app.rpc.doc :as-alias doc]
    [app.util.services :as sv]))
@@ -33,6 +34,8 @@
 
 (sv/defmethod ::add-profile-plugin
   {::doc/added "2.18"
+   ::climit/id [[:profile-plugin-ops/by-profile ::rpc/profile-id]
+                [:profile-plugin-ops/global]]
    ::sm/params schema:add-profile-plugin
    ::sm/result ctp/schema:registry-entry
    ::db/transaction true}
@@ -42,14 +45,21 @@
   (let [profile (profile/get-profile conn profile-id ::db/for-update true)
         plugins (get-in profile [:props :plugins] {:ids [] :data {}})
         plugin-id (:plugin-id plugin)
-        plugins (-> plugins
-                    (update :ids #(vec (distinct (conj % plugin-id))))
-                    (assoc-in [:data plugin-id] plugin))]
-    (db/update! conn :profile
-                {:props (db/tjson (assoc (:props profile) :plugins plugins))}
-                {:id profile-id}
-                {::db/return-keys false})
-    plugin))
+        exists? (contains? (set (:ids plugins)) plugin-id)]
+    (when (and (not exists?) (>= (count (:ids plugins)) ctp/max-plugins))
+      (ex/raise :type :validation
+                :code :too-many-plugins
+                :hint "plugin registry exceeds maximum size"))
+    (let [plugins (-> plugins
+                      (update :ids #(vec (distinct (conj % plugin-id))))
+                      (assoc-in [:data plugin-id] plugin))
+          props   (assoc (:props profile) :plugins plugins)]
+      (profile/check-props-size! (:props profile) props)
+      (db/update! conn :profile
+                  {:props (db/tjson props)}
+                  {:id profile-id}
+                  {::db/return-keys false})
+      plugin)))
 
 (def ^:private
   schema:remove-profile-plugin
@@ -58,18 +68,32 @@
 
 (sv/defmethod ::remove-profile-plugin
   {::doc/added "2.18"
+   ::climit/id [[:profile-plugin-ops/by-profile ::rpc/profile-id]
+                [:profile-plugin-ops/global]]
    ::sm/params schema:remove-profile-plugin
    ::sm/result :nil
    ::db/transaction true}
   [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id plugin-id]}]
   (let [profile (profile/get-profile conn profile-id ::db/for-update true)
         plugins (get-in profile [:props :plugins] {:ids [] :data {}})
-        plugin-id-str (str plugin-id)
-        plugins (-> plugins
-                    (update :ids #(vec (remove (partial = plugin-id-str) %)))
-                    (update :data dissoc plugin-id-str))]
-    (db/update! conn :profile
-                {:props (db/tjson (assoc (:props profile) :plugins plugins))}
-                {:id profile-id}
-                {::db/return-keys false})
-    nil))
+        plugin-id-str (str plugin-id)]
+    (if-not (or (some #(= % plugin-id-str) (:ids plugins))
+                (contains? (:data plugins) plugin-id-str))
+      ;; Nothing to remove: skip the write (and the size check) entirely.
+      nil
+      (let [plugins (-> plugins
+                        (update :ids #(vec (remove (partial = plugin-id-str) %)))
+                        (update :data dissoc plugin-id-str))
+            props   (assoc (:props profile) :plugins plugins)]
+        ;; Removal only shrinks props, so this never raises; kept for uniformity
+        ;; so the user-facing profile.props RPC writes (update-profile-props,
+        ;; update-profile-notifications, add/remove-profile-plugin) all go
+        ;; through the size check. System writers (OIDC login merge,
+        ;; management subscription update) are exempt: they write fixed-key,
+        ;; non-accumulating shapes.
+        (profile/check-props-size! (:props profile) props)
+        (db/update! conn :profile
+                    {:props (db/tjson props)}
+                    {:id profile-id}
+                    {::db/return-keys false})
+        nil))))

backend/src/app/rpc/commands/profile.clj

@@ -36,6 +36,7 @@
    [cuerdas.core :as str]))
 
 (declare check-profile-existence!)
+(declare check-props-size!)
 (declare decode-row)
 (declare filter-props)
 (declare get-profile)
@@ -280,6 +281,8 @@
         (-> (get profile :props)
             (assoc :notifications notifications))]
 
+    (check-props-size! (:props profile) props)
+
     (db/update! conn :profile
                 {:props (db/tjson props)}
                 {:id profile-id}
@@ -469,6 +472,28 @@
   [:map {:title "update-profile-props"}
    [:props schema:props-writeable]])
 
+(defn- props-size
+  "Returns the serialized size in UTF-8 bytes of the props map."
+  [props]
+  (if-let [pg (db/tjson props)]
+    (alength (.getBytes ^String (.getValue ^org.postgresql.util.PGobject pg) "UTF-8"))
+    0))
+
+(defn check-props-size!
+  "Raises :props-too-large when the new props exceed the configured total
+  size limit *and* grow beyond the current size. Profiles that already
+  exceed the limit can still shrink or hold steady (grandfathered), but
+  cannot grow further."
+  [old-props new-props]
+  (let [limit    (cf/get :profile-props-max-size)
+        old-size (props-size old-props)
+        new-size (props-size new-props)]
+    (when (and (> new-size limit)
+               (> new-size old-size))
+      (ex/raise :type :validation
+                :code :props-too-large
+                :hint "profile props exceed maximum size"))))
+
 (defn update-profile-props
   [{:keys [::db/conn] :as cfg} profile-id props]
   (let [profile (get-profile conn profile-id ::db/for-update true)
@@ -482,6 +507,8 @@
                            (:props profile)
                            (apply dissoc props system-managed-props))]
 
+    (check-props-size! (:props profile) props)
+
     (db/update! conn :profile
                 {:props (db/tjson props)}
                 {:id profile-id}

backend/test/backend_tests/rpc_plugins_test.clj

@@ -7,6 +7,8 @@
 (ns backend-tests.rpc-plugins-test
   (:require
    [app.common.uuid :as uuid]
+   [app.config :as cf]
+   [app.db :as db]
    [app.rpc :as-alias rpc]
    [app.rpc.commands.profile :as profile]
    [backend-tests.helpers :as th]
@@ -145,6 +147,126 @@
       (t/is (= "Test Plugin" (get-in plugins [:data plugin-id-1 :name])))
       (t/is (= "Second Plugin" (get-in plugins [:data plugin-id-2 :name]))))))
 
+(t/deftest add-profile-plugin-rejects-oversized-code
+  ;; The merged props must not exceed :profile-props-max-size
+  (let [profile (th/create-profile* 1)
+        plugin  (assoc valid-plugin :code (apply str (repeat 200 "x")))
+        data    {::th/type :add-profile-plugin
+                 ::rpc/profile-id (:id profile)
+                 :plugin plugin}]
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [out (th/command! data)]
+        (t/is (th/ex-info? (:error out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :props-too-large))))))
+
+(t/deftest remove-profile-plugin-allowed-on-oversized-profile
+  ;; Removal shrinks props, so it passes even under a tight limit
+  (let [profile (th/create-profile* 1)
+        plugin  (assoc valid-plugin :code (apply str (repeat 200 "x")))]
+    ;; Seed an oversized registry while the limit is high
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100000})]
+      (let [out (th/command! {::th/type :add-profile-plugin
+                              ::rpc/profile-id (:id profile)
+                              :plugin plugin})]
+        (t/is (nil? (:error out)))))
+    ;; Removal under a tight limit still passes
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [out (th/command! {::th/type :remove-profile-plugin
+                              ::rpc/profile-id (:id profile)
+                              :plugin-id (uuid/uuid plugin-id-1)})]
+        (t/is (nil? (:error out)))))))
+
+(t/deftest add-profile-plugin-rejects-51st-plugin
+  ;; The registry holds at most 50 plugins; the 51st (new id) must fail
+  (let [profile (th/create-profile* 1)]
+    ;; Seed 50 plugins
+    (doseq [i (range 50)]
+      (let [plugin (assoc valid-plugin
+                          :plugin-id (str (uuid/next))
+                          :name (str "Plugin " i))
+            out    (th/command! {::th/type :add-profile-plugin
+                                 ::rpc/profile-id (:id profile)
+                                 :plugin plugin})]
+        (t/is (nil? (:error out)) (str "seed plugin " i " should install"))))
+    ;; The 51st must fail with a specific error
+    (let [extra (assoc valid-plugin
+                       :plugin-id (str (uuid/next))
+                       :name "One Too Many")
+          out   (th/command! {::th/type :add-profile-plugin
+                              ::rpc/profile-id (:id profile)
+                              :plugin extra})]
+      (t/is (th/ex-info? (:error out)))
+      (t/is (th/ex-of-type? (:error out) :validation))
+      (t/is (th/ex-of-code? (:error out) :too-many-plugins)))
+    ;; And nothing extra was persisted
+    (let [saved (th/db-get :profile {:id (:id profile)})
+          props (profile/decode-row saved)]
+      (t/is (= 50 (count (get-in props [:props :plugins :ids])))))))
+
+(t/deftest add-profile-plugin-updates-existing-at-limit
+  ;; Re-adding an existing id at the limit is an update, not a new entry
+  (let [profile (th/create-profile* 1)
+        ids     (mapv (fn [_] (str (uuid/next))) (range 50))]
+    (doseq [[i pid] (map-indexed vector ids)]
+      (th/command! {::th/type :add-profile-plugin
+                    ::rpc/profile-id (:id profile)
+                    :plugin (assoc valid-plugin :plugin-id pid :name (str "Plugin " i))}))
+    (let [out (th/command! {::th/type :add-profile-plugin
+                            ::rpc/profile-id (:id profile)
+                            :plugin (assoc valid-plugin :plugin-id (first ids) :name "Renamed")})]
+      (t/is (nil? (:error out)))
+      (let [saved (th/db-get :profile {:id (:id profile)})
+            props (profile/decode-row saved)]
+        (t/is (= 50 (count (get-in props [:props :plugins :ids]))))
+        (t/is (= "Renamed" (get-in props [:props :plugins :data (first ids) :name])))))))
+
+(t/deftest add-profile-plugin-full-registry-reports-too-many-before-size
+  ;; A full registry plus oversized content reports the count guard,
+  ;; which runs before the size check
+  (let [profile (th/create-profile* 1)]
+    ;; Seed 50 plugins under a generous limit
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 1000000})]
+      (doseq [i (range 50)]
+        (let [plugin (assoc valid-plugin
+                            :plugin-id (str (uuid/next))
+                            :name (str "Plugin " i))
+              out    (th/command! {::th/type :add-profile-plugin
+                                   ::rpc/profile-id (:id profile)
+                                   :plugin plugin})]
+          (t/is (nil? (:error out)) (str "seed plugin " i " should install")))))
+    ;; Tight limit + 51st small plugin: count wins over size
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [extra (assoc valid-plugin
+                         :plugin-id (str (uuid/next))
+                         :name "One Too Many")
+            out   (th/command! {::th/type :add-profile-plugin
+                                ::rpc/profile-id (:id profile)
+                                :plugin extra})]
+        (t/is (th/ex-info? (:error out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :too-many-plugins))))
+    ;; And nothing extra was persisted
+    (let [saved (th/db-get :profile {:id (:id profile)})
+          props (profile/decode-row saved)]
+      (t/is (= 50 (count (get-in props [:props :plugins :ids])))))))
+
+(t/deftest remove-profile-plugin-noop-on-oversized-profile-without-plugins
+  ;; Removing an absent id changes nothing: no write, no size failure,
+  ;; and no :plugins key is manufactured
+  (let [profile (th/create-profile* 1)
+        big     {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}]
+    (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)})
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [out (th/command! {::th/type :remove-profile-plugin
+                              ::rpc/profile-id (:id profile)
+                              :plugin-id (uuid/next)})]
+        (t/is (nil? (:error out)))))
+    (let [saved (th/db-get :profile {:id (:id profile)})
+          props (profile/decode-row saved)]
+      (t/is (nil? (get-in props [:props :plugins])))
+      (t/is (= big (get props :props))))))
+
 (t/deftest update-profile-props-rejects-plugins
   (let [profile (th/create-profile* 1)
         data    {::th/type :update-profile-props

backend/test/backend_tests/rpc_profile_test.clj

@@ -1341,6 +1341,104 @@
     (t/is (th/ex-of-code? (:error out) :params-validation))))
 
 
+(t/deftest update-profile-props-rejects-oversized-props
+  ;; The merged props must not exceed :profile-props-max-size
+  (let [profile (th/create-profile* 1)]
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [data {::th/type :update-profile-props
+                  ::rpc/profile-id (:id profile)
+                  :props {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}}
+            out  (th/command! data)]
+        (t/is (th/ex-info? (:error out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :props-too-large))))))
+
+
+(t/deftest update-profile-props-grandfathers-oversized-profile
+  ;; Profiles that already exceed the limit can shrink or hold steady,
+  ;; but cannot grow further
+  (let [profile (th/create-profile* 1)
+        big     {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}]
+    ;; Seed an already-oversized profile directly in DB (bypasses RPC validation)
+    (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)})
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      ;; Shrinking update passes
+      (let [data {::th/type :update-profile-props
+                  ::rpc/profile-id (:id profile)
+                  :props {:onboarding-questions {:big-blob "small"}}}
+            out  (th/command! data)]
+        (t/is (nil? (:error out))))
+      ;; Growing update fails
+      (let [data {::th/type :update-profile-props
+                  ::rpc/profile-id (:id profile)
+                  :props {:onboarding-questions {:big-blob (apply str (repeat 300 "x"))}}}
+            out  (th/command! data)]
+        (t/is (th/ex-info? (:error out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :props-too-large))))))
+
+
+(t/deftest check-props-size-measures-bytes-not-chars
+  ;; The limit is in UTF-8 bytes: multibyte content that fits in chars
+  ;; but exceeds the byte limit must be rejected
+  (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 200})]
+    ;; 70 ASCII chars (~87 bytes serialized) passes
+    (t/is (nil? (profile/check-props-size! {} {:blob (apply str (repeat 70 "x"))})))
+    ;; 70 CJK chars (~227 bytes serialized, still 70 chars) raises
+    (try
+      (profile/check-props-size! {} {:blob (apply str (repeat 70 "日"))})
+      (t/is false "should have thrown")
+      (catch clojure.lang.ExceptionInfo e
+        (t/is (= :validation (:type (ex-data e))))
+        (t/is (= :props-too-large (:code (ex-data e))))))))
+
+(t/deftest update-profile-props-allows-steady-size-on-oversized-profile
+  ;; Same size (not smaller) on an oversized profile passes
+  (let [profile (th/create-profile* 1)
+        big     {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}]
+    (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)})
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [data {::th/type :update-profile-props
+                  ::rpc/profile-id (:id profile)
+                  :props {:onboarding-questions {:big-blob (apply str (repeat 200 "y"))}}}
+            out  (th/command! data)]
+        (t/is (nil? (:error out)))))))
+
+
+(t/deftest update-profile-notifications-rejects-growth-on-oversized-profile
+  ;; The notifications write path goes through the same size check:
+  ;; growing an oversized profile fails
+  (let [profile (th/create-profile* 1)
+        big     {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}}]
+    (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)})
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [data {::th/type :update-profile-notifications
+                  ::rpc/profile-id (:id profile)
+                  :dashboard-comments :all
+                  :email-comments :all
+                  :email-invites :all}
+            out  (th/command! data)]
+        (t/is (th/ex-info? (:error out)))
+        (t/is (th/ex-of-type? (:error out) :validation))
+        (t/is (th/ex-of-code? (:error out) :props-too-large))))))
+
+(t/deftest update-profile-notifications-allows-steady-on-oversized-profile
+  ;; Same-size notifications write on an oversized profile passes
+  (let [profile (th/create-profile* 1)
+        notifications {:dashboard-comments :all
+                       :email-comments :all
+                       :email-invites :all}
+        big     {:onboarding-questions {:big-blob (apply str (repeat 200 "x"))}
+                 :notifications notifications}]
+    (th/db-update! :profile {:props (db/tjson big)} {:id (:id profile)})
+    (with-redefs [cf/get (th/config-get-mock {:profile-props-max-size 100})]
+      (let [data (merge {::th/type :update-profile-notifications
+                         ::rpc/profile-id (:id profile)}
+                        notifications)
+            out  (th/command! data)]
+        (t/is (nil? (:error out)))))))
+
+
 (t/deftest prepare-register-profile-password-too-short
   (let [data {::th/type :prepare-register-profile
               :email "user@example.com"

common/src/app/common/types/plugins.cljc

@@ -41,21 +41,25 @@
   "Schema for plugin permissions - a set of valid permission strings."
   [:set {:gen/max 11} (into [:enum] (sort valid-permissions))])
 
+(def max-plugins
+  "Maximum number of plugins a profile can hold."
+  50)
+
 (def schema:registry-entry
   [:map
    [:plugin-id :string]
    [:version {:optional true} :int]
-   [:name :string]
-   [:description {:optional true} :string]
-   [:host :string]
-   [:code :string]
-   [:icon {:optional true} :string]
+   [:name [:string {:max 500}]]
+   [:description {:optional true} [:string {:max 4096}]]
+   [:host [:string {:max 500}]]
+   [:code [:string {:max 1048576}]] ;; 1 MiB chars; byte budget enforced by profile-props-max-size
+   [:icon {:optional true} [:string {:max 262144}]] ;; 256 KiB chars; see above
    [:permissions schema:permissions]])
 
 (def schema:plugin-registry
   [:map
-   [:ids [:vector :string]]
+   [:ids [:vector {:max max-plugins} :string]]
    [:data
-    [:map-of {:gen/max 5}
+    [:map-of {:gen/max 5 :max max-plugins}
      :string
      schema:registry-entry]]])

common/test/common_tests/runner.cljc

@@ -82,6 +82,7 @@
    [common-tests.types.objects-map-test]
    [common-tests.types.organization-test]
    [common-tests.types.path-data-test]
+   [common-tests.types.plugins-test]
    [common-tests.types.shape-decode-encode-test]
    [common-tests.types.shape-interactions-test]
    [common-tests.types.shape-layout-test]
@@ -162,6 +163,7 @@
    'common-tests.types.objects-map-test
    'common-tests.types.organization-test
    'common-tests.types.path-data-test
+   'common-tests.types.plugins-test
    'common-tests.types.shape-decode-encode-test
    'common-tests.types.shape-interactions-test
    'common-tests.types.shape-layout-test

common/test/common_tests/types/plugins_test.cljc

@@ -0,0 +1,64 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns common-tests.types.plugins-test
+  (:require
+   [app.common.schema :as sm]
+   [app.common.types.plugins :as ctp]
+   [clojure.test :as t]))
+
+(def ^:private valid-entry
+  {:plugin-id "plugin-1"
+   :name "Test Plugin"
+   :description "A test plugin"
+   :host "https://example.com"
+   :code "(function() {})()"
+   :permissions #{"content:read"}})
+
+(t/deftest registry-entry-accepts-valid-plugin
+  (t/is (true? (sm/validate ctp/schema:registry-entry valid-entry))))
+
+(t/deftest registry-entry-rejects-oversized-code
+  (t/is (false? (sm/validate ctp/schema:registry-entry
+                             (assoc valid-entry
+                                    :code (apply str (repeat (inc (* 1024 1024)) "x")))))))
+
+(t/deftest registry-entry-rejects-oversized-name
+  (t/is (false? (sm/validate ctp/schema:registry-entry
+                             (assoc valid-entry :name (apply str (repeat 501 "x")))))))
+
+(t/deftest registry-entry-rejects-oversized-host
+  (t/is (false? (sm/validate ctp/schema:registry-entry
+                             (assoc valid-entry :host (apply str (repeat 501 "x")))))))
+
+(t/deftest registry-entry-rejects-oversized-description
+  (t/is (false? (sm/validate ctp/schema:registry-entry
+                             (assoc valid-entry :description (apply str (repeat 4097 "x")))))))
+
+(t/deftest registry-entry-rejects-oversized-icon
+  (t/is (false? (sm/validate ctp/schema:registry-entry
+                             (assoc valid-entry :icon (apply str (repeat 262145 "x")))))))
+
+(t/deftest registry-entry-accepts-values-at-max
+  (t/is (true? (sm/validate ctp/schema:registry-entry
+                            (assoc valid-entry
+                                   :name (apply str (repeat 500 "x"))
+                                   :host (apply str (repeat 500 "x"))
+                                   :description (apply str (repeat 4096 "x"))
+                                   :icon (apply str (repeat 262144 "x"))
+                                   :code (apply str (repeat 1048576 "x")))))))
+
+(defn- make-registry
+  [n]
+  (let [ids (mapv #(str "plugin-" %) (range n))]
+    {:ids ids
+     :data (into {} (map (fn [id] [id (assoc valid-entry :plugin-id id)]) ids))}))
+
+(t/deftest plugin-registry-accepts-fifty-plugins
+  (t/is (true? (sm/validate ctp/schema:plugin-registry (make-registry 50)))))
+
+(t/deftest plugin-registry-rejects-more-than-fifty-plugins
+  (t/is (false? (sm/validate ctp/schema:plugin-registry (make-registry 51)))))

frontend/src/app/main/ui/workspace/plugins.cljs

@@ -56,7 +56,7 @@
           icon))
 
 (mf/defc plugin-entry*
-  [{:keys [index manifest user-can-edit on-open-plugin on-remove-plugin]}]
+  [{:keys [index manifest user-can-edit on-open-plugin on-remove-plugin remove-disabled]}]
 
   (let [{:keys [plugin-id host icon name description permissions]} manifest
         plugins-permissions-peek (deref refs/plugins-permissions-peek)
@@ -100,6 +100,7 @@
      [:> icon-button* {:variant "ghost"
                        :aria-label (tr "workspace.plugins.remove-plugin")
                        :on-click handle-delete-click
+                       :disabled remove-disabled
                        :icon i/delete}]]))
 
 (mf/defc plugin-management-dialog
@@ -126,6 +127,11 @@
         fetching-manifest?
         (mf/use-state false)
 
+        ;; Ids with a persist request in flight; their remove button
+        ;; is disabled until the request completes.
+        in-flight*
+        (mf/use-state #{})
+
         on-url-change
         (mf/use-fn
          (fn [value]
@@ -175,12 +181,23 @@
          (fn [plugin-index]
            (let [plugins-list (preg/plugins-list)
                  plugin (nth plugins-list plugin-index)]
-             (st/emit! (ev/event {::ev/name "remove-plugin"
-                                  :name (:name plugin)
-                                  :host (:host plugin)}))
-             (dp/close-plugin! plugin)
-             (preg/remove-plugin! plugin)
-             (reset! plugins-state* (preg/plugins-list)))))]
+             (modal/show!
+              {:type :confirm
+               :title (tr "workspace.plugins.remove-confirmation.title")
+               :message (tr "workspace.plugins.remove-confirmation.message" (:name plugin))
+               :accept-label (tr "workspace.plugins.remove-plugin")
+               :on-accept (fn [_]
+                            (st/emit! (ev/event {::ev/name "remove-plugin"
+                                                 :name (:name plugin)
+                                                 :host (:host plugin)}))
+                            (dp/close-plugin! plugin)
+                            (preg/remove-plugin! plugin)
+                            (reset! plugins-state* (preg/plugins-list))
+                            (modal/show! :plugin-management {}))}))))]
+
+    (mf/with-effect []
+      (let [listener (preg/subscribe-in-flight! #(reset! in-flight* %))]
+        (partial preg/unsubscribe-in-flight! listener)))
 
     [:div {:class (stl/css :modal-overlay)}
      [:div {:class (stl/css :modal-dialog :plugin-management)}
@@ -236,6 +253,7 @@
                                 :index idx
                                 :manifest manifest
                                 :user-can-edit user-can-edit?
+                                :remove-disabled (contains? @in-flight* (:plugin-id manifest))
                                 :on-open-plugin on-open-plugin
                                 :on-remove-plugin on-remove-plugin}])]])]]]))
 

frontend/src/app/plugins/register.cljs

@@ -122,36 +122,129 @@
 
 (declare remove-plugin!)
 
+;; Tracks plugin ids with a persist request in flight, so rapid repeated
+;; install/remove clicks on the same plugin cannot stack RPC writes.
+(defonce ^:private in-flight (atom #{}))
+
+(defonce ^:private in-flight-listeners (atom #{}))
+
+(defn subscribe-in-flight!
+  "Subscribes f, called with the in-flight id set on every change.
+  Calls f immediately with the current set. Returns f."
+  [f]
+  (swap! in-flight-listeners conj f)
+  (f @in-flight)
+  f)
+
+(defn unsubscribe-in-flight!
+  [f]
+  (swap! in-flight-listeners disj f)
+  nil)
+
+(defn- notify-in-flight!
+  []
+  (let [ids @in-flight]
+    (doseq [f @in-flight-listeners]
+      (f ids))))
+
+(defn- track!
+  [plugin-id]
+  (swap! in-flight conj plugin-id)
+  (notify-in-flight!))
+
+(defn- release!
+  [plugin-id]
+  (swap! in-flight disj plugin-id)
+  (notify-in-flight!))
+
+(defn- validation-error?
+  [err]
+  (= :validation (:type (ex-data err))))
+
+(defn- drop-local!
+  [{:keys [plugin-id]}]
+  (swap! registry #(-> %
+                       (update :ids (fn [ids] (vec (remove (partial = plugin-id) ids))))
+                       (update :data dissoc plugin-id))))
+
+(defn- insert-at
+  [ids idx id]
+  (let [v   (vec ids)
+        idx (max 0 (min idx (count v)))]
+    (vec (concat (subvec v 0 idx) [id] (subvec v idx)))))
+
 (defn install-plugin!
   [plugin]
-  (letfn [(update-ids [ids]
-            (conj
-             (->> ids (remove #(= % (:plugin-id plugin))))
-             (:plugin-id plugin)))]
-    (swap! registry #(-> %
-                         (update :ids update-ids)
-                         (update :data assoc (:plugin-id plugin) plugin)))
-    (->> (rp/cmd! :add-profile-plugin {:plugin plugin})
-         (rx/subs! identity
-                   (fn [err]
-                     (remove-plugin! plugin)
-                     (.error js/console "Failed to install plugin:" err))))))
+  (let [plugin-id (:plugin-id plugin)
+        previous  (get-plugin plugin-id)
+        prev-idx  (.indexOf (vec (:ids @registry)) plugin-id)]
+    (when-not (contains? @in-flight plugin-id)
+      (track! plugin-id)
+      (letfn [(update-ids [ids]
+                (conj
+                 (->> ids (remove #(= % (:plugin-id plugin))))
+                 (:plugin-id plugin)))]
+        (swap! registry #(-> %
+                             (update :ids update-ids)
+                             (update :data assoc (:plugin-id plugin) plugin)))
+        (->> (rp/cmd! :add-profile-plugin {:plugin plugin})
+             (rx/subs! (fn [_]
+                         (release! plugin-id))
+                       (fn [err]
+                         (release! plugin-id)
+                         (if (validation-error? err)
+                           ;; The server kept the previous version (if any)
+                           ;; in its original position: drop the optimistic
+                           ;; entry and restore both position and data.
+                           (if previous
+                             (swap! registry #(-> %
+                                                  (update :ids (fn [ids]
+                                                                 (insert-at (remove (partial = plugin-id) ids)
+                                                                            prev-idx
+                                                                            plugin-id)))
+                                                  (assoc-in [:data plugin-id] previous)))
+                             (drop-local! plugin))
+                           ;; One-shot compensating write with terminal
+                           ;; callbacks: never re-arms tracking or rollback.
+                           (do
+                             (drop-local! plugin)
+                             (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id})
+                                  (rx/subs! (fn [_] nil)
+                                            (fn [err2]
+                                              (.error js/console "Rollback remove failed:" err2))))))
+                         (.error js/console "Failed to install plugin:" err))))))))
 
 (defn remove-plugin!
   [{:keys [plugin-id]}]
-  (let [plugin (get-plugin plugin-id)]
-    (letfn [(update-ids [ids]
-              (->> ids
-                   (remove #(= % plugin-id))))]
-      (swap! registry #(-> %
-                           (update :ids update-ids)
-                           (update :data dissoc plugin-id)))
-      (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id})
-           (rx/subs! identity
-                     (fn [err]
-                       (when plugin
-                         (install-plugin! plugin))
-                       (.error js/console "Failed to remove plugin:" err)))))))
+  (let [stored   (get-plugin plugin-id)
+        prev-idx (.indexOf (vec (:ids @registry)) plugin-id)]
+    (when-not (contains? @in-flight plugin-id)
+      (track! plugin-id)
+      (letfn [(update-ids [ids]
+                (->> ids
+                     (remove #(= % plugin-id))))]
+        (swap! registry #(-> %
+                             (update :ids update-ids)
+                             (update :data dissoc plugin-id)))
+        (->> (rp/cmd! :remove-profile-plugin {:plugin-id plugin-id})
+             (rx/subs! (fn [_]
+                         (release! plugin-id))
+                       (fn [err]
+                         (release! plugin-id)
+                         (when stored
+                           ;; Restore at the original position; the server
+                           ;; still holds the entry on validation errors.
+                           (swap! registry #(-> %
+                                                (update :ids insert-at prev-idx plugin-id)
+                                                (update :data assoc plugin-id stored)))
+                           ;; One-shot compensating write with terminal
+                           ;; callbacks on any other failure.
+                           (when-not (validation-error? err)
+                             (->> (rp/cmd! :add-profile-plugin {:plugin stored})
+                                  (rx/subs! (fn [_] nil)
+                                            (fn [err2]
+                                              (.error js/console "Rollback install failed:" err2))))))
+                         (.error js/console "Failed to remove plugin:" err))))))))
 
 (defn check-permission
   [plugin-id permission]

frontend/test/frontend_tests/plugins/register_test.cljs

@@ -0,0 +1,271 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS SUBSIDIARY SL
+
+(ns frontend-tests.plugins.register-test
+  (:require
+   [app.main.repo :as rp]
+   [app.plugins.register :as preg]
+   [beicon.v2.core :as rx]
+   [cljs.test :as t :include-macros true]
+   [frontend-tests.helpers.mock :as mock]))
+
+(defn- record-cmd-mock
+  "Mock rp/cmd! that records calls in mock/rpc-calls and answers
+  with (response-fn cmd params)."
+  [response-fn]
+  (mock/stub
+   (fn [cmd params]
+     (swap! mock/rpc-calls conj {:cmd cmd :params params})
+     (response-fn cmd params))))
+
+(defn- cmds
+  []
+  (mapv :cmd @mock/rpc-calls))
+
+;; --- install-plugin! ---
+
+(t/deftest install-success-releases-id
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock (fn [_ _] (rx/of {:ok true})))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-install-ok"}]
+          (preg/install-plugin! plugin)
+          (t/is (= [:add-profile-plugin] (cmds)))
+          (t/is (= plugin (preg/get-plugin "reg-install-ok")))
+          ;; id released: a second install issues a second RPC
+          (preg/install-plugin! plugin)
+          (t/is (= 2 (count @mock/rpc-calls)))
+          (done')))
+      done)))
+
+(t/deftest install-while-in-flight-issues-no-second-rpc
+  (t/async done
+    (let [subjects (atom [])]
+      (mock/with-mocks
+        {rp/cmd! (record-cmd-mock
+                  (fn [_ _]
+                    (let [sb (rx/subject)]
+                      (swap! subjects conj sb)
+                      sb)))}
+        (fn [done']
+          (let [plugin {:plugin-id "reg-install-dedupe"}]
+            (preg/install-plugin! plugin)
+            (preg/install-plugin! plugin)
+            (t/is (= 1 (count @mock/rpc-calls)) "second install while in flight is skipped")
+            ;; complete the pending call; the id is released afterwards
+            (rx/push! (first @subjects) {:ok true})
+            (preg/install-plugin! plugin)
+            (t/is (= 2 (count @mock/rpc-calls)))
+            (done')))
+        done))))
+
+(t/deftest install-validation-error-cleans-local-only
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [_ _] (rx/throw (ex-info "rejected" {:type :validation :code :props-too-large}))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-install-validation"}]
+          (preg/install-plugin! plugin)
+          (t/is (= 1 (count @mock/rpc-calls)) "no rollback write after validation rejection")
+          (t/is (nil? (preg/get-plugin "reg-install-validation")))
+          ;; id released: the next install retries the RPC
+          (preg/install-plugin! plugin)
+          (t/is (= 2 (count @mock/rpc-calls)))
+          (done')))
+      done)))
+
+(t/deftest install-validation-error-restores-previous-version
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [_ params]
+                  (if (= "v2" (get-in params [:plugin :name]))
+                    (rx/throw (ex-info "rejected" {:type :validation}))
+                    (rx/of {:ok true}))))}
+      (fn [done']
+        (let [v1 {:plugin-id "reg-install-prev" :name "v1"}
+              v2 {:plugin-id "reg-install-prev" :name "v2"}]
+          (preg/install-plugin! v1)
+          (preg/install-plugin! v2)
+          (t/is (= 2 (count @mock/rpc-calls)))
+          (t/is (= v1 (preg/get-plugin "reg-install-prev"))
+                "the server-kept version is restored, not dropped")
+          (done')))
+      done)))
+
+(t/deftest install-validation-error-keeps-original-position
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [_ params]
+                  (if (= "v2" (get-in params [:plugin :name]))
+                    (rx/throw (ex-info "rejected" {:type :validation}))
+                    (rx/of {:ok true}))))}
+      (fn [done']
+        (let [own #{"reg-pos-a" "reg-pos-b" "reg-pos-c"}
+              v1  {:plugin-id "reg-pos-b" :name "v1"}
+              v2  {:plugin-id "reg-pos-b" :name "v2"}]
+          (preg/install-plugin! {:plugin-id "reg-pos-a"})
+          (preg/install-plugin! v1)
+          (preg/install-plugin! {:plugin-id "reg-pos-c"})
+          (preg/install-plugin! v2)
+          ;; installs prepend, so newest-first; the rejected update
+          ;; must preserve order and restore v1
+          (t/is (= ["reg-pos-c" "reg-pos-b" "reg-pos-a"]
+                   (filterv own (mapv :plugin-id (preg/plugins-list)))))
+          (t/is (= v1 (preg/get-plugin "reg-pos-b")))
+          (done')))
+      done)))
+
+(t/deftest install-persistent-failure-terminates
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [_ _] (rx/throw (ex-info "boom" {:type :other}))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-install-hang"}]
+          (preg/install-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds))
+                "one-shot rollback: no further calls")
+          (t/is (nil? (preg/get-plugin "reg-install-hang")))
+          (done')))
+      done)))
+
+(t/deftest install-non-validation-error-rolls-back-via-rpc
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [cmd _]
+                  (if (= cmd :add-profile-plugin)
+                    (rx/throw (ex-info "boom" {:type :other}))
+                    (rx/of nil))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-install-rollback"}]
+          (preg/install-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds)))
+          (t/is (nil? (preg/get-plugin "reg-install-rollback")))
+          (done')))
+      done)))
+
+;; --- remove-plugin! ---
+
+(t/deftest remove-success-releases-id
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock (fn [_ _] (rx/of {:ok true})))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-remove-ok"}]
+          (preg/install-plugin! plugin)
+          (preg/remove-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds)))
+          (t/is (nil? (preg/get-plugin "reg-remove-ok")))
+          ;; id released: a second remove issues another RPC
+          (preg/remove-plugin! plugin)
+          (t/is (= 3 (count @mock/rpc-calls)))
+          (done')))
+      done)))
+
+(t/deftest remove-while-in-flight-issues-no-second-rpc
+  (t/async done
+    (let [subjects (atom [])]
+      (mock/with-mocks
+        {rp/cmd! (record-cmd-mock
+                  (fn [cmd _]
+                    (if (= cmd :add-profile-plugin)
+                      (rx/of {:ok true})
+                      (let [sb (rx/subject)]
+                        (swap! subjects conj sb)
+                        sb))))}
+        (fn [done']
+          (let [plugin {:plugin-id "reg-remove-dedupe"}]
+            (preg/install-plugin! plugin)
+            (preg/remove-plugin! plugin)
+            (preg/remove-plugin! plugin)
+            (t/is (= 2 (count @mock/rpc-calls)) "second remove while in flight is skipped")
+            ;; complete the pending call; the id is released afterwards
+            (rx/push! (first @subjects) nil)
+            (preg/remove-plugin! plugin)
+            (t/is (= 3 (count @mock/rpc-calls)))
+            (done')))
+        done))))
+
+(t/deftest remove-validation-error-restores-local-only
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [cmd _]
+                  (if (= cmd :add-profile-plugin)
+                    (rx/of {:ok true})
+                    (rx/throw (ex-info "rejected" {:type :validation})))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-remove-validation"}]
+          (preg/install-plugin! plugin)
+          (preg/remove-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin] (cmds))
+                "no reinstall write after validation rejection")
+          (t/is (= plugin (preg/get-plugin "reg-remove-validation"))
+                "local entry is restored")
+          (done')))
+      done)))
+
+(t/deftest remove-non-validation-error-reinstalls-via-rpc
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [cmd _]
+                  (if (= cmd :remove-profile-plugin)
+                    (rx/throw (ex-info "boom" {:type :other}))
+                    (rx/of {:ok true}))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-remove-rollback"}]
+          (preg/install-plugin! plugin)
+          (preg/remove-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin :add-profile-plugin] (cmds)))
+          (t/is (= plugin (preg/get-plugin "reg-remove-rollback")))
+          (done')))
+      done)))
+
+(t/deftest remove-persistent-failure-terminates
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [cmd _]
+                  (if (= cmd :add-profile-plugin)
+                    (rx/of {:ok true})
+                    (rx/throw (ex-info "boom" {:type :other})))))}
+      (fn [done']
+        (let [plugin {:plugin-id "reg-remove-hang"}]
+          (preg/install-plugin! plugin)
+          (preg/remove-plugin! plugin)
+          (t/is (= [:add-profile-plugin :remove-profile-plugin :add-profile-plugin] (cmds))
+                "one-shot rollback: no further calls")
+          (t/is (= plugin (preg/get-plugin "reg-remove-hang")))
+          (done')))
+      done)))
+
+(t/deftest remove-validation-error-keeps-original-position
+  (t/async done
+    (mock/with-mocks
+      {rp/cmd! (record-cmd-mock
+                (fn [cmd _]
+                  (if (= cmd :add-profile-plugin)
+                    (rx/of {:ok true})
+                    (rx/throw (ex-info "rejected" {:type :validation})))))}
+      (fn [done']
+        (let [own    #{"reg-idx-a" "reg-idx-b" "reg-idx-c"}
+              plugin {:plugin-id "reg-idx-b"}]
+          (preg/install-plugin! {:plugin-id "reg-idx-a"})
+          (preg/install-plugin! plugin)
+          (preg/install-plugin! {:plugin-id "reg-idx-c"})
+          (preg/remove-plugin! plugin)
+          ;; installs prepend, so the order is newest-first;
+          ;; the failed removal must preserve it exactly
+          (t/is (= ["reg-idx-c" "reg-idx-b" "reg-idx-a"]
+                   (filterv own (mapv :plugin-id (preg/plugins-list)))))
+          (done')))
+      done)))

frontend/test/frontend_tests/runner.cljs

@@ -59,6 +59,7 @@
    [frontend-tests.plugins.page-active-validation-test]
    [frontend-tests.plugins.page-test]
    [frontend-tests.plugins.parser-test]
+   [frontend-tests.plugins.register-test]
    [frontend-tests.plugins.shape-bugfixes-test]
    [frontend-tests.plugins.text-test]
    [frontend-tests.plugins.tokens-test]
@@ -168,6 +169,7 @@
    'frontend-tests.plugins.page-active-validation-test
    'frontend-tests.plugins.page-test
    'frontend-tests.plugins.parser-test
+   'frontend-tests.plugins.register-test
    'frontend-tests.plugins.shape-bugfixes-test
    'frontend-tests.plugins.text-test
    'frontend-tests.plugins.tokens-test

frontend/translations/en.po

@@ -9073,7 +9073,17 @@ msgstr "Read the profile information of the current user."
 msgid "workspace.plugins.plugin-list-link"
 msgstr "Plugins List"
 
-#: src/app/main/ui/workspace/plugins.cljs:101
+#: src/app/main/ui/workspace/plugins.cljs:187
+msgid "workspace.plugins.remove-confirmation.message"
+msgstr ""
+"Are you sure you want to remove the plugin %s? You can install it again at "
+"any time."
+
+#: src/app/main/ui/workspace/plugins.cljs:186
+msgid "workspace.plugins.remove-confirmation.title"
+msgstr "Remove plugin"
+
+#: src/app/main/ui/workspace/plugins.cljs:101, src/app/main/ui/workspace/plugins.cljs:188
 msgid "workspace.plugins.remove-plugin"
 msgstr "Remove plugin"
 

frontend/translations/es.po

@@ -8821,7 +8821,15 @@ msgstr "Leer la información del usuario actual."
 msgid "workspace.plugins.plugin-list-link"
 msgstr "Lista de extensiones"
 
-#: src/app/main/ui/workspace/plugins.cljs:101
+#: src/app/main/ui/workspace/plugins.cljs:187
+msgid "workspace.plugins.remove-confirmation.message"
+msgstr "¿Seguro que quieres eliminar la extensión %s? Puedes instalarla de nuevo en cualquier momento."
+
+#: src/app/main/ui/workspace/plugins.cljs:186
+msgid "workspace.plugins.remove-confirmation.title"
+msgstr "Eliminar extensión"
+
+#: src/app/main/ui/workspace/plugins.cljs:101, src/app/main/ui/workspace/plugins.cljs:188
 msgid "workspace.plugins.remove-plugin"
 msgstr "Eliminar extensión"