diff --git a/CHANGELOG.md b/CHANGELOG.md index cf0862353..ad035cd1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- MCP: detect a broken/logged-out session (expired/revoked token, expired session) and re-initialize instead of hanging the tool call until timeout; relies on plumcp 0.2.2 correlating HTTP error responses to the originating request. +- MCP OAuth: proactively refresh an expired token before a tool call instead of waiting for the server to reject it. + ## 0.142.2 - `/context` now shows where auto-compaction triggers: a `🔲` marker on the threshold cell of the grid plus an `Auto-compaction at N%` line. diff --git a/deps-lock.json b/deps-lock.json index c8109e2a8..707c08329 100644 --- a/deps-lock.json +++ b/deps-lock.json @@ -1063,24 +1063,24 @@ "hash": "sha256-RLTLjpPU9rJiwE7Qdx1w3WbnbUXX/HVYIGcaYmVcVDk=" }, { - "mvn-path": "io/github/plumce/plumcp.core-json-cheshire/0.2.0-beta6/plumcp.core-json-cheshire-0.2.0-beta6.jar", + "mvn-path": "io/github/plumce/plumcp.core-json-cheshire/0.2.2/plumcp.core-json-cheshire-0.2.2.jar", "mvn-repo": "https://repo.clojars.org/", - "hash": "sha256-3HJeEyYWHhRYLVOg6NbqUNY5ggjlbZP8HLkWhxcrxH0=" + "hash": "sha256-KuvTGmGhr1XKHZtxgrIKqFs1R3YSGCbjvXQllZTiR7g=" }, { - "mvn-path": "io/github/plumce/plumcp.core-json-cheshire/0.2.0-beta6/plumcp.core-json-cheshire-0.2.0-beta6.pom", + "mvn-path": "io/github/plumce/plumcp.core-json-cheshire/0.2.2/plumcp.core-json-cheshire-0.2.2.pom", "mvn-repo": "https://repo.clojars.org/", - "hash": "sha256-KDt9ErjS5xemxNULIZ0jW9VMty/QqIptUqEo35S+9EI=" + "hash": "sha256-5ECb6C6r7dEUpay8xw6ykojq2Cc0Kj1sS57Xuu7yA0Y=" }, { - "mvn-path": "io/github/plumce/plumcp.core/0.2.0-beta6/plumcp.core-0.2.0-beta6.jar", + "mvn-path": "io/github/plumce/plumcp.core/0.2.2/plumcp.core-0.2.2.jar", "mvn-repo": "https://repo.clojars.org/", - "hash": "sha256-su36hUT/Y6qK58rSdbjldLmwyNX+MwTnGxIvXNOU5C8=" + "hash": "sha256-XvWt0TS0ZmSxWNVNK5IHJ+5OQ6Ya2ZZKUbKkdn0s5WE=" }, { - "mvn-path": "io/github/plumce/plumcp.core/0.2.0-beta6/plumcp.core-0.2.0-beta6.pom", + "mvn-path": "io/github/plumce/plumcp.core/0.2.2/plumcp.core-0.2.2.pom", "mvn-repo": "https://repo.clojars.org/", - "hash": "sha256-wSQlx/kT5oAkiUB5t1XhrwD4w8K8TSYKMVzSFHmnWvk=" + "hash": "sha256-L8GyN7xLfJS7d5sN7dquXqtm/mBr/pJPvlkBq2/i0Wo=" }, { "mvn-path": "io/methvin/directory-watcher/0.17.3/directory-watcher-0.17.3.jar", diff --git a/deps.edn b/deps.edn index a3033fc9f..96adcbd65 100644 --- a/deps.edn +++ b/deps.edn @@ -3,7 +3,7 @@ org.clojure/core.async {:mvn/version "1.8.741"} org.babashka/cli {:mvn/version "0.8.65"} com.github.clojure-lsp/jsonrpc4clj {:mvn/version "1.0.2"} - io.github.plumce/plumcp.core-json-cheshire {:mvn/version "0.2.0-beta6"} + io.github.plumce/plumcp.core-json-cheshire {:mvn/version "0.2.2"} org.yaml/snakeyaml {:mvn/version "2.4"} ;; used by eca.shared for YAML parsing borkdude/dynaload {:mvn/version "0.3.5"} dev.ericdallo/rewrite-json {:mvn/version "0.1.1"} diff --git a/src/eca/features/tools/mcp.clj b/src/eca/features/tools/mcp.clj index 376e8db69..917dd47d7 100644 --- a/src/eca/features/tools/mcp.clj +++ b/src/eca/features/tools/mcp.clj @@ -1073,13 +1073,32 @@ (do-call-tool new-client name arguments nil) (tool-call-error (format "Failed to re-initialize MCP server '%s'" server-name)))) +(defn ^:private refresh-expired-auth! + "Refresh a locally-expired OAuth token before a tool call so we don't send a + doomed request. A successful refresh updates db* (the request middleware + picks it up). Returns true when a full reinit is needed instead (no/failed + refresh)." + [server-name db* config metrics] + (let [db @db* + server-config (get-in config [:mcpServers server-name]) + url (:url server-config) + mcp-auth (get-in db [:mcp-auth (mcp-auth-key server-name server-config db)])] + (when (and url + (:access-token mcp-auth) + (token-expired? (:expires-at mcp-auth))) + (logger/info logger-tag + (format "MCP server '%s' OAuth token expired, refreshing before tool call" server-name)) + (not (try-refresh-token! server-name db* url metrics server-config))))) + (defn call-tool! [server-name name arguments {:keys [db db* config metrics]}] (if-let [[mcp-client needs-reinit?*] (when-let [{:keys [client tools needs-reinit?*]} (get-in db [:mcp-clients server-name])] (when (some #(= name (:name %)) tools) [client needs-reinit?*]))] - (if (and needs-reinit?* @needs-reinit?* db* config metrics) - ;; Already flagged — reinit before attempting the call + (if (and needs-reinit?* db* config metrics + (or @needs-reinit?* + (refresh-expired-auth! server-name db* config metrics))) + ;; Flagged, or expired token couldn't be refreshed — reinit before the call (reinit-and-call-tool! server-name mcp-client db* config metrics name arguments) (let [result (do-call-tool mcp-client name arguments needs-reinit?*)] (cond diff --git a/test/eca/features/tools/mcp_test.clj b/test/eca/features/tools/mcp_test.clj index 888dc6995..31ea36b3c 100644 --- a/test/eca/features/tools/mcp_test.clj +++ b/test/eca/features/tools/mcp_test.clj @@ -669,3 +669,51 @@ nil))) (is (= [[:refresh "s" "https://example.com/mcp"] [:init "s"]] @calls*) "refresh runs before init, scoped to the server's url")))) + +(defn ^:private call-tool-db* [expires-at] + (atom {:mcp-clients {"s" {:client :client-s + :tools [{:name "tool"}] + :needs-reinit?* (atom false)}} + :mcp-auth {"s" {:access-token "old" :expires-at expires-at}}})) + +(def ^:private call-tool-ctx-config {:mcpServers {"s" {:url "https://example.com/mcp"}}}) + +(deftest call-tool!-refreshes-expired-token-test + (testing "expired token is refreshed in place, then the call runs on the existing client (no reinit)" + (let [db* (call-tool-db* 0) + calls* (atom [])] + (with-redefs [mcp/try-refresh-token! (fn [name _db* url _metrics _config] + (swap! calls* conj [:refresh name url]) + true) + mcp/reinitialize-server! (fn [& _] (swap! calls* conj [:reinit])) + mcp/do-call-tool (fn [client _name _args _nr] + (swap! calls* conj [:call client]) + {:error false :contents []})] + (is (= {:error false :contents []} + (mcp/call-tool! "s" "tool" {} {:db @db* :db* db* + :config call-tool-ctx-config :metrics {}}))) + (is (= [[:refresh "s" "https://example.com/mcp"] [:call :client-s]] @calls*))))) + + (testing "expired token whose refresh fails routes to a full reinit" + (let [db* (call-tool-db* 0) + calls* (atom [])] + (with-redefs [mcp/try-refresh-token! (fn [_name _db* _url _metrics _config] nil) + mcp/reinitialize-server! (fn [name _client _db* _config _metrics] + (swap! calls* conj [:reinit name])) + mcp/do-call-tool (fn [client _name _args _nr] + (swap! calls* conj [:call client]) + {:error false :contents []})] + (mcp/call-tool! "s" "tool" {} {:db @db* :db* db* + :config call-tool-ctx-config :metrics {}}) + (is (= [:reinit "s"] (first @calls*)) "refresh failure reinitializes before calling")))) + + (testing "fresh token: no proactive refresh, calls directly" + (let [db* (call-tool-db* (+ (quot (System/currentTimeMillis) 1000) 3600)) + calls* (atom [])] + (with-redefs [mcp/try-refresh-token! (fn [& _] (swap! calls* conj [:refresh]) true) + mcp/do-call-tool (fn [client _name _args _nr] + (swap! calls* conj [:call client]) + {:error false :contents []})] + (mcp/call-tool! "s" "tool" {} {:db @db* :db* db* + :config call-tool-ctx-config :metrics {}}) + (is (= [[:call :client-s]] @calls*) "no refresh when token is not expired")))))