diff --git a/backend/src/app/rpc/commands/access_token.clj b/backend/src/app/rpc/commands/access_token.clj index 331e2bdb36..0aee2c42a8 100644 --- a/backend/src/app/rpc/commands/access_token.clj +++ b/backend/src/app/rpc/commands/access_token.clj @@ -22,6 +22,12 @@ [row] (dissoc row :perms)) +(def ^:private sql:clean-old-mcp-tokens + "DELETE FROM access_token + WHERE profile_id = ? + AND id != ? + AND type = 'mcp'") + (defn create-access-token [{:keys [::db/conn] :as cfg} profile-id name expiration type] (let [token-id (uuid/next) @@ -42,6 +48,13 @@ :updated-at created-at :expires-at expires-at :perms (db/create-array conn "text" [])})] + + ;; If the created token is of mcp type, we should proceed to + ;; delete all other mcp tokens on the table for the current + ;; profile. + (when (= type "mcp") + (db/exec! conn [sql:clean-old-mcp-tokens profile-id (:id token)])) + (decode-row token))) (defn repl:create-access-token @@ -85,14 +98,20 @@ (->> (db/query pool :access-token {:profile-id profile-id} {:order-by [[:expires-at :asc] [:created-at :asc]] - :columns [:id :name :perms :type :created-at :updated-at :expires-at]}) - (mapv decode-row))) + :columns [:id :name :perms :type :created-at :updated-at :expires-at :token]}) + (map decode-row) + (map (fn [{:keys [type] :as row}] + (if (not= type "mcp") + (dissoc row :token) + row))) + (vec))) (def ^:private schema:get-current-mcp-token [:map {:title "get-current-mcp-token"}]) (sv/defmethod ::get-current-mcp-token {::doc/added "2.15" + ::doc/deprecated true ::sm/params schema:get-current-mcp-token} [{:keys [::db/pool]} {:keys [::rpc/profile-id ::rpc/request-at]}] (->> (db/query pool :access-token diff --git a/backend/test/backend_tests/rpc_access_tokens_test.clj b/backend/test/backend_tests/rpc_access_tokens_test.clj index fb08dc65fa..5554633b72 100644 --- a/backend/test/backend_tests/rpc_access_tokens_test.clj +++ b/backend/test/backend_tests/rpc_access_tokens_test.clj @@ -120,5 +120,83 @@ ::rpc/profile-id (:id prof)})] ;; (th/print-result! result) (t/is (nil? error)) - (t/is (string? (:token result))))))) + (t/is (string? (:token result))))) + (t/testing "get-access-tokens returns :token for MCP tokens but not for regular tokens" + (let [;; Create a regular token + regular-out (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :name "regular token" + :perms ["get-profile"]}) + regular-token (:result regular-out) + + ;; Create an MCP token + mcp-out (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :type "mcp" + :name "mcp token" + :perms []}) + mcp-token (:result mcp-out) + + ;; Fetch all tokens + {:keys [error result]} + (th/command! {::th/type :get-access-tokens + ::rpc/profile-id (:id prof)})] + + (t/is (nil? error)) + + ;; Find our tokens in the result + (let [regular (some #(when (= (:id %) (:id regular-token)) %) result) + mcp (some #(when (= (:id %) (:id mcp-token)) %) result)] + + ;; Regular tokens should NOT have :token + (t/is (some? regular)) + (t/is (not (contains? regular :token))) + + ;; MCP tokens SHOULD have :token + (t/is (some? mcp)) + (t/is (contains? mcp :token)) + (t/is (string? (:token mcp)))))) + + (t/testing "creating MCP token removes previous MCP tokens" + (let [;; Create first MCP token + first-out (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :type "mcp" + :name "first mcp" + :perms []}) + first-mcp (:result first-out) + + ;; Create second MCP token + second-out (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :type "mcp" + :name "second mcp" + :perms []}) + second-mcp (:result second-out) + + ;; Create third MCP token + third-out (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :type "mcp" + :name "third mcp" + :perms []}) + third-mcp (:result third-out) + + ;; Fetch all tokens + {:keys [error result]} + (th/command! {::th/type :get-access-tokens + ::rpc/profile-id (:id prof)})] + + (t/is (nil? error)) + + ;; Count MCP tokens - should only be 1 (the third one) + (let [mcp-tokens (filter #(= (:type %) "mcp") result)] + (t/is (= 1 (count mcp-tokens))) + (t/is (= (:id third-mcp) (:id (first mcp-tokens))))) + + ;; Verify the first and second MCP tokens are gone + (let [all-ids (set (map :id result))] + (t/is (not (contains? all-ids (:id first-mcp)))) + (t/is (not (contains? all-ids (:id second-mcp)))) + (t/is (contains? all-ids (:id third-mcp)))))))) diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index c9eaf0e802..f6eabba1f9 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -473,6 +473,9 @@ (defn access-tokens-fetched [access-tokens] (ptk/reify ::access-tokens-fetched + IDeref + (-deref [_] access-tokens) + ptk/UpdateEvent (update [_ state] (assoc state :access-tokens access-tokens)))) @@ -487,21 +490,25 @@ ;; --- EVENT: create-access-token -(defn access-token-created +(defn- access-token-created [access-token] (ptk/reify ::access-token-created ptk/UpdateEvent (update [_ state] - (assoc state :access-token-created access-token)))) + (-> state + (assoc :access-token-created access-token) + (update :access-tokens conj access-token))))) (defn create-access-token - [{:keys [] :as params}] + [params] (ptk/reify ::create-access-token ptk/WatchEvent (watch [_ _ _] (let [{:keys [on-success on-error] :or {on-success identity - on-error rx/throw}} (meta params)] + on-error rx/throw}} + (meta params)] + (->> (rp/cmd! :create-access-token params) (rx/map access-token-created) (rx/tap on-success) @@ -511,8 +518,15 @@ (defn delete-access-token [{:keys [id] :as params}] - (assert (uuid? id)) + (assert (uuid? id) "expect valid token id") + (ptk/reify ::delete-access-token + ptk/UpdateEvent + (update [_ state] + (update state :access-tokens + (fn [tokens] + (into [] (remove #(= id (:id %))) tokens)))) + ptk/WatchEvent (watch [_ _ _] (let [{:keys [on-success on-error] diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 16115314b0..87668b2e8f 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -73,6 +73,7 @@ [app.main.repo :as rp] [app.main.router :as rt] [app.main.store :as st] + [app.plugins.register :as preg] [app.render-wasm :as wasm] [app.render-wasm.api :as wasm.api] [app.render-wasm.wasm :as wasm-state] @@ -253,8 +254,12 @@ (rx/merge (rx/of (dp/check-open-plugin) (fdf/fix-deleted-fonts-for-local-library file-id)) - (when (contains? cf/flags :mcp) - (rx/of (mcp/init))))))) + (if (contains? cf/flags :mcp) + ;; We wait the plugin runtime to be ready before launch the + ;; mcp initialization + (->> (rx/from (preg/wait-for-runtime)) + (rx/map (fn [_] (mcp/init)))) + (rx/empty)))))) (defn- bundle-fetched [{:keys [file file-id thumbnails] :as bundle}] @@ -376,10 +381,7 @@ (rx/of (ntf/hide) (dcmt/retrieve-comment-threads file-id) (dcmt/fetch-profiles) - (df/fetch-fonts team-id)) - - (when (contains? cf/flags :mcp) - (rx/of (du/fetch-access-tokens)))) + (df/fetch-fonts team-id))) ;; Once the essential data is fetched, lets proceed to ;; fetch the file bundle diff --git a/frontend/src/app/main/data/workspace/mcp.cljs b/frontend/src/app/main/data/workspace/mcp.cljs index f2c3c069db..55d15e4852 100644 --- a/frontend/src/app/main/data/workspace/mcp.cljs +++ b/frontend/src/app/main/data/workspace/mcp.cljs @@ -6,12 +6,14 @@ (ns app.main.data.workspace.mcp (:require + [app.common.data :as d] [app.common.logging :as log] + [app.common.time :as ct] [app.common.uri :as u] [app.config :as cf] [app.main.broadcast :as mbc] [app.main.data.plugins :as dp] - [app.main.repo :as rp] + [app.main.data.profile :as du] [app.main.store :as st] [app.plugins.register :as preg] [app.util.timers :as ts] @@ -22,7 +24,8 @@ (log/set-level! :info) -(def ^:private default-manifest +(def ^:private + default-manifest {:code "plugin.js" :name "Penpot MCP Plugin" :version 2 @@ -34,7 +37,8 @@ "comment:read" "comment:write" "content:write" "content:read"}}) -(defonce interval-sub (atom nil)) +(defonce interval-sub + (atom nil)) (defn connect-mcp [] @@ -44,20 +48,8 @@ (rx/of (mbc/event :mcp/force-disconnect {}) (ptk/data-event ::connect))))) -(defn finalize-workspace? - [event] - (= (ptk/type event) :app.main.data.workspace/finalize-workspace)) - -(defn set-mcp-active - [value] - (ptk/reify ::set-mcp-active - ptk/UpdateEvent - (update [_ state] - (assoc-in state [:mcp :active] value)))) - -(defn start-reconnect-watcher! +(defn- start-reconnect-watcher [] - (st/emit! (set-mcp-active true)) (when (nil? @interval-sub) (reset! interval-sub @@ -72,7 +64,6 @@ (defn stop-reconnect-watcher! [] - (st/emit! (set-mcp-active false)) (when @interval-sub (rx/dispose! @interval-sub) (reset! interval-sub nil))) @@ -83,7 +74,9 @@ (ptk/reify ::update-mcp-status ptk/UpdateEvent (update [_ state] - (update-in state [:profile :props] assoc :mcp-enabled value)) + (-> state + (update :mcp assoc :enabled value) + (update-in [:profile :props] assoc :mcp-enabled value))) ptk/WatchEvent (watch [_ _ _] @@ -123,76 +116,120 @@ (effect [_ _ _] (stop-reconnect-watcher!)))) -(defn init-mcp - [stream] - ;; Wait for plugins runtime to be initialized before starting the MCP plugin. - ;; This ensures global.ɵloadPlugin is available when start-plugin! is called. - (->> (rx/from (preg/wait-for-runtime)) - (rx/mapcat - (fn [_] - (->> (rp/cmd! :get-current-mcp-token) - (rx/tap - (fn [{:keys [token]}] - (when token - (dp/start-plugin! - (assoc default-manifest - :url (str (u/join cf/public-uri "plugins/mcp/manifest.json")) - :host (str (u/join cf/public-uri "plugins/mcp/"))) +(defn- init-mcp-plugin + "Waits the plugin runtime and initializes the bundled MCP plugin." + [{:keys [token]}] + (ptk/reify ::init-mcp-plugin + ptk/EffectEvent + (effect [_ _ stream] + (let [manifest (-> default-manifest + (assoc :url (str (u/join cf/public-uri "plugins/mcp/manifest.json"))) + (assoc :host (str (u/join cf/public-uri "plugins/mcp/")))) - ;; API extension for MCP server - #js {:mcp - #js - {:getToken (constantly token) - :getServerUrl #(str cf/mcp-ws-uri) - :setMcpStatus - (fn [status] - (when (= status "connected") - (start-reconnect-watcher!)) - (st/emit! (update-mcp-connection-status status)) - (log/info :hint "MCP STATUS" :status status)) + stopper-s (rx/merge + (rx/filter (ptk/type? :app.main.data.workspace/finalize-workspace) stream) + (rx/filter (ptk/type? ::stop-mcp-plugin) stream)) - :on - (fn [event cb] - (when-let [event - (case event - "disconnect" ::disconnect - "connect" ::connect - nil)] + extension #js {:getToken (constantly token) + :getServerUrl #(str cf/mcp-ws-uri) + :setMcpStatus + (fn [status] + (when (= status "connected") + (start-reconnect-watcher)) + (st/emit! (update-mcp-connection-status status)) + (log/info :hint "MCP STATUS" :status status)) - (let [stopper (rx/filter finalize-workspace? stream)] - (->> stream - (rx/filter (ptk/type? event)) - (rx/take-until stopper) - (rx/subs! #(cb))))))}}))))))))) + :on + (fn [event cb] + (when-let [event + (case event + "disconnect" ::disconnect + "connect" ::connect + nil)] + + (->> stream + (rx/filter (ptk/type? event)) + (rx/take-until stopper-s) + (rx/subs! (fn [_] (cb))))))}] + + (dp/start-plugin! manifest #js {:mcp extension}))))) + +(defn- stop-mcp-plugin + [] + (ptk/reify ::stop-mcp-plugin + ptk/EffectEvent + (effect [_ _ _] + (dp/close-plugin! default-manifest)))) + +(defn- init-mcp-state + [access-tokens] + (let [token (d/seek #(= "mcp" (:type %)) access-tokens) + valid? (and token + (as-> (get token :expires-at) expires-at + (or (nil? expires-at) + (> expires-at (ct/now)))))] + + (ptk/reify ::init-mcp-state + IDeref + (-deref [_] + (when valid? token)) + + ptk/UpdateEvent + (update [_ state] + (if token + (update state :mcp (fn [state] + (-> state + (assoc :token (:token token)) + (assoc :token-id (:id token)) + (assoc :token-valid valid?)))) + state))))) (defn init + "Initialize MCP runtime. This event expects plugin runtime initialized." [] (ptk/reify ::init ptk/UpdateEvent (update [_ state] - (update state :mcp assoc :active true)) + (let [profile (get state :profile) + mcp-enabled? (-> profile :props :mcp-enabled boolean)] + (update state :mcp assoc :enabled mcp-enabled?))) ptk/WatchEvent (watch [_ state stream] - (let [stoper-s (rx/merge + (let [stopper-s (rx/merge (rx/filter (ptk/type? :app.main.data.workspace/finalize-workspace) stream) (rx/filter (ptk/type? ::init) stream)) + session-id (get state :session-id) - enabled? (-> state :profile :props :mcp-enabled)] + mcp-state (get state :mcp)] (->> (rx/merge - (if enabled? - (rx/merge - (init-mcp stream) + (rx/of (du/fetch-access-tokens)) - (->> mbc/stream - (rx/filter (mbc/type? :mcp/force-disconnect)) - (rx/filter (fn [{:keys [id]}] - (not= session-id id))) - (rx/map deref) - (rx/map (fn [] (user-disconnect-mcp))))) + ;; Wait until access tokens are initialized and are + ;; setup on the state + (->> stream + (rx/filter (ptk/type? ::du/access-tokens-fetched)) + (rx/map deref) + (rx/take 1) + (rx/map init-mcp-state)) + + (if (:enabled mcp-state) + (->> stream + (rx/filter (ptk/type? ::init-mcp-state)) + (rx/take 1) + (rx/map deref) + (rx/filter some?) + (rx/map init-mcp-plugin)) (rx/empty)) + (->> mbc/stream + (rx/filter (mbc/type? :mcp/force-disconnect)) + (rx/filter (fn [{:keys [id]}] + (not= session-id id))) + (rx/map deref) + (rx/map (fn [] (user-disconnect-mcp)))) + (->> mbc/stream (rx/filter (mbc/type? :mcp/enable)) (rx/mapcat (fn [_] @@ -206,6 +243,7 @@ (rx/filter (mbc/type? :mcp/disable)) (rx/mapcat (fn [_] (rx/of (update-mcp-status false) - (user-disconnect-mcp)))))) + (user-disconnect-mcp) + (stop-mcp-plugin)))))) - (rx/take-until stoper-s)))))) + (rx/take-until stopper-s)))))) diff --git a/frontend/src/app/main/refs.cljs b/frontend/src/app/main/refs.cljs index 771bbcd04f..791520efd9 100644 --- a/frontend/src/app/main/refs.cljs +++ b/frontend/src/app/main/refs.cljs @@ -10,7 +10,6 @@ [app.common.data :as d] [app.common.data.macros :as dm] [app.common.files.helpers :as cph] - [app.common.time :as ct] [app.common.types.shape-tree :as ctt] [app.common.types.shape.layout :as ctl] [app.common.types.tokens-lib :as ctob] @@ -156,14 +155,6 @@ (def mcp (l/derived :mcp st/state)) -(def mcp-key-expired? - (l/derived (fn [state] - (when-let [expires-at (some->> (:access-tokens state) - (some #(when (= (:type %) "mcp") %)) - :expires-at)] - (> (ct/now) expires-at))) - st/state)) - (def workspace-drawing (l/derived :workspace-drawing st/state)) diff --git a/frontend/src/app/main/ui/settings/integrations.cljs b/frontend/src/app/main/ui/settings/integrations.cljs index e7d51438bc..ffd7db5ad1 100644 --- a/frontend/src/app/main/ui/settings/integrations.cljs +++ b/frontend/src/app/main/ui/settings/integrations.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.settings.integrations (:require-macros [app.main.style :as stl]) (:require + [app.common.data :as d] [app.common.data.macros :as dm] [app.common.schema :as sm] [app.common.time :as ct] @@ -43,7 +44,7 @@ [:name [::sm/text {:max 250}]] [:expiration-date [::sm/text {:max 250}]]]) -(def ^:private schema:form-mcp-key +(def ^:private schema:form-mcp-token [:map [:expiration-date [::sm/text {:max 250}]]]) @@ -51,7 +52,7 @@ {:name "" :expiration-date "never"}) -(def form-initial-data-mcp-key +(def form-initial-data-mcp-token {:expiration-date "never"}) (mf/defc input-copy* @@ -70,7 +71,7 @@ (mf/defc token-created* {::mf/private true} - [{:keys [title mcp-key?]}] + [{:keys [title is-mcp]}] (let [token-created (mf/deref refs/access-token-created) on-copy-to-clipboard @@ -81,7 +82,7 @@ (clipboard/to-clipboard (:token token-created)) (st/emit! (ntf/show {:level :info :type :toast - :content (if mcp-key? + :content (if is-mcp (tr "integrations.notification.success.mcp-key-copied") (tr "integrations.notification.success.token-copied")) :timeout notification-timeout}))))] @@ -97,7 +98,7 @@ [:> text* {:as "div" :typography t/body-small :class (stl/css :color-primary)} - (if mcp-key? + (if is-mcp (tr "integrations.mcp-key.info.non-recuperable") (tr "integrations.token.info.non-recuperable"))]] @@ -109,14 +110,14 @@ :typography t/body-small :class (stl/css :color-secondary)} (if (:expires-at token-created) - (if mcp-key? + (if is-mcp (tr "integrations.mcp-key.will-expire" (ct/format-inst (:expires-at token-created) "PPP")) (tr "integrations.token.will-expire" (ct/format-inst (:expires-at token-created) "PPP"))) - (if mcp-key? + (if is-mcp (tr "integrations.mcp-key.will-not-expire") (tr "integrations.token.will-not-expire")))]] - (when mcp-key? + (when is-mcp [:div {:class (stl/css :modal-content)} [:> text* {:as "div" :typography t/body-small @@ -125,15 +126,15 @@ [:textarea {:class (stl/css :textarea) :wrap "off" :rows 7 - :read-only true} - (dm/str - "{\n" - " \"mcpServers\": {\n" - " \"penpot\": {\n" - " \"url\": \"" cf/mcp-server-url "?userToken=" (:token token-created "") "\"\n" - " }\n" - " }" - "\n}")]]) + :read-only true + :value (dm/str + "{\n" + " \"mcpServers\": {\n" + " \"penpot\": {\n" + " \"url\": \"" cf/mcp-server-url "?userToken=" (:token token-created "") "\"\n" + " }\n" + " }" + "\n}")}]]) [:div {:class (stl/css :modal-footer)} [:> button* {:variant "secondary" @@ -142,13 +143,13 @@ (mf/defc create-token* {::mf/private true} - [{:keys [title info mcp-key? on-created]}] + [{:keys [title info is-mcp on-created]}] (let [form (fm/use-form - :initial (if mcp-key? - form-initial-data-mcp-key + :initial (if is-mcp + form-initial-data-mcp-token form-initial-data-access-token) - :schema (if mcp-key? - schema:form-mcp-key + :schema (if is-mcp + schema:form-mcp-token schema:form-access-token)) on-error @@ -158,12 +159,14 @@ on-success (mf/use-fn - #(st/emit! (du/fetch-access-tokens) - (ntf/success (tr "integrations.notification.success.created")) - (on-created))) + (mf/deps on-created) + (fn [] + (when (fn? on-created) + (on-created)))) on-submit (mf/use-fn + (mf/deps is-mcp) (fn [form] (let [cdata (:clean-data @form) mdata {:on-success (partial on-success form) @@ -171,9 +174,12 @@ expiration (:expiration-date cdata) params (cond-> {:name (:name cdata) :perms (:perms cdata)} - (not= "never" expiration) (assoc :expiration expiration) - (true? mcp-key?) (assoc :type "mcp" - :name "MCP key"))] + (not= "never" expiration) + (assoc :expiration expiration) + + (true? is-mcp) + (assoc :type "mcp" :name "MCP key"))] + (st/emit! (du/create-access-token (with-meta params mdata))))))] [:> fc/form* {:form form @@ -193,7 +199,7 @@ :class (stl/css :color-primary)} info]]) - (if mcp-key? + (if is-mcp [:div {:class (stl/css :modal-content)} [:> text* {:as "div" :typography t/body-medium @@ -233,7 +239,8 @@ {::mf/register modal/components ::mf/register-as :create-access-token} [] - (let [created? (mf/use-state false) + (let [created? + (mf/use-state false) on-close (mf/use-fn @@ -243,7 +250,9 @@ on-created (mf/use-fn - #(reset! created? true))] + (fn [] + (reset! created? true) + (st/emit! (ntf/success (tr "integrations.notification.success.created")))))] [:div {:class (stl/css :modal-overlay)} [:div {:class (stl/css :modal-container)} @@ -258,9 +267,9 @@ [:> create-token* {:title (tr "integrations.create-access-token.title") :on-created on-created}])]])) -(mf/defc generate-mcp-key-modal +(mf/defc generate-mcp-token-modal {::mf/register modal/components - ::mf/register-as :generate-mcp-key} + ::mf/register-as :generate-mcp-token} [] (let [created? (mf/use-state false) @@ -273,7 +282,10 @@ on-created (mf/use-fn (fn [] + ;; NOTE: Analytics events use "key" terminology for historical reasons; + ;; these names are immutable to avoid breaking analytics dashboards. (st/emit! (du/update-profile-props {:mcp-enabled true}) + (ntf/success (tr "integrations.notification.success.created")) (ev/event {::ev/name "generate-mcp-key" ::ev/origin "integrations"}) (ev/event {::ev/name "enable-mcp" @@ -292,20 +304,20 @@ (if @created? [:> token-created* {:title (tr "integrations.generate-mcp-key.title.created") - :mcp-key? true}] + :is-mcp true}] [:> create-token* {:title (tr "integrations.generate-mcp-key.title") - :mcp-key? true + :is-mcp true :on-created on-created}])]])) -(mf/defc regenerate-mcp-key-modal - {::mf/register modal/components - ::mf/register-as :regenerate-mcp-key} - [] - (let [created? (mf/use-state false) +(defn- mcp-access-token? + [atoken] + (= "mcp" (:type atoken))) - tokens (mf/deref refs/access-tokens) - mcp-key (some #(when (= (:type %) "mcp") %) tokens) - mcp-key-id (:id mcp-key) +(mf/defc regenerate-mcp-token-modal + {::mf/register modal/components + ::mf/register-as :regenerate-mcp-token} + [] + (let [created? (mf/use-state false) on-close (mf/use-fn @@ -316,8 +328,9 @@ on-created (mf/use-fn (fn [] - (st/emit! (du/delete-access-token {:id mcp-key-id}) - (du/update-profile-props {:mcp-enabled true}) + ;; NOTE: Analytics event uses "key" terminology for historical reasons. + (st/emit! (du/update-profile-props {:mcp-enabled true}) + (ntf/success (tr "integrations.notification.success.created")) (ev/event {::ev/name "regenerate-mcp-key" ::ev/origin "integrations"}) (mbc/event :mcp/enable {})) @@ -333,16 +346,16 @@ (if @created? [:> token-created* {:title (tr "integrations.regenerate-mcp-key.title.created") - :mcp-key? true}] + :is-mcp true}] [:> create-token* {:title (tr "integrations.regenerate-mcp-key.title") :info (tr "integrations.regenerate-mcp-key.info") - :mcp-key? true + :is-mcp true :on-created on-created}])]])) (mf/defc token-item* {::mf/private true ::mf/wrap [mf/memo]} - [{:keys [name expires-at on-delete]}] + [{:keys [id name expires-at on-delete]}] (let [expires-txt (some-> expires-at (ct/format-inst "PPP")) expired? (and (some? expires-at) (> (ct/now) expires-at)) @@ -357,18 +370,23 @@ (mf/use-fn #(reset! menu-open* (not menu-open?))) + handle-delete + (mf/use-fn + (mf/deps on-delete id) + #(on-delete id)) + handle-open-confirm-modal (mf/use-fn - (mf/deps on-delete) + (mf/deps handle-delete) (fn [] (st/emit! (modal/show {:type :confirm :title (tr "integrations.delete-token.title") :message (tr "integrations.delete-token.message") :accept-label (tr "integrations.delete-token.accept") - :on-accept on-delete})))) + :on-accept handle-delete})))) options - (mf/with-memo [on-delete] + (mf/with-memo [handle-delete] [{:name (tr "labels.delete") :id "token-delete" :handler handle-open-confirm-modal}])] @@ -403,22 +421,31 @@ :left -138 :options options}]]])) +(defn valid-mcp-token? + "Given access tokens list, return if it contains a valid (not expired) mcp key" + [{:keys [expires-at] :as token}] + (if token + (or (nil? expires-at) + (> expires-at (ct/now))) + false)) + (mf/defc mcp-server-section* {::mf/private true} - [] - (let [tokens (mf/deref refs/access-tokens) - profile (mf/deref refs/profile) - mcp-key-expired? (mf/deref refs/mcp-key-expired?) + [{:keys [access-tokens]}] + (let [profile (mf/deref refs/profile) - mcp-key (some #(when (= (:type %) "mcp") %) tokens) - mcp-token (:token mcp-key "") - mcp-url (dm/str cf/mcp-server-url "?userToken=" mcp-token) - mcp-enabled? (true? (-> profile :props :mcp-enabled)) + token (mf/with-memo [access-tokens] + (when-let [token (d/seek mcp-access-token? access-tokens)] + (assoc token :is-valid (valid-mcp-token? token)))) - show-enabled? (and mcp-enabled? (false? mcp-key-expired?)) + token-valid? (get token :is-valid) + token-str (get token :token) + enabled? (-> profile :props :mcp-enabled boolean) - tooltip-id - (mf/use-id) + url (dm/str cf/mcp-server-url "?userToken=" token-str) + + show-enabled? (and enabled? token-valid?) + tooltip-id (mf/use-id) handle-mcp-change (mf/use-fn @@ -437,19 +464,18 @@ (mbc/event :mcp/enable {}) (mbc/event :mcp/disable {}))))) - handle-generate-mcp-key + handle-open-modal-generate (mf/use-fn - #(st/emit! (modal/show {:type :generate-mcp-key}))) + #(st/emit! (modal/show {:type :generate-mcp-token}))) - handle-regenerate-mcp-key + handle-open-modal-regenerate (mf/use-fn - #(st/emit! (modal/show {:type :regenerate-mcp-key}))) + #(st/emit! (modal/show {:type :regenerate-mcp-token}))) handle-delete (mf/use-fn - (mf/deps mcp-key) - (fn [] - (let [params {:id (:id mcp-key)} + (fn [id] + (let [params {:id id} mdata {:on-success #(st/emit! (du/fetch-access-tokens))}] (st/emit! (du/delete-access-token (with-meta params mdata)) (du/update-profile-props {:mcp-enabled false}) @@ -457,10 +483,10 @@ on-copy-to-clipboard (mf/use-fn - (mf/deps mcp-url) + (mf/deps url) (fn [event] (dom/prevent-default event) - (clipboard/to-clipboard mcp-url) + (clipboard/to-clipboard url) (st/emit! (ntf/show {:level :info :type :toast :content (tr "integrations.notification.success.copied-link") @@ -492,7 +518,7 @@ (tr "integrations.mcp-server.status")] [:div {:class (stl/css :mcp-server-block)} - (when mcp-key-expired? + (when (and (some? token) (not token-valid?)) [:> notification-pill* {:level :error :type :context} [:div {:class (stl/css :mcp-server-notification)} @@ -512,14 +538,16 @@ (tr "integrations.mcp-server.status.disabled")) :default-checked show-enabled? :on-change handle-mcp-change}] - (when (and (false? mcp-enabled?) (nil? mcp-key)) - [:div {:class (stl/css :mcp-server-switch-cover) - :on-click handle-generate-mcp-key}]) - (when (true? mcp-key-expired?) - [:div {:class (stl/css :mcp-server-switch-cover) - :on-click handle-regenerate-mcp-key}])]]] - (when (some? mcp-key) + (when-not token + [:div {:class (stl/css :mcp-server-switch-cover) + :on-click handle-open-modal-generate}]) + + (when (and token (not token-valid?)) + [:div {:class (stl/css :mcp-server-switch-cover) + :on-click handle-open-modal-regenerate}])]]] + + (when (some? token) [:div {:class (stl/css :mcp-server-key)} [:> text* {:as "h3" :typography t/headline-small @@ -530,7 +558,7 @@ [:div {:class (stl/css :mcp-server-regenerate)} [:> button* {:variant "primary" :class (stl/css :fit-content) - :on-click handle-regenerate-mcp-key} + :on-click handle-open-modal-regenerate} (tr "integrations.mcp-server.mcp-keys.regenerate")] [:> tooltip* {:content (tr "integrations.mcp-server.mcp-keys.tootip") :id tooltip-id} @@ -538,21 +566,24 @@ :class (stl/css :color-secondary)}]]] [:div {:class (stl/css :list)} - [:> token-item* {:key (:id mcp-key) - :name (:name mcp-key) - :expires-at (:expires-at mcp-key) + [:> token-item* {:key (:id token) + :id (:id token) + :name (:name token) + :expires-at (:expires-at token) :on-delete handle-delete}]]]]) [:> notification-pill* {:level :default :type :context} [:div {:class (stl/css :mcp-server-notification)} - [:> text* {:as "div" - :typography t/body-medium - :class (stl/css :color-secondary)} - (tr "integrations.mcp-server.mcp-keys.info")] + (when token + [:> text* {:as "div" + :typography t/body-medium + :class (stl/css :color-secondary)} + (tr "integrations.mcp-server.mcp-keys.info")]) - [:> input-copy* {:value mcp-url - :on-copy-to-clipboard on-copy-to-clipboard}] + (when token + [:> input-copy* {:value url + :on-copy-to-clipboard on-copy-to-clipboard}]) [:> text* {:as "div" :typography t/body-medium @@ -561,12 +592,15 @@ :target "_blank" :rel "noopener noreferrer" :class (stl/css :mcp-server-notification-link)} - (tr "integrations.mcp-server.mcp-keys.help") [:> icon* {:icon-id i/open-link}]]]]]])) + (tr "integrations.mcp-server.mcp-keys.help") + [:> icon* {:icon-id i/open-link}]]]]]])) (mf/defc access-tokens-section* {::mf/private true} - [] - (let [tokens (mf/deref refs/access-tokens) + [{:keys [access-tokens]}] + (let [access-tokens + (mf/with-memo [access-tokens] + (not-empty (filter #(nil? (:type %)) access-tokens))) handle-click (mf/use-fn @@ -595,40 +629,49 @@ :on-click handle-click} (tr "integrations.access-tokens.create")] - (if (empty? tokens) + (if access-tokens + [:div {:class (stl/css :list)} + (for [{:keys [id] :as token} access-tokens] + [:> token-item* {:key (dm/str id) + :id id + :name (:name token) + :expires-at (:expires-at token) + :on-delete handle-delete}])] + [:div {:class (stl/css :frame)} [:> text* {:as "div" :typography t/body-medium :class (stl/css :color-secondary :text-center)} [:div (tr "integrations.access-tokens.empty.no-access-tokens")] - [:div (tr "integrations.access-tokens.empty.add-one")]]] - - [:div {:class (stl/css :list)} - (for [token tokens] - (when (nil? (:type token)) - [:> token-item* {:key (:id token) - :name (:name token) - :expires-at (:expires-at token) - :on-delete (partial handle-delete (:id token))}]))])])) + [:div (tr "integrations.access-tokens.empty.add-one")]]])])) (mf/defc integrations-page* [] - (mf/with-effect [] - (dom/set-html-title (tr "title.settings.integrations")) - (st/emit! (du/fetch-access-tokens))) + (let [access-tokens (mf/deref refs/access-tokens) + props (mf/props {:access-tokens access-tokens}) - [:div {:class (stl/css :integrations)} - [:> heading* {:level 1 - :typography t/title-large - :class (stl/css :color-primary)} - (tr "integrations.title")] + access-tokens-enabled? + (contains? cf/flags :access-tokens) - (when (contains? cf/flags :mcp) - [:> mcp-server-section*]) + mcp-enabled? + (contains? cf/flags :mcp)] - (when (and (contains? cf/flags :mcp) - (contains? cf/flags :access-tokens)) - [:hr {:class (stl/css :separator)}]) + (mf/with-effect [] + (dom/set-html-title (tr "title.settings.integrations")) + (st/emit! (du/fetch-access-tokens))) - (when (contains? cf/flags :access-tokens) - [:> access-tokens-section*])]) + [:div {:class (stl/css :integrations)} + [:> heading* {:level 1 + :typography t/title-large + :class (stl/css :color-primary)} + (tr "integrations.title")] + + (when ^boolean mcp-enabled? + [:> mcp-server-section* props]) + + (when (and ^boolean mcp-enabled? + ^boolean access-tokens-enabled?) + [:hr {:class (stl/css :separator)}]) + + (when ^boolean access-tokens-enabled? + [:> access-tokens-section* props])])) diff --git a/frontend/src/app/main/ui/workspace/main_menu.cljs b/frontend/src/app/main/ui/workspace/main_menu.cljs index e9511b3be6..8b87b01d5a 100644 --- a/frontend/src/app/main/ui/workspace/main_menu.cljs +++ b/frontend/src/app/main/ui/workspace/main_menu.cljs @@ -107,7 +107,7 @@ plugins? (features/active-feature? @st/state "plugins/runtime") - mcp? + mcp-enabled? (contains? cf/flags :mcp) show-shortcuts @@ -137,9 +137,9 @@ :on-close on-close :class (stl/css-case :base-menu true :sub-menu true - :pos-final-5 (not (or plugins? mcp?)) - :pos-final-6 (not= plugins? mcp?) - :pos-final-7 (and plugins? mcp?))} + :pos-final-5 (not (or plugins? mcp-enabled?)) + :pos-final-6 (not= plugins? mcp-enabled?) + :pos-final-7 (and plugins? mcp-enabled?))} [:> dropdown-menu-item* {:class (stl/css :base-menu-item :submenu-item) :on-click nav-to-helpc-center :on-key-down (fn [event] @@ -787,18 +787,15 @@ (mf/defc mcp-menu* {::mf/private true} - [{:keys [on-close]}] - (let [plugins? (features/active-feature? @st/state "plugins/runtime") + [{:keys [on-close mcp]}] + (let [plugins-enabled? (features/use-feature "plugins/runtime") + has-valid-token? (get mcp :token-valid) + enabled? (get mcp :enabled) - profile (mf/deref refs/profile) - mcp (mf/deref refs/mcp) - mcp-key-expired? (mf/deref refs/mcp-key-expired?) + conn-status (get mcp :connection-status) + connected? (= conn-status "connected") - mcp-enabled? (true? (-> profile :props :mcp-enabled)) - mcp-connection (get mcp :connection-status) - mcp-connected? (= mcp-connection "connected") - - show-enabled? (and mcp-enabled? (not mcp-key-expired?)) + show-enabled? (and enabled? has-valid-token?) on-nav-to-integrations (mf/use-fn @@ -815,8 +812,9 @@ on-toggle-mcp-plugin (mf/use-fn + (mf/deps connected?) (fn [] - (if mcp-connected? + (if connected? (st/emit! (mcp/user-disconnect-mcp) (ev/event {::ev/name "disconnect-mcp-plugin" ::ev/origin "workspace:menu"})) @@ -833,17 +831,18 @@ [:> dropdown-menu* {:show true :class (stl/css-case :base-menu true :sub-menu true - :pos-5 (not plugins?) - :pos-6 plugins?) + :pos-5 (not plugins-enabled?) + :pos-6 plugins-enabled?) :on-close on-close} - (when (and show-enabled? (not mcp-key-expired?)) + + (when (and show-enabled? has-valid-token?) [:> dropdown-menu-item* {:id "mcp-menu-toggle-mcp-plugin" :class (stl/css :base-menu-item :submenu-item) :on-click on-toggle-mcp-plugin :on-key-down on-toggle-mcp-plugin-key-down} [:span {:class (stl/css :item-name)} - (if mcp-connected? + (if connected? (tr "workspace.header.menu.mcp.plugin.status.disconnect") (tr "workspace.header.menu.mcp.plugin.status.connect"))]]) @@ -864,6 +863,7 @@ show-menu? (deref show-menu*) selected-sub-menu* (mf/use-state nil) selected-sub-menu (deref selected-sub-menu*) + mcp (mf/deref refs/mcp) toggle-menu (mf/use-fn @@ -1055,17 +1055,17 @@ :class (stl/css :item-arrow)}]]) (when (contains? cf/flags :mcp) - (let [mcp (mf/deref refs/mcp) - mcp-key-expired? (mf/deref refs/mcp-key-expired?) + (let [enabled? (get mcp :enabled) + conn-status (get mcp :connection-status) + has-valid-token? (get mcp :token-valid) - mcp-enabled? (true? (-> profile :props :mcp-enabled)) - mcp-connection (get mcp :connection-status) - mcp-connected? (= mcp-connection "connected") - mcp-error? (= mcp-connection "error") + connected? (= conn-status "connected") + error? (= conn-status "error") - active? (and mcp-enabled? mcp-connected?) - failed? (or (and mcp-enabled? mcp-error?) - (true? mcp-key-expired?))] + + active? (and enabled? connected?) + failed? (or (and enabled? error?) + (not has-valid-token?))] [:> dropdown-menu-item* {:class (stl/css :base-menu-item :menu-item) :on-click on-menu-click @@ -1140,7 +1140,7 @@ :on-close close-sub-menu}] :mcp - [:> mcp-menu* {:on-close close-sub-menu}] + [:> mcp-menu* {:on-close close-sub-menu :mcp mcp}] :help-info [:> help-info-menu* {:layout layout diff --git a/frontend/src/app/main/ui/workspace/top_toolbar.cljs b/frontend/src/app/main/ui/workspace/top_toolbar.cljs index eb9675beae..dcce4fecc6 100644 --- a/frontend/src/app/main/ui/workspace/top_toolbar.cljs +++ b/frontend/src/app/main/ui/workspace/top_toolbar.cljs @@ -31,26 +31,28 @@ (mf/defc mcp-indicator* [] - (let [profile (mf/deref refs/profile) - mcp (mf/deref refs/mcp) - mcp-key-expired? (mf/deref refs/mcp-key-expired?) + (let [mcp (mf/deref refs/mcp) - mcp-enabled? (true? (-> profile :props :mcp-enabled)) - mcp-connected? (= "connected" (:connection-status mcp)) - show-indicator? (and mcp-enabled? (false? mcp-key-expired?)) + conn-status (get mcp :connection-status) + has-valid-token? (get mcp :token-valid) - mcp-menu-open* (mf/use-state false) - mcp-menu-open? (deref mcp-menu-open*) + enabled? (get mcp :enabled) - toggle-mcp-menu + mcp-connected? (= "connected" conn-status) + show-indicator? (and enabled? has-valid-token?) + + menu-open* (mf/use-state false) + menu-open? (deref menu-open*) + + toggle-menu (mf/use-fn (fn [event] (dom/stop-propagation event) - (swap! mcp-menu-open* not))) + (swap! menu-open* not))) - close-mcp-menu + close-menu (mf/use-fn - #(reset! mcp-menu-open* false)) + #(reset! menu-open* false)) connect-mcp (mf/use-fn @@ -64,8 +66,8 @@ :aria-label (tr "workspace.toolbar.mcp") :class (stl/css-case :main-toolbar-options-button true :mcp-button true - :selected mcp-menu-open?) - :on-click toggle-mcp-menu + :selected menu-open?) + :on-click toggle-menu :data-tool "mcp" :data-testid "mcp-btn"} [:span {:class (stl/css-case :mcp-status-dot true @@ -73,8 +75,8 @@ [:span {:class (stl/css-case :mcp-button-label true :connected mcp-connected?)} (tr "workspace.toolbar.mcp")]] - [:> dropdown-menu* {:show mcp-menu-open? - :on-close close-mcp-menu + [:> dropdown-menu* {:show menu-open? + :on-close close-menu :class (stl/css :mcp-menu)} (if mcp-connected? [:li {:class (stl/css :mcp-menu-info) diff --git a/frontend/test/frontend_tests/data/workspace_mcp_test.cljs b/frontend/test/frontend_tests/data/workspace_mcp_test.cljs index e81dab6780..7f16894984 100644 --- a/frontend/test/frontend_tests/data/workspace_mcp_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_mcp_test.cljs @@ -6,31 +6,25 @@ (ns frontend-tests.data.workspace-mcp-test (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.main.data.profile :as du] [app.main.data.workspace.mcp :as mcp] [cljs.test :as t :include-macros true] [potok.v2.core :as ptk])) -(t/deftest test-set-mcp-active - (t/testing "sets :active to true" - (let [state {:mcp {:active false}} - result (ptk/update (mcp/set-mcp-active true) state)] - (t/is (true? (get-in result [:mcp :active]))))) - - (t/testing "sets :active to false" - (let [state {:mcp {:active true}} - result (ptk/update (mcp/set-mcp-active false) state)] - (t/is (false? (get-in result [:mcp :active])))))) - (t/deftest test-update-mcp-status - (t/testing "enables MCP in profile props" - (let [state {:profile {:props {:mcp-enabled false}}} + (t/testing "enables MCP in profile props and mcp state" + (let [state {:profile {:props {:mcp-enabled false}} :mcp {}} result (ptk/update (mcp/update-mcp-status true) state)] - (t/is (true? (get-in result [:profile :props :mcp-enabled]))))) + (t/is (true? (get-in result [:profile :props :mcp-enabled]))) + (t/is (true? (get-in result [:mcp :enabled]))))) - (t/testing "disables MCP in profile props" - (let [state {:profile {:props {:mcp-enabled true}}} + (t/testing "disables MCP in profile props and mcp state" + (let [state {:profile {:props {:mcp-enabled true}} :mcp {:enabled true}} result (ptk/update (mcp/update-mcp-status false) state)] - (t/is (false? (get-in result [:profile :props :mcp-enabled])))))) + (t/is (false? (get-in result [:profile :props :mcp-enabled]))) + (t/is (false? (get-in result [:mcp :enabled])))))) (t/deftest test-update-mcp-connection-status (t/testing "sets connection status to connected" @@ -43,8 +37,89 @@ result (ptk/update (mcp/update-mcp-connection-status "disconnected") state)] (t/is (= "disconnected" (get-in result [:mcp :connection-status])))))) -(t/deftest test-init-sets-active - (t/testing "init sets :mcp :active to true" - (let [state {:mcp {:active false}} +(t/deftest test-init-sets-enabled + (t/testing "init sets :mcp :enabled to true when profile has mcp-enabled" + (let [state {:mcp {} :profile {:props {:mcp-enabled true}}} result (ptk/update (mcp/init) state)] - (t/is (true? (get-in result [:mcp :active])))))) + (t/is (true? (get-in result [:mcp :enabled]))))) + + (t/testing "init sets :mcp :enabled to false when profile has mcp-disabled" + (let [state {:mcp {:enabled true} :profile {:props {:mcp-enabled false}}} + result (ptk/update (mcp/init) state)] + (t/is (false? (get-in result [:mcp :enabled]))))) + + (t/testing "init sets :mcp :enabled to false when profile has no mcp-enabled prop" + (let [state {:mcp {:enabled true} :profile {:props {}}} + result (ptk/update (mcp/init) state)] + (t/is (false? (get-in result [:mcp :enabled])))))) + +(t/deftest test-init-mcp-state + (let [token-id (uuid/next)] + (t/testing "with valid MCP token (future expiration)" + (let [future-date (ct/plus (ct/now) #js {:hours 24}) + tokens [{:id token-id :type "mcp" :token "abc123" :expires-at future-date}] + event (#'mcp/init-mcp-state tokens) + state {:mcp {}} + result (ptk/update event state)] + (t/is (= "abc123" (get-in result [:mcp :token]))) + (t/is (= token-id (get-in result [:mcp :token-id]))) + (t/is (true? (get-in result [:mcp :token-valid]))) + ;; deref should return the token when valid + (t/is (some? @event)))) + + (t/testing "with MCP token with no expiration" + (let [tokens [{:id token-id :type "mcp" :token "abc123" :expires-at nil}] + event (#'mcp/init-mcp-state tokens) + state {:mcp {}} + result (ptk/update event state)] + (t/is (= "abc123" (get-in result [:mcp :token]))) + (t/is (true? (get-in result [:mcp :token-valid]))) + (t/is (some? @event)))) + + (t/testing "with expired MCP token" + (let [past-date (ct/minus (ct/now) #js {:hours 24}) + tokens [{:id token-id :type "mcp" :token "abc123" :expires-at past-date}] + event (#'mcp/init-mcp-state tokens) + state {:mcp {}} + result (ptk/update event state)] + (t/is (= "abc123" (get-in result [:mcp :token]))) + (t/is (false? (get-in result [:mcp :token-valid]))) + ;; deref should return nil when token is expired + (t/is (nil? @event)))) + + (t/testing "with no MCP token" + (let [tokens [{:id token-id :type nil :token "regular-token"}] + event (#'mcp/init-mcp-state tokens) + state {:mcp {:existing "data"}} + result (ptk/update event state)] + ;; state should be unchanged when no MCP token exists + (t/is (= {:mcp {:existing "data"}} result)) + (t/is (nil? @event)))) + + (t/testing "with mixed tokens finds MCP token" + (let [regular-id (uuid/next) + mcp-id (uuid/next) + tokens [{:id regular-id :type nil :token "regular"} + {:id mcp-id :type "mcp" :token "mcp-token" :expires-at nil}] + event (#'mcp/init-mcp-state tokens) + result (ptk/update event {:mcp {}})] + (t/is (= "mcp-token" (get-in result [:mcp :token]))) + (t/is (= mcp-id (get-in result [:mcp :token-id]))))))) + +(t/deftest test-delete-access-token-optimistic-update + (let [token-1 {:id (uuid/next) :name "token-1"} + token-2 {:id (uuid/next) :name "token-2"} + token-3 {:id (uuid/next) :name "token-3"}] + + (t/testing "removes token from :access-tokens optimistically" + (let [state {:access-tokens [token-1 token-2 token-3]} + event (du/delete-access-token {:id (:id token-2)}) + result (ptk/update event state)] + (t/is (= 2 (count (:access-tokens result)))) + (t/is (= [token-1 token-3] (:access-tokens result))))) + + (t/testing "state unchanged when token id not found" + (let [state {:access-tokens [token-1 token-2]} + event (du/delete-access-token {:id (uuid/next)}) + result (ptk/update event state)] + (t/is (= 2 (count (:access-tokens result))))))))