From 1d74c56179ff88080b54146bc5a80bb1cf1eb38e Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Fri, 2 Oct 2026 17:09:58 +0200 Subject: [PATCH 1/2] Add safe on-demand model catalog refresh Refresh provider catalogs without restarting the server or changing chat selections. Serialize sync turns, preserve usable models on fetch failure, and report accurate fallback warnings. --- CHANGELOG.md | 2 + docs/protocol.md | 24 ++ .../integration/initialize_test.clj | 25 ++ integration-test/llm_mock/ollama.clj | 6 +- src/eca/features/login.clj | 18 +- src/eca/features/providers.clj | 15 +- src/eca/handlers.clj | 37 +- src/eca/llm_providers/ollama.clj | 12 +- src/eca/models.clj | 107 +++++- src/eca/server.clj | 5 + test/eca/handlers_test.clj | 198 ++++++++++ test/eca/llm_providers/ollama_test.clj | 24 +- test/eca/models_test.clj | 337 ++++++++++++++++++ test/eca/server_test.clj | 21 +- 14 files changed, 796 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84e67e269..b263a96b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add `models/refresh` to update the model catalog without restarting; report provider fallback and stale-model warnings while preserving chat selections. + ## 0.163.0 - Add subagent `timeoutSeconds`; on timeout or max steps the subagent gets a final no-tools turn to report findings, and a parent stop keeps its partial output. #625 diff --git a/docs/protocol.md b/docs/protocol.md index 42894a7b8..66e2a0b9b 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -3504,6 +3504,30 @@ interface MCPRemoveServerResult { } ``` +## Model Catalog + +### Refresh Models (↩️) + +Request a fresh model catalog without changing provider configuration or chat selections. The request completes after reconciliation. It does not select a model. `providers/list` remains a read-only snapshot. + +_Request:_ + +* method: `models/refresh` +* params: `{}` + +_Response:_ + +```typescript +interface ModelsRefreshResult { + modelCount: number; + warnings: { provider: string; message: string }[]; +} +``` + +A successful response can contain warnings. Discovery uses the native provider endpoint first, then `models.dev`. If both sources fail, the server retains that provider's last known in-memory models and reports them as stale. A disk-cache fallback produces one `models.dev` warning because cached metadata or fallback lists can be stale. An Ollama fetch failure retains its last known models when Ollama remains configured and reports stale data. A valid empty Ollama catalog removes those models. Failed near-expiry token renewal produces a provider warning. Static configured models and available Ollama models remain eligible. If no usable models remain, the request returns a JSON-RPC error with code `no_usable_model_catalog`. The server keeps the current model list in that case. + +After successful reconciliation, the server sends `config/updated` with `chat.models` when that list changes. The response does not carry the list. The refresh notification does not contain `selectModel`, `selectAgent`, or `selectVariant`. Existing chat selections stay unchanged, even if a selected model leaves the catalog. + ## Provider Management ### List Providers (↩️) diff --git a/integration-test/integration/initialize_test.clj b/integration-test/integration/initialize_test.clj index 459bea4ad..66b97ff7e 100644 --- a/integration-test/integration/initialize_test.clj +++ b/integration-test/integration/initialize_test.clj @@ -4,6 +4,7 @@ [clojure.test :refer [deftest is testing]] [integration.eca :as eca] [integration.fixture :as fixture] + [llm-mock.mocks :as llm.mocks] [matcher-combinators.matchers :as m] [matcher-combinators.test :refer [match?]])) @@ -96,3 +97,27 @@ :selectAgent "code" :welcomeMessage (m/pred #(string/includes? % "Welcome to ECA!"))}} (eca/client-awaits-server-notification :config/updated)))))) + +(deftest refresh-models-in-running-server + (eca/start-process!) + (eca/request! (fixture/initialize-request + {:initializationOptions + (assoc fixture/default-init-options :providers + {"ollama" {:url (str fixture/base-llm-mock-url "/ollama")}})})) + (eca/notify! (fixture/initialized-notification)) + (is (match? {:chat {:models ["ollama/qwen3"]}} + (eca/client-awaits-server-notification :config/updated))) + + (testing "a refresh returns its result and publishes a changed catalog" + (llm.mocks/set-case! :refresh-new-model) + (is (match? {:modelCount (m/pred number?) + :warnings (m/pred vector?)} + (eca/request! [:models/refresh {}]))) + (is (match? {:chat {:models ["ollama/qwen4"]}} + (eca/client-awaits-server-notification :config/updated)))) + + (testing "an empty catalog returns a JSON-RPC error, not a success result" + (llm.mocks/set-case! :refresh-empty) + (let [response (eca/request! [:models/refresh {}])] + (is (match? {:error {:code "no_usable_model_catalog"}} response)) + (is (not (contains? response :result)))))) diff --git a/integration-test/llm_mock/ollama.clj b/integration-test/llm_mock/ollama.clj index b2c32e74e..51eaef2c9 100644 --- a/integration-test/llm_mock/ollama.clj +++ b/integration-test/llm_mock/ollama.clj @@ -95,7 +95,11 @@ (defn handle-ollama-tags [_req] {:status 200 :headers {"Content-Type" "application/json"} - :body (json/generate-string {:models [{:model "qwen3"}]})}) + :body (json/generate-string + {:models (case llm.mocks/*case* + :refresh-new-model [{:model "qwen4"}] + :refresh-empty [] + [{:model "qwen3"}])})}) (defn handle-ollama-show [req] (let [_body (some-> (slurp (:body req)) (json/parse-string true))] diff --git a/src/eca/features/login.clj b/src/eca/features/login.clj index dfee134bd..0002fda81 100644 --- a/src/eca/features/login.clj +++ b/src/eca/features/login.clj @@ -206,13 +206,14 @@ otherwise call provider endpoints with a stale short-lived token (e.g. Copilot's session token) and get a 401. `ctx` needs {:db* :messenger :config :metrics}." - [{:keys [db*] :as ctx}] + [{:keys [db* on-renew-error] :as ctx}] (doseq [provider (keys (:auth @db*))] (maybe-renew-auth-token! {:provider provider :on-error (fn [msg] (logger/warn logger-tag - (format "Could not renew '%s' auth token before sync: %s" provider msg)))} + (format "Could not renew '%s' auth token before sync: %s" provider msg)) + (when on-renew-error (on-renew-error provider msg)))} ctx))) (defn login-done! [{:keys [chat-id db* messenger metrics provider send-msg!]} @@ -223,12 +224,15 @@ (db/update-global-auth-cache! @db* provider metrics)) (when-not skip-models-sync? (models/sync-models! db* - (config/all @db*) ;; force get updated config + #(config/all @db*) (fn [new-models] - (messenger/config-updated - messenger - {:chat - {:models (sort (keys new-models))}})))) + (config/notify-fields-changed-only! + {:chat {:models (sort (keys new-models))}} + messenger db*)) + {:before-sync (fn [config _warn!] + (renew-expiring-auth-tokens! + {:db* db* :messenger messenger + :config config :metrics metrics}))})) (swap! db* assoc-in [:chats chat-id :login-provider] nil) (swap! db* assoc-in [:chats chat-id :status] :idle) (when-not silent? diff --git a/src/eca/features/providers.clj b/src/eca/features/providers.clj index 95a19128b..74a3baec4 100644 --- a/src/eca/features/providers.clj +++ b/src/eca/features/providers.clj @@ -213,11 +213,18 @@ and send both config/updated and providers/updated notifications." [provider-name db* messenger metrics] (db/update-global-auth-cache! @db* provider-name metrics) - (let [config (config/all @db*)] - (models/sync-models! db* config + (let [sync-config* (volatile! nil)] + (models/sync-models! db* #(config/all @db*) (fn [new-models] - (messenger/config-updated messenger {:chat {:models (sort (keys new-models))}}) - (notify-provider-updated! provider-name db* config messenger))))) + (config/notify-fields-changed-only! + {:chat {:models (sort (keys new-models))}} messenger db*) + (notify-provider-updated! provider-name db* @sync-config* messenger)) + {:before-sync (fn [config _warn!] + (vreset! sync-config* config) + ;; Login already depends on providers, so resolve renewal at call time. + ((requiring-resolve 'eca.features.login/renew-expiring-auth-tokens!) + {:db* db* :messenger messenger + :config config :metrics metrics}))}))) ;; --- Login dispatch --- diff --git a/src/eca/handlers.clj b/src/eca/handlers.clj index 0084b04ed..a494faa2b 100644 --- a/src/eca/handlers.clj +++ b/src/eca/handlers.clj @@ -120,15 +120,15 @@ (let [new-providers-hash (hash (:providers config))] (when (not= (:providers-config-hash @db*) new-providers-hash) (swap! db* assoc :providers-config-hash new-providers-hash) - ;; Refresh near-expiry OAuth tokens before fetching catalogs, so - ;; this non-interactive path doesn't hit /models with a stale - ;; short-lived token (e.g. Copilot) and 401. - (f.login/renew-expiring-auth-tokens! - {:db* db* :messenger messenger :config config :metrics metrics}) ;; The callback re-reads config, so plugin agents are included ;; when plugin resolution finishes before model sync. - (models/sync-models! db* config (fn [_models] - (notify-chat-config-state! db* messenger))))))] + (models/sync-models! db* #(config/all @db*) + (fn [_models] + (notify-chat-config-state! db* messenger)) + {:before-sync (fn [config _warn!] + (f.login/renew-expiring-auth-tokens! + {:db* db* :messenger messenger + :config config :metrics metrics}))}))))] (swap! db* assoc-in [:config-updated-fns :sync-models] (fn [_prev-config new-config] (sync-models-and-notify! new-config))) (swap! db* assoc-in [:config-updated-fns :mcp-reconcile] @@ -613,6 +613,29 @@ :status nil :exit-code nil}))) +(defn models-refresh [{:keys [db* messenger metrics model-sync-turn]} _params] + (metrics/task metrics :eca/models-refresh + (try + (let [{:keys [model-count warnings]} + (models/sync-models! + db* #(config/all @db*) + (fn [new-models] + (config/notify-fields-changed-only! + {:chat {:models (sort (keys new-models))}} messenger db*)) + {:refresh? true + :turn model-sync-turn + :before-sync (fn [config warn!] + (f.login/renew-expiring-auth-tokens! + {:db* db* :messenger messenger :config config + :metrics metrics :on-renew-error warn!}))})] + {:modelCount model-count :warnings warnings}) + (catch clojure.lang.ExceptionInfo e + (if (= :no-usable-model-catalog (:type (ex-data e))) + {:error {:code "no_usable_model_catalog" :message (ex-message e)}} + (throw e))) + (finally + (when model-sync-turn (deliver (:done model-sync-turn) true)))))) + (defn providers-list [{:keys [db* config metrics]} _params] (metrics/task metrics :eca/providers-list (f.providers/providers-list db* config))) diff --git a/src/eca/llm_providers/ollama.clj b/src/eca/llm_providers/ollama.clj index 58e3889cd..b8ae6f1fc 100644 --- a/src/eca/llm_providers/ollama.clj +++ b/src/eca/llm_providers/ollama.clj @@ -2,6 +2,7 @@ (:require [cheshire.core :as json] [clojure.java.io :as io] + [clojure.string :as string] [eca.client-http :as client] [eca.llm-util :as llm-util] [eca.logger :as logger] @@ -28,12 +29,17 @@ (if (= 200 status) (do (llm-util/log-response logger-tag rid "api/tags" body) - (:models body)) + (let [models (:models body)] + (when (and (vector? models) + (every? #(and (map? %) + (string? (:model %)) + (not (string/blank? (:model %)))) models)) + models))) (do (logger/warn logger-tag "Unknown status code:" status) - []))) + nil))) (catch Exception _ - []))) + nil))) (defn model-capabilities [{:keys [model api-url]}] (try diff --git a/src/eca/models.clj b/src/eca/models.clj index dbf6b9451..26130daa7 100644 --- a/src/eca/models.clj +++ b/src/eca/models.clj @@ -17,6 +17,23 @@ (def ^:private logger-tag "[MODELS]") +;; Every caller uses the same queue, including login and startup syncs. +(def ^:private sync-tail* (atom (promise))) +(deliver @sync-tail* true) +(defn reserve-sync! + "Reserve a catalog sync turn before scheduling async work, preserving request order." + [] + (let [done (promise)] + {:previous (first (swap-vals! sync-tail* (constantly done))) + :done done})) + +(def ^:dynamic *refresh-warnings* nil) +(def ^:dynamic *cached-models-dev?* nil) + +(defn ^:private warn-refresh! [provider message] + (when *refresh-warnings* + (swap! *refresh-warnings* conj {:provider provider :message message}))) + (def ^:private models-dev-api-url "https://models.dev/api.json") (def ^:private models-dev-timeout-ms 5000) (def ^:private provider-models-timeout-ms 10000) @@ -129,6 +146,7 @@ fetched) (if-let [cached (read-models-dev-cache)] (do + (when *cached-models-dev?* (reset! *cached-models-dev?* true)) (logger/info logger-tag " Using cached models.dev catalog fallback") cached) {})))) @@ -765,10 +783,26 @@ (if (provider-configured? provider provider-config db config) [provider (future - (or (fetch-provider-native-models-with-fallback - provider provider-config config db) - (fetch-single-provider-models-dev - provider provider-config config models-dev-index)))] + (let [native? (contains? native-models-endpoint-providers (:api provider-config)) + native (when native? + (try + (fetch-provider-native-models-with-fallback + provider provider-config config db) + (catch Exception e + (logger/warn logger-tag + (format "Provider '%s': Model discovery failed: %s" + provider (.getMessage e))) + nil))) + fallback (when (nil? native) + (fetch-single-provider-models-dev + provider provider-config config models-dev-index))] + (when (and native? (nil? native)) + (warn-refresh! provider (if (some? fallback) + "Native catalog failed; using models.dev fallback" + "Native catalog failed"))) + (when (and (not native?) (nil? fallback)) + (warn-refresh! provider "No usable remote catalog available")) + (or native fallback)))] (do (logger/debug logger-tag (format "Provider '%s': Skipping model fetch (not configured)" @@ -836,8 +870,10 @@ (and alias-model (contains? models alias-model)) alias-model (contains? models model-id) model-id)))) -(defn sync-models! [db* config on-models-updated] +(defn ^:private sync-models-now! [db* config on-models-updated refresh?] (let [models-dev-data (models-dev) + _ (when (and *cached-models-dev?* @*cached-models-dev?*) + (warn-refresh! "models.dev" "Remote catalog unavailable; cached metadata or fallback lists can be stale")) known-models (all models-dev-data) db @db* {:keys [models]} (fetch-provider-model-catalogs config db models-dev-data) @@ -846,13 +882,15 @@ config models) ollama-api-url (llm-util/provider-api-url "ollama" config) + ollama-catalog (when (or (not refresh?) (contains? (:providers config) "ollama")) + (llm-providers.ollama/list-models {:api-url ollama-api-url})) ollama-models (mapv (fn [{:keys [model] :as ollama-model}] (let [capabilities (llm-providers.ollama/model-capabilities {:api-url ollama-api-url :model model})] (assoc ollama-model :tools (boolean (some #(= % "tools") capabilities)) :reason? (boolean (some #(= % "thinking") capabilities))))) - (llm-providers.ollama/list-models {:api-url ollama-api-url})) + ollama-catalog) ollama-models-config (get-in config [:providers "ollama" :models]) local-models (reduce (fn [models {:keys [model] :as ollama-model}] @@ -865,9 +903,60 @@ ollama-models) authenticated-models (into {} (filter #(auth-valid? (first %) db config) all-supported-models)) - all-models (merge authenticated-models local-models)] - (swap! db* assoc :models all-models) - (on-models-updated all-models))) + retained-models (when refresh? + (into {} + (filter (fn [[full-model _]] + (let [[provider _] (shared/full-model->provider+model full-model) + provider-config (get-in config [:providers provider])] + (and provider-config + (fetch-model-catalog-enabled? provider-config) + (provider-configured? provider provider-config db config) + (not (contains? models provider)) + (auth-valid? full-model db config))))) + (:models db))) + retained-ollama-models (when (and refresh? (contains? (:providers config) "ollama") + (nil? ollama-catalog)) + (into {} + (filter (fn [[full-model _]] + (= "ollama" (first (shared/full-model->provider+model full-model))))) + (:models db))) + all-models (merge retained-models retained-ollama-models authenticated-models local-models)] + (when refresh? + (doseq [provider (distinct (map (comp first shared/full-model->provider+model first) retained-models))] + (warn-refresh! provider "No usable remote catalog; retained last known in-memory models (stale)")) + (when (and (contains? (:providers config) "ollama") (nil? ollama-catalog)) + (warn-refresh! "ollama" (if (seq retained-ollama-models) + "Ollama catalog fetch failed; retained last known in-memory models (stale)" + "Ollama catalog fetch failed")))) + (if (and refresh? (empty? all-models)) + (throw (ex-info "No usable model catalog available; current models unchanged" {:type :no-usable-model-catalog})) + (do + (swap! db* assoc :models all-models) + (let [callback-result (on-models-updated all-models)] + (if refresh? + {:model-count (count all-models)} + callback-result)))))) + +(defn sync-models! + "Accept a config map or a zero-argument config function. Resolve it inside the + reserved turn, then call before-sync with the resolved config and warn function." + ([db* config on-models-updated] + (sync-models! db* config on-models-updated {})) + ([db* config on-models-updated {:keys [refresh? before-sync turn]}] + (let [{:keys [previous done]} (or turn (reserve-sync!)) + warnings* (when refresh? (atom []))] + (try + @previous + (binding [*refresh-warnings* warnings* + *cached-models-dev?* (when refresh? (atom false))] + (let [config (if (fn? config) (config) config)] + (when before-sync + (before-sync config (fn [provider message] (warn-refresh! provider message)))) + (let [result (sync-models-now! db* config on-models-updated refresh?)] + (when refresh? + (assoc result :warnings @warnings*))))) + (finally + (deliver done true)))))) (comment (require '[clojure.pprint :as pprint]) diff --git a/src/eca/server.clj b/src/eca/server.clj index 1c861cc64..27340f969 100644 --- a/src/eca/server.clj +++ b/src/eca/server.clj @@ -8,6 +8,7 @@ [eca.logger :as logger] [eca.messenger :as messenger] [eca.metrics :as metrics] + [eca.models :as models] [eca.nrepl :as nrepl] [eca.opentelemetry :as opentelemetry] [eca.remote.messenger :as remote.messenger] @@ -240,6 +241,10 @@ (defmethod jsonrpc.server/receive-request "jobs/readOutput" [_ components params] (eventually (handlers/jobs-read-output (with-config components) params))) +(defmethod jsonrpc.server/receive-request "models/refresh" [_ components params] + (let [turn (models/reserve-sync!)] + (eventually (handlers/models-refresh (assoc components :model-sync-turn turn) params)))) + (defmethod jsonrpc.server/receive-request "providers/list" [_ components params] (eventually (handlers/providers-list (with-config components) params))) diff --git a/test/eca/handlers_test.clj b/test/eca/handlers_test.clj index 9a4a188d4..bb0a33867 100644 --- a/test/eca/handlers_test.clj +++ b/test/eca/handlers_test.clj @@ -1,10 +1,15 @@ (ns eca.handlers-test (:require [clojure.test :refer [deftest is testing]] + [hato.client :as http] [eca.config :as config] [eca.db :as db] + [eca.features.hooks :as f.hooks] + [eca.features.login :as f.login] + [eca.features.providers :as f.providers] [eca.features.tools :as f.tools] [eca.handlers :as handlers] + [eca.llm-providers.ollama :as ollama] [eca.models :as models] [eca.test-helper :as h] [matcher-combinators.matchers :as m] @@ -12,6 +17,199 @@ (h/reset-components-before-test) +(deftest models-refresh-test + (h/reset-components!) + (swap! (h/db*) assoc :models {"openai/old" {:tools true}} + :chats {"c1" {:model "openai/old"}}) + (with-redefs [models/sync-models! (fn [db* _config callback _opts] + (swap! db* assoc :models {"openai/new" {:tools true}}) + (callback (:models @db*)) + {:model-count 1 :warnings []})] + (is (= {:modelCount 1 :warnings []} + (handlers/models-refresh (h/components) {}))) + (is (= "openai/old" (get-in (h/db) [:chats "c1" :model]))) + (is (= [{:chat {:models ["openai/new"]}}] + (:config-updated (h/messages)))))) + +(deftest login-and-provider-model-notifications-keep-refresh-mirror-current-test + (doseq [[path initial changed action] + [["chat login" ["openai/a"] ["openai/a" "openai/b"] + (fn [] (f.login/login-done! {:chat-id "c1" :db* (h/db*) + :messenger (h/messenger) :metrics (h/metrics) + :provider "openai" :send-msg! (fn [_])} + :silent? true))] + ["Providers login" ["openai/a"] ["openai/a" "openai/b"] + (fn [] (f.providers/provider-login-input "openai" {:api-key "test"} + (h/db*) (h/config) (h/messenger) (h/metrics)))] + ["Providers logout" ["openai/a" "openai/b"] ["openai/a"] + (fn [] (f.providers/provider-logout "openai" (h/db*) (h/config) + (h/messenger) (h/metrics)))]]] + (testing path + (h/reset-components!) + (let [next-models* (atom changed)] + (config/notify-fields-changed-only! {:chat {:models initial}} (h/messenger) (h/db*)) + (with-redefs [db/update-global-cache! (fn [& _]) + models/sync-models! (fn [db* _config callback & _] + (let [new-models (zipmap @next-models* (repeat {}))] + (swap! db* assoc :models new-models) + (callback new-models) + {:model-count (count new-models) :warnings []}))] + (action) + (is (= [{:chat {:models initial}} {:chat {:models changed}}] + (:config-updated (h/messages)))) + (when (not= path "chat login") + (is (= 1 (count (:provider-updated (h/messages)))))) + (action) + (is (= 2 (count (:config-updated (h/messages)))) + "an unchanged login/logout catalog must not send another model list") + (when (not= path "chat login") + (is (= 2 (count (:provider-updated (h/messages)))) + "provider status must still be sent when the models are unchanged")) + (reset! next-models* initial) + (handlers/models-refresh (h/components) {}) + (is (= [{:chat {:models initial}} + {:chat {:models changed}} + {:chat {:models initial}}] + (:config-updated (h/messages)))) + (handlers/models-refresh (h/components) {}) + (is (= 3 (count (:config-updated (h/messages))))) + (when (not= path "chat login") + (is (= 2 (count (:provider-updated (h/messages))))))))))) + +(deftest models-refresh-uses-config-at-turn-test + (h/reset-components!) + (let [current* (atom {:providers {"old" {:api "openai-chat" :url "https://old.test" :key "key"}}}) + entered (promise) + release (promise) + renewed* (atom []) + fetched* (atom []) + first-turn (models/reserve-sync!) + second-turn (models/reserve-sync!)] + (with-redefs [config/all (fn [_] @current*) + models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + f.login/renew-expiring-auth-tokens! (fn [{:keys [config]}] + (swap! renewed* conj config)) + http/get (fn [url _] + (swap! fetched* conj url) + {:status 200 :body {:data [{:id "found"}]}})] + (let [first-sync (future (models/sync-models! (h/db*) {:providers {}} + (fn [_] (deliver entered true) @release) + {:turn first-turn}))] + (try + (is (= true (deref entered 5000 false))) + (let [refresh (future (handlers/models-refresh + (assoc (h/components) :model-sync-turn second-turn) {}))] + (reset! current* {:providers {"new" {:api "openai-chat" :url "https://new.test" :key "key"}}}) + (is (empty? @renewed*)) + (deliver release true) + (is (not= :timeout (deref first-sync 5000 :timeout))) + (is (= {:modelCount 1 :warnings []} (deref refresh 5000 :timeout))) + (is (= [@current*] @renewed*)) + (is (= ["https://new.test/models"] @fetched*))) + (finally (deliver release true))))))) + +(deftest login-syncs-read-config-inside-turn-test + (doseq [[path sync!] + [["chat login" (fn [] (f.login/login-done! + {:chat-id "c1" :db* (h/db*) :messenger (h/messenger) + :metrics (h/metrics) :provider "new" :send-msg! (fn [_])} + :silent? true))] + ["provider login/logout" (fn [] (f.providers/sync-and-notify! + "new" (h/db*) (h/messenger) (h/metrics)))]]] + (testing path + (h/reset-components!) + (let [current* (atom {:providers {}}) + entered (promise) + release (promise) + fetched* (atom []) + renewed* (atom []) + blocker (models/reserve-sync!)] + (with-redefs [config/all (fn [_] @current*) + db/update-global-cache! (fn [& _]) + models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + f.login/renew-expiring-auth-tokens! (fn [{:keys [config]}] + (swap! renewed* conj config)) + http/get (fn [url _] + (swap! fetched* conj url) + {:status 200 :body {:data [{:id "found"}]}})] + (let [work (future (deliver entered true) (sync!))] + (try + (is (= true (deref entered 5000 false))) + (reset! current* {:providers {"new" {:api "openai-chat" :url "https://new.test" :key "key"}}}) + (is (empty? @renewed*)) + (deliver (:done blocker) true) + (is (not= :timeout (deref work 5000 :timeout))) + (is (= ["https://new.test/models"] @fetched*)) + (is (= [@current*] @renewed*) + "renewal uses the provider config from its sync turn") + (finally (deliver (:done blocker) true))))))))) + +(deftest initialized-sync-renews-after-queue-test + (h/reset-components!) + (let [initial (assoc (config/all (h/db)) :providers {}) + current* (atom initial) + entered (promise) + renewed-once (promise) + renewed* (atom []) + blocker (models/reserve-sync!) + sync! models/sync-models!] + (try + (with-redefs [config/all (fn [_] @current*) + config/listen-for-changes! (fn [& _]) + f.hooks/trigger-if-matches! (fn [& _]) + f.tools/init-servers! (fn [& _]) + models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + f.login/renew-expiring-auth-tokens! (fn [{:keys [config]}] + (swap! renewed* conj config) + (deliver renewed-once true)) + models/sync-models! (fn [& args] + (deliver entered true) + (apply sync! args))] + (handlers/initialized (h/components)) + (is (= true (deref entered 5000 false))) + (is (empty? @renewed*) "startup renewal waits for its sync turn") + (reset! current* (assoc initial :providers {"new" {:fetchModels false :models {"pinned" {}}}})) + (deliver (:done blocker) true) + (is (= true (deref renewed-once 5000 false))) + (is (= [@current*] @renewed*)) + (let [callback (get-in @(h/db*) [:config-updated-fns :sync-models]) + newer (assoc initial :providers {"latest" {:fetchModels false :models {"pinned" {}}}})] + (reset! current* newer) + (callback @current* newer) + (is (= [(:providers (first @renewed*)) (:providers newer)] + (mapv :providers @renewed*))))) + (finally (deliver (:done blocker) true))))) + +(deftest models-refresh-renewal-warning-test + (h/reset-components!) + (with-redefs [f.login/renew-expiring-auth-tokens! + (fn [{:keys [on-renew-error]}] (on-renew-error "openai" "expired")) + models/sync-models! + (fn [db* config callback {:keys [before-sync]}] + (let [warnings* (atom [])] + (before-sync (if (fn? config) (config) config) + (fn [provider message] + (swap! warnings* conj {:provider provider :message message}))) + (swap! db* assoc :models {"openai/new" {}}) + (callback (:models @db*)) + {:model-count 1 :warnings @warnings*}))] + (is (= {:modelCount 1 :warnings [{:provider "openai" :message "expired"}]} + (handlers/models-refresh (h/components) {}))))) + +(deftest models-refresh-no-catalog-test + (h/reset-components!) + (with-redefs [models/sync-models! + (fn [_ _ _ _] + (throw (ex-info "No usable model catalog available; current models unchanged" + {:type :no-usable-model-catalog})))] + (is (= {:error {:code "no_usable_model_catalog" + :message "No usable model catalog available; current models unchanged"}} + (handlers/models-refresh (h/components) {}))) + (is (empty? (:config-updated (h/messages)))))) + (deftest initialize-test (testing "initializationOptions config is merged properly with default init config" (let [db* (atom {})] diff --git a/test/eca/llm_providers/ollama_test.clj b/test/eca/llm_providers/ollama_test.clj index 8ea725829..16f0a63d7 100644 --- a/test/eca/llm_providers/ollama_test.clj +++ b/test/eca/llm_providers/ollama_test.clj @@ -12,8 +12,8 @@ (let [req* (atom nil) fake-api-url "http://localhost:99" fake-response {:status 200 - :body {:models [{:name "model-a"} - {:name "model-b"}]}}] + :body {:models [{:model "model-a"} + {:model "model-b"}]}}] (with-client-proxied {} (fn handler [req] @@ -26,7 +26,25 @@ (select-keys @req* [:method :uri]))) ;; response parsing - (is (= [{:name "model-a"} {:name "model-b"}] result))))))) + (is (= [{:model "model-a"} {:model "model-b"}] result))))))) + +(deftest list-models-failure-vs-empty-test + (let [api-url "http://localhost:99"] + (testing "a valid empty catalog is a successful fetch" + (with-redefs [http/get (fn [_ _] {:status 200 :body {:models []}})] + (is (= [] (llm-providers.ollama/list-models {:api-url api-url}))))) + (testing "HTTP failure is not an empty catalog" + (with-redefs [http/get (fn [_ _] {:status 503 :body {:models []}})] + (is (nil? (llm-providers.ollama/list-models {:api-url api-url}))))) + (testing "transport exceptions are not empty catalogs" + (with-redefs [http/get (fn [_ _] (throw (ex-info "offline" {})))] + (is (nil? (llm-providers.ollama/list-models {:api-url api-url}))))) + (testing "a malformed successful catalog is not empty" + (doseq [body [{} {:models nil} {:models {}} {:models "invalid"} + {:models [nil]} {:models [{}]} {:models [{:model ""}]} + {:models [{:model 42}]}]] + (with-redefs [http/get (fn [_ _] {:status 200 :body body})] + (is (nil? (llm-providers.ollama/list-models {:api-url api-url})))))))) (deftest model-capabilities-test (testing "fetches capabilities for a specific Ollama model" diff --git a/test/eca/models_test.clj b/test/eca/models_test.clj index 1594275b6..e4bb3e066 100644 --- a/test/eca/models_test.clj +++ b/test/eca/models_test.clj @@ -7,6 +7,7 @@ [matcher-combinators.matchers :as m] [matcher-combinators.test :refer [match?]] [eca.cache :as cache] + [eca.llm-providers.ollama :as ollama] [eca.llm-util :as llm-util] [eca.logger :as logger] [eca.models :as models])) @@ -19,6 +20,342 @@ {:keys [models]} (#'models/fetch-provider-model-catalogs config db models-dev-data)] (#'models/build-all-supported-models known-models config models))) +(deftest manual-refresh-catalog-test + (let [config {:providers {"remote" {:api "openai-chat" :url "https://example.test" + :key "test" :models {"pinned" {}}}}} + db* (atom {:models {"remote/old" {:tools false}}}) + notified* (atom []) + catalog* (atom [{:id "new"}])] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [_ _] {:status 200 :body {:data @catalog*}})] + (let [refresh! #(models/sync-models! db* config + (fn [found] (swap! notified* conj found)) + {:refresh? true})] + (is (= {:model-count 2 :warnings []} (refresh!))) + (is (= #{"remote/new" "remote/pinned"} (set (keys (:models @db*))))) + (is (= {:model-count 2 :warnings []} (refresh!))) + (is (= 2 (count @notified*))) + (reset! catalog* [{:id "newer"}]) + (is (= {:model-count 2 :warnings []} (refresh!))) + (is (= #{"remote/newer" "remote/pinned"} (set (keys (:models @db*))))))))) + +(deftest manual-refresh-fallback-test + (let [config {:providers {"remote" {:api "openai-chat" :url "https://example.test" :key "test"}}} + db* (atom {:models {"remote/old" {:tools false :limit {:context 123}}}}) + response* (atom {:status 503 :body "offline"})] + (with-redefs [models/models-dev (fn [] {"remote" {"api" "https://example.test" + "models" {"from-dev" {}}}}) + ollama/list-models (fn [_] []) + http/get (fn [_ _] @response*)] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= #{"remote/from-dev"} (set (keys (:models @db*))))) + (is (= 1 (:model-count result))) + (is (= "remote" (get-in result [:warnings 0 :provider]))) + (is (re-find #"models.dev" (get-in result [:warnings 0 :message]))))) + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [_ _] @response*)] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (contains? (:models @db*) "remote/from-dev") + "the last known provider models remain available") + (is (= 1 (:model-count result))) + (is (some #(re-find #"stale" (:message %)) (:warnings result))))))) + +(deftest manual-refresh-static-and-ollama-test + (let [db* (atom {:models {}}) + config {:providers {"local" {:api "openai-chat" :url "https://local.test" + :fetchModels false :models {"pinned" {}}} + "ollama" {:models {"llama" {}}}}}] + (with-redefs [models/models-dev (fn [] {}) + http/get (fn [& _] (throw (ex-info "unexpected remote fetch" {}))) + ollama/list-models (fn [_] [{:model "llama"}]) + ollama/model-capabilities (fn [_] ["tools"])] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= {:model-count 2 :warnings []} result)) + (is (= #{"local/pinned" "ollama/llama"} (set (keys (:models @db*))))))))) + +(deftest manual-refresh-cached-models-dev-warning-test + (let [temp-dir (fs/create-temp-dir "eca-refresh-cache-test") + config {:providers {"remote" {:api "openai-chat" :url "https://example.test" :key "test"}}}] + (try + (spit (io/file (str temp-dir) "models-dev.json") + "{\"remote\":{\"api\":\"https://example.test\",\"models\":{\"cached\":{}}}}") + (with-redefs [cache/global-dir (fn [] (io/file (str temp-dir))) + ollama/list-models (fn [_] []) + http/get (fn [_ _] {:status 503 :body "offline"})] + (let [db* (atom {:models {}}) + result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= 1 (:model-count result))) + (is (contains? (:models @db*) "remote/cached")) + (is (= 1 (count (filter #(= "models.dev" (:provider %)) (:warnings result))))) + (is (some #(and (= "models.dev" (:provider %)) + (re-find #"cached metadata or fallback lists can be stale" (:message %))) + (:warnings result))))) + (finally + (fs/delete-tree temp-dir))))) + +(deftest manual-refresh-cached-models-dev-native-success-warning-test + (let [temp-dir (fs/create-temp-dir "eca-refresh-native-cache-test") + config {:providers {"remote" {:api "openai-chat" :url "https://example.test" :key "test"}}}] + (try + (spit (io/file (str temp-dir) "models-dev.json") + "{\"remote\":{\"api\":\"https://example.test\",\"models\":{\"cached\":{}}}}") + (with-redefs [cache/global-dir (fn [] (io/file (str temp-dir))) + ollama/list-models (fn [_] []) + http/get (fn [url _] + (if (re-find #"models.dev" url) + {:status 503 :body "offline"} + {:status 200 :body {:data [{:id "live"}]}}))] + (let [db* (atom {:models {}}) + result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= #{"remote/live"} (set (keys (:models @db*))))) + (is (= [{:provider "models.dev" + :message "Remote catalog unavailable; cached metadata or fallback lists can be stale"}] + (:warnings result))))) + (finally + (fs/delete-tree temp-dir))))) + +(deftest manual-refresh-partial-failure-test + (let [db* (atom {:models {"stale/old" {:tools false :limit {:context 123}}}}) + config {:providers {"stale" {:api "openai-chat" :url "https://stale.test" :key "test"} + "fresh" {:api "openai-chat" :url "https://fresh.test" :key "test"}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [url _] + (if (re-find #"fresh.test" url) + {:status 200 :body {:data [{:id "new"}]}} + {:status 503 :body "offline"}))] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= 2 (:model-count result))) + (is (= #{"fresh/new" "stale/old"} (set (keys (:models @db*))))) + (is (= {:tools false :limit {:context 123}} + (get-in @db* [:models "stale/old"]))) + (is (some #(and (= "stale" (:provider %)) + (re-find #"stale" (:message %))) (:warnings result))))))) + +(deftest manual-refresh-missing-provider-catalog-warning-test + (let [db* (atom {:models {}}) + config {:providers {"missing" {:api "google" :url "https://missing.test" :key "test"} + "fresh" {:api "openai-chat" :url "https://fresh.test" :key "test"}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [url _] + (if (re-find #"fresh.test" url) + {:status 200 :body {:data [{:id "new"}]}} + (throw (ex-info "unexpected request" {:url url}))))] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= 1 (:model-count result))) + (is (= #{"fresh/new"} (set (keys (:models @db*))))) + (is (some #(= "missing" (:provider %)) (:warnings result))))))) + +(deftest manual-refresh-serializes-syncs-test + (let [db* (atom {:models {}}) + config {:providers {"remote" {:api "openai-chat" :url "https://example.test" :key "test"}}} + entered (promise) + release (promise) + second-started (promise) + active* (atom 0) + max-active* (atom 0) + calls* (atom 0)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [_ _] + (let [active (swap! active* inc) + call (swap! calls* inc)] + (swap! max-active* max active) + (when (= 1 call) + (deliver entered true) + @release) + (swap! active* dec) + {:status 200 :body {:data [{:id (str "model-" call)}]}}))] + (let [first-sync (future (models/sync-models! db* config (fn [_]) {:refresh? true}))] + (try + (is (= true (deref entered 5000 false))) + (let [second-sync (future + (deliver second-started true) + (models/sync-models! db* config (fn [_]) {:refresh? true}))] + (is (= true (deref second-started 5000 false))) + (deliver release true) + (is (= 1 (:model-count (deref first-sync 5000 nil)))) + (is (= 1 (:model-count (deref second-sync 5000 nil)))) + (is (= 1 @max-active*)) + (is (= #{"remote/model-2"} (set (keys (:models @db*)))))) + (finally + (deliver release true))))))) + +(deftest manual-refresh-reserved-request-order-test + (let [db* (atom {:models {}}) + config {:providers {"remote" {:api "openai-chat" :url "https://example.test" :key "test"}}} + first-turn (models/reserve-sync!) + second-turn (models/reserve-sync!) + second-started (promise) + calls* (atom 0)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [_ _] + {:status 200 + :body {:data [{:id (str "model-" (swap! calls* inc))}]}})] + (let [second-sync (future + (deliver second-started true) + (models/sync-models! db* config (fn [_]) + {:refresh? true :turn second-turn}))] + (is (= true (deref second-started 5000 false))) + (let [first-sync (future (models/sync-models! db* config (fn [_]) + {:refresh? true :turn first-turn}))] + (is (= 1 (:model-count (deref first-sync 5000 nil)))) + (is (= 1 (:model-count (deref second-sync 5000 nil)))) + (is (= #{"remote/model-2"} (set (keys (:models @db*)))))))))) + +(deftest queued-sync-resolves-config-and-renews-inside-turn-test + (let [db* (atom {:models {}}) + current* (atom {:providers {"old" {:api "openai-chat" :url "https://old.test" :key "key"}}}) + entered (promise) + release (promise) + renewed* (atom []) + fetched* (atom []) + first-turn (models/reserve-sync!) + second-turn (models/reserve-sync!)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] []) + http/get (fn [url _] + (swap! fetched* conj url) + {:status 200 :body {:data [{:id "found"}]}})] + (let [first-sync (future (models/sync-models! db* + {:providers {}} + (fn [_] (deliver entered true) @release) + {:turn first-turn}))] + (try + (is (= true (deref entered 5000 false))) + (let [second-sync (future (models/sync-models! db* (fn [] @current*) (fn [_]) + {:turn second-turn + :before-sync (fn [config _warn!] + (swap! renewed* conj config))}))] + (reset! current* {:providers {"new" {:api "openai-chat" :url "https://new.test" :key "key"}}}) + (is (empty? @renewed*)) + (is (empty? @fetched*)) + (deliver release true) + (is (not= :timeout (deref first-sync 5000 :timeout))) + (is (not= :timeout (deref second-sync 5000 :timeout))) + (is (= [@current*] @renewed*)) + (is (= ["https://new.test/models"] @fetched*)) + (is (= #{"new/found"} (set (keys (:models @db*)))))) + (finally (deliver release true))))))) + +(deftest queued-sync-releases-turn-after-before-sync-error-test + (let [db* (atom {:models {}}) + first-turn (models/reserve-sync!) + second-turn (models/reserve-sync!)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] [])] + (is (thrown? clojure.lang.ExceptionInfo + (models/sync-models! db* {:providers {}} (fn [_]) + {:turn first-turn + :before-sync (fn [_ _] (throw (ex-info "renewal failed" {})))}))) + (is (not= :timeout + (deref (future (models/sync-models! db* {:providers {}} (fn [_]) + {:turn second-turn})) 5000 :timeout)))))) + +(deftest manual-refresh-ollama-failure-test + (let [old {:tools true :reason? true :limit {:context 4096}} + db* (atom {:models {"ollama/old" old}}) + config {:providers {"ollama" {} + "local" {:api "openai-chat" :fetchModels false + :models {"pinned" {}}}}} + notified* (atom nil)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (let [result (models/sync-models! db* config #(reset! notified* %) {:refresh? true})] + (is (= 2 (:model-count result))) + (is (= old (get-in @db* [:models "ollama/old"]))) + (is (= (:models @db*) @notified*)) + (is (some #(and (= "ollama" (:provider %)) + (re-find #"stale" (:message %))) (:warnings result))))))) + +(deftest manual-refresh-ollama-failure-without-prior-models-test + (let [db* (atom {:models {}}) + config {:providers {"ollama" {} + "local" {:api "openai-chat" :fetchModels false + :models {"pinned" {}}}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= 1 (:model-count result))) + (is (= #{"local/pinned"} (set (keys (:models @db*))))) + (is (some #(and (= "ollama" (:provider %)) + (re-find #"fetch failed" (:message %)) + (not (re-find #"stale" (:message %)))) + (:warnings result))))))) + +(deftest manual-refresh-ollama-valid-empty-test + (let [db* (atom {:models {"ollama/old" {:tools true}}}) + config {:providers {"ollama" {} + "local" {:api "openai-chat" :fetchModels false + :models {"pinned" {}}}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] [])] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= {:model-count 1 :warnings []} result)) + (is (= #{"local/pinned"} (set (keys (:models @db*))))))))) + +(deftest manual-refresh-ollama-removed-test + (let [db* (atom {:models {"ollama/old" {:tools true}}}) + config {:providers {"local" {:api "openai-chat" :fetchModels false + :models {"pinned" {}}}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (let [result (models/sync-models! db* config (fn [_]) {:refresh? true})] + (is (= {:model-count 1 :warnings []} result)) + (is (= #{"local/pinned"} (set (keys (:models @db*))))))))) + +(deftest manual-refresh-ollama-only-failure-test + (let [old {"ollama/old" {:tools true}} + db* (atom {:models old}) + notified* (atom 0)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (let [result (models/sync-models! db* {:providers {"ollama" {}}} + (fn [_] (swap! notified* inc)) {:refresh? true})] + (is (= 1 (:model-count result))) + (is (= old (:models @db*))) + (is (= 1 @notified*)) + (is (some #(and (= "ollama" (:provider %)) + (re-find #"stale" (:message %))) (:warnings result))))))) + +(deftest startup-ollama-failure-does-not-retain-models-test + (let [db* (atom {:models {"ollama/old" {:tools true}}}) + config {:providers {"ollama" {} + "local" {:api "openai-chat" :fetchModels false + :models {"pinned" {}}}}}] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (models/sync-models! db* config (fn [_])) + (is (= #{"local/pinned"} (set (keys (:models @db*)))))))) + +(deftest manual-refresh-ollama-no-prior-models-test + (let [db* (atom {:models {}}) + notified* (atom 0)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] nil)] + (is (thrown-with-msg? clojure.lang.ExceptionInfo #"No usable model catalog" + (models/sync-models! db* {:providers {"ollama" {}}} + (fn [_] (swap! notified* inc)) + {:refresh? true}))) + (is (= {} (:models @db*))) + (is (zero? @notified*))))) + +(deftest manual-refresh-total-failure-test + (let [db* (atom {:models {"removed/old" {:tools true}}}) + notified* (atom 0)] + (with-redefs [models/models-dev (fn [] {}) + ollama/list-models (fn [_] [])] + (is (thrown-with-msg? clojure.lang.ExceptionInfo #"No usable model catalog" + (models/sync-models! db* {:providers {}} + (fn [_] (swap! notified* inc)) + {:refresh? true}))) + (is (= {"removed/old" {:tools true}} (:models @db*))) + (is (zero? @notified*))))) + (deftest fetch-models-dev-data-test (testing "Uses hato with json-string-keys and global client options" (let [request* (atom nil)] diff --git a/test/eca/server_test.clj b/test/eca/server_test.clj index a965f0ba7..541efce16 100644 --- a/test/eca/server_test.clj +++ b/test/eca/server_test.clj @@ -2,8 +2,27 @@ (:require [babashka.process :as p] [clojure.test :refer [deftest is testing]] + [eca.handlers :as handlers] [eca.server :as server] - [eca.test-helper :as h])) + [eca.test-helper :as h] + [jsonrpc4clj.server :as jsonrpc.server])) + +(deftest models-refresh-request-test + (let [entered (promise) + release (promise)] + (with-redefs [handlers/models-refresh (fn [{:keys [model-sync-turn]} _] + (deliver entered model-sync-turn) + @release + (deliver (:done model-sync-turn) true) + {:model-count 1 :warnings []})] + (let [response (jsonrpc.server/receive-request "models/refresh" {} {})] + (try + (is (some? (:done (deref entered 5000 nil))) + "the request dispatches with a reserved sync turn") + (deliver release true) + (is (= {:model-count 1 :warnings []} (deref response 5000 nil))) + (finally + (deliver release true))))))) (defn ^:private spawn-blocking-process [] ;; Long-running child whose pid we own. `sleep 600` is fine on Linux/macOS; From ba48629e3d264fb79b82f68310ec048cf957218b Mon Sep 17 00:00:00 2001 From: Hakan Nilsson Date: Mon, 5 Oct 2026 10:02:23 +0200 Subject: [PATCH 2/2] Preserve model sync order across provider auth requests Reserve sync turns before dispatch so a refresh cannot publish a stale catalog after a prior login or logout request. Release unused turns on failure or when no sync occurs. --- src/eca/features/providers.clj | 102 ++++++++++++++++++--------------- src/eca/handlers.clj | 8 +-- src/eca/server.clj | 16 +++++- test/eca/server_test.clj | 81 ++++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 52 deletions(-) diff --git a/src/eca/features/providers.clj b/src/eca/features/providers.clj index 74a3baec4..f73115304 100644 --- a/src/eca/features/providers.clj +++ b/src/eca/features/providers.clj @@ -208,23 +208,28 @@ (messenger/provider-updated messenger (build-provider-status provider-name @db* config))) +(def ^:dynamic *model-sync-turn* nil) + (defn sync-and-notify! "Common post-login/logout action: persist auth cache, re-sync models, and send both config/updated and providers/updated notifications." - [provider-name db* messenger metrics] - (db/update-global-auth-cache! @db* provider-name metrics) - (let [sync-config* (volatile! nil)] - (models/sync-models! db* #(config/all @db*) - (fn [new-models] - (config/notify-fields-changed-only! - {:chat {:models (sort (keys new-models))}} messenger db*) - (notify-provider-updated! provider-name db* @sync-config* messenger)) - {:before-sync (fn [config _warn!] - (vreset! sync-config* config) - ;; Login already depends on providers, so resolve renewal at call time. - ((requiring-resolve 'eca.features.login/renew-expiring-auth-tokens!) - {:db* db* :messenger messenger - :config config :metrics metrics}))}))) + ([provider-name db* messenger metrics] + (sync-and-notify! provider-name db* messenger metrics nil)) + ([provider-name db* messenger metrics turn] + (db/update-global-auth-cache! @db* provider-name metrics) + (let [sync-config* (volatile! nil)] + (models/sync-models! db* #(config/all @db*) + (fn [new-models] + (config/notify-fields-changed-only! + {:chat {:models (sort (keys new-models))}} messenger db*) + (notify-provider-updated! provider-name db* @sync-config* messenger)) + {:turn (or turn *model-sync-turn*) + :before-sync (fn [config _warn!] + (vreset! sync-config* config) + ;; Login already depends on providers, so resolve renewal at call time. + ((requiring-resolve 'eca.features.login/renew-expiring-auth-tokens!) + {:db* db* :messenger messenger + :config config :metrics metrics}))})))) ;; --- Login dispatch --- @@ -294,38 +299,43 @@ (defn provider-login-input "Processes login input submitted by the client." - [provider-name data db* config messenger metrics] - (if (:code data) - ;; OAuth code exchange (e.g., Anthropic after browser auth) - (complete-oauth-code! provider-name data db* config messenger metrics) - ;; API key input - (let [base-provider (config/provider-base provider-name config) - api-key (get data :api-key) - models-str (get data :models) - url (get data :url) - provider-cfg (get provider-configs base-provider) - models-map (when (not-empty models-str) - (into {} (map (fn [m] [(string/trim m) {}]) - (string/split models-str #","))))] - (if (get provider-login-fields base-provider) - ;; Providers that save to config (google, deepseek, openrouter, z-ai, azure) - (let [config-update (shared/assoc-some - (merge {:key api-key} provider-cfg) - :url (or url (:url provider-cfg)) - :models models-map)] - (config/update-global-config! {:providers {provider-name config-update}}) - (swap! db* assoc-in [:auth provider-name] {:step :login/done :type :auth/token})) - ;; Manual key for OAuth providers (anthropic, openai) - (swap! db* assoc-in [:auth provider-name] {:step :login/done - :type :auth/token - :api-key api-key - :mode :manual})) - (sync-and-notify! provider-name db* messenger metrics) - {:action "done"}))) + ([provider-name data db* config messenger metrics] + (provider-login-input provider-name data db* config messenger metrics nil)) + ([provider-name data db* config messenger metrics turn] + (if (:code data) + ;; OAuth code exchange (e.g., Anthropic after browser auth) + (binding [*model-sync-turn* turn] + (complete-oauth-code! provider-name data db* config messenger metrics)) + ;; API key input + (let [base-provider (config/provider-base provider-name config) + api-key (get data :api-key) + models-str (get data :models) + url (get data :url) + provider-cfg (get provider-configs base-provider) + models-map (when (not-empty models-str) + (into {} (map (fn [m] [(string/trim m) {}]) + (string/split models-str #","))))] + (if (get provider-login-fields base-provider) + ;; Providers that save to config (google, deepseek, openrouter, z-ai, azure) + (let [config-update (shared/assoc-some + (merge {:key api-key} provider-cfg) + :url (or url (:url provider-cfg)) + :models models-map)] + (config/update-global-config! {:providers {provider-name config-update}}) + (swap! db* assoc-in [:auth provider-name] {:step :login/done :type :auth/token})) + ;; Manual key for OAuth providers (anthropic, openai) + (swap! db* assoc-in [:auth provider-name] {:step :login/done + :type :auth/token + :api-key api-key + :mode :manual})) + (sync-and-notify! provider-name db* messenger metrics turn) + {:action "done"})))) (defn provider-logout "Clears auth for a provider and re-syncs models." - [provider-name db* _config messenger metrics] - (swap! db* assoc-in [:auth provider-name] {}) - (sync-and-notify! provider-name db* messenger metrics) - {}) + ([provider-name db* config messenger metrics] + (provider-logout provider-name db* config messenger metrics nil)) + ([provider-name db* _config messenger metrics turn] + (swap! db* assoc-in [:auth provider-name] {}) + (sync-and-notify! provider-name db* messenger metrics turn) + {})) diff --git a/src/eca/handlers.clj b/src/eca/handlers.clj index a494faa2b..8dec142ac 100644 --- a/src/eca/handlers.clj +++ b/src/eca/handlers.clj @@ -645,11 +645,11 @@ (handle-expected-errors (f.providers/provider-login (:provider params) (:method params) db* config messenger metrics)))) -(defn providers-login-input [{:keys [db* config messenger metrics]} params] +(defn providers-login-input [{:keys [db* config messenger metrics model-sync-turn]} params] (metrics/task metrics :eca/providers-login-input (handle-expected-errors - (f.providers/provider-login-input (:provider params) (:data params) db* config messenger metrics)))) + (f.providers/provider-login-input (:provider params) (:data params) db* config messenger metrics model-sync-turn)))) -(defn providers-logout [{:keys [db* config messenger metrics]} params] +(defn providers-logout [{:keys [db* config messenger metrics model-sync-turn]} params] (metrics/task metrics :eca/providers-logout - (f.providers/provider-logout (:provider params) db* config messenger metrics))) + (f.providers/provider-logout (:provider params) db* config messenger metrics model-sync-turn))) diff --git a/src/eca/server.clj b/src/eca/server.clj index 27340f969..fbc18bcb1 100644 --- a/src/eca/server.clj +++ b/src/eca/server.clj @@ -252,10 +252,22 @@ (eventually (handlers/providers-login (with-config components) params))) (defmethod jsonrpc.server/receive-request "providers/loginInput" [_ components params] - (eventually (handlers/providers-login-input (with-config components) params))) + (let [turn (models/reserve-sync!)] + (eventually + (try + (handlers/providers-login-input (assoc (with-config components) :model-sync-turn turn) params) + (finally + (when-not (realized? (:done turn)) + (deliver (:done turn) true))))))) (defmethod jsonrpc.server/receive-request "providers/logout" [_ components params] - (eventually (handlers/providers-logout (with-config components) params))) + (let [turn (models/reserve-sync!)] + (eventually + (try + (handlers/providers-logout (assoc (with-config components) :model-sync-turn turn) params) + (finally + (when-not (realized? (:done turn)) + (deliver (:done turn) true))))))) (defmethod jsonrpc.server/receive-request "completion/inline" [_ components params] (eventually (handlers/completion-inline (with-config components) params))) diff --git a/test/eca/server_test.clj b/test/eca/server_test.clj index 541efce16..1447cda74 100644 --- a/test/eca/server_test.clj +++ b/test/eca/server_test.clj @@ -2,7 +2,10 @@ (:require [babashka.process :as p] [clojure.test :refer [deftest is testing]] + [eca.db :as db] + [eca.features.login :as login] [eca.handlers :as handlers] + [eca.models :as models] [eca.server :as server] [eca.test-helper :as h] [jsonrpc4clj.server :as jsonrpc.server])) @@ -24,6 +27,84 @@ (finally (deliver release true))))))) +(deftest provider-mutation-precedes-refresh-test + (doseq [[method handler provider-params initial-auth] + [["providers/logout" #'handlers/providers-logout {:provider "openai"} true] + ["providers/loginInput" #'handlers/providers-login-input + {:provider "openai" :data {:api-key "new-key"}} false]]] + (testing method + (h/reset-components!) + (swap! (h/db*) assoc-in [:auth "openai"] + (if initial-auth {:step :login/done :type :auth/token :api-key "old-key"} {})) + (let [entered (promise) + refresh-entered (promise) + release (promise) + original @handler + original-refresh handlers/models-refresh + catalog (fn [] (if (get-in @(h/db*) [:auth "openai" :api-key]) + "openai/authorized" "openai/anonymous"))] + (with-redefs-fn + {handler (fn [components params] + (deliver entered true) + @release + (original components params)) + #'handlers/models-refresh (fn [components params] + (deliver refresh-entered true) + (original-refresh components params)) + #'db/update-global-cache! (fn [& _]) + #'login/renew-expiring-auth-tokens! (fn [& _]) + #'eca.models/sync-models-now! + (fn [db* _config callback _refresh?] + (let [name (catalog) + models {name {}}] + (swap! db* assoc :models models) + (callback models) + {:model-count 1}))} + (fn [] + (let [mutation (jsonrpc.server/receive-request method (h/components) provider-params)] + (try + (is (= true (deref entered 5000 :timeout))) + (let [refresh (jsonrpc.server/receive-request "models/refresh" (h/components) {})] + (is (= true (deref refresh-entered 5000 :timeout))) + (deliver release true) + (is (not= :timeout (deref mutation 5000 :timeout))) + (is (= {:modelCount 1 :warnings []} (deref refresh 5000 :timeout))) + (is (= [(if initial-auth "openai/anonymous" "openai/authorized")] + (get-in (last (:config-updated (h/messages))) [:chat :models]))) + (is (= 1 (count (:config-updated (h/messages)))) + "refresh must not publish the old catalog before the mutation")) + (finally (deliver release true)))))))))) + +(deftest provider-unused-turn-does-not-block-refresh-test + (doseq [[result handler] [[:no-sync (fn [_ _] {:action "input"})] + [:error (fn [_ _] (throw (ex-info "invalid input" {})))]]] + (testing (name result) + (h/reset-components!) + (let [entered (promise) + release (promise)] + (with-redefs [handlers/providers-login-input (fn [components params] + (deliver entered true) + @release + (handler components params)) + login/renew-expiring-auth-tokens! (fn [& _]) + models/models-dev (fn [] {})] + (let [mutation (jsonrpc.server/receive-request + "providers/loginInput" (h/components) + {:provider "openai" :data {:api-key "key"}})] + (try + (is (= true (deref entered 5000 :timeout))) + (let [refresh (jsonrpc.server/receive-request "models/refresh" (h/components) {})] + (deliver release true) + (is (if (= result :error) + (try + (deref mutation 5000 :timeout) + false + (catch java.util.concurrent.ExecutionException _ true)) + (= {:action "input"} (deref mutation 5000 :timeout)))) + (is (not= :timeout (deref refresh 5000 :timeout)) + "an unused turn must not strand later refreshes")) + (finally (deliver release true))))))))) + (defn ^:private spawn-blocking-process [] ;; Long-running child whose pid we own. `sleep 600` is fine on Linux/macOS; ;; subprocess-based tests are skipped on Windows.