From a46f23e0eea1828418bf654fe21767a64e0ccea6 Mon Sep 17 00:00:00 2001 From: Jakub Zika Date: Wed, 8 Apr 2026 10:35:38 +0200 Subject: [PATCH] Fix auth refresh propagation during streaming tool calls Auth tokens were captured in provider closures at prompt start. When tokens expired during long-running tool calls like spawn_agent, the renewed credentials in db* weren't propagated back to the recursive streaming calls, causing mid-session auth failures. on-tools-called! now returns fresh-api-key and provider-auth so providers can use refreshed credentials for subsequent requests. Also renews tokens 60 seconds before expiration to avoid race between check and request. --- CHANGELOG.md | 1 + src/eca/features/chat/tool_calls.clj | 21 +++++--- src/eca/features/login.clj | 23 +++++---- src/eca/llm_providers/anthropic.clj | 6 +-- src/eca/llm_providers/copilot.clj | 2 +- src/eca/llm_providers/openai.clj | 11 +++-- src/eca/llm_providers/openai_chat.clj | 4 +- test/eca/features/chat/tool_calls_test.clj | 56 ++++++++++++++++++++++ test/eca/llm_providers/openai_test.clj | 49 +++++++++++++++++++ 9 files changed, 145 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ce1a39f4..91a829e9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## 0.124.2 - Fix OpenAI Responses API tool calls not executing when streaming response returns empty output, and fix spurious retries caused by stale tool-call state with Copilot encrypted IDs. #398 +- Fix auth refresh propagation during streaming and `spawn_agent` tool calls, preventing mid-session failures. ## 0.124.1 diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj index faa056b57..6777ff4a8 100644 --- a/src/eca/features/chat/tool_calls.clj +++ b/src/eca/features/chat/tool_calls.clj @@ -748,8 +748,8 @@ :details details :summary summary :progress-text (if (= name "spawn_agent") - "Waiting subagent" - "Calling tool")}))) + "Waiting subagent" + "Calling tool")}))) (let [tool-call-state (get-tool-call-state @db* chat-id id) {:keys [code text]} (:decision-reason tool-call-state) effective-hook-continue (when hook-rejected? hook-continue) @@ -834,8 +834,15 @@ nil)))) (do (lifecycle/maybe-renew-auth-token chat-ctx) - (if-let [continue-fn (:continue-fn chat-ctx)] - (continue-fn all-tools user-messages) - {:tools all-tools - :new-messages (shared/messages-after-last-compact-marker - (get-in @db* [:chats chat-id :messages]))})))))))))) + (let [provider-auth (get-in @db* [:auth (:provider chat-ctx)]) + [_ fresh-api-key] (llm-util/provider-api-key (:provider chat-ctx) + provider-auth + config) + result (if-let [continue-fn (:continue-fn chat-ctx)] + (continue-fn all-tools user-messages) + {:tools all-tools + :new-messages (get-in @db* [:chats chat-id :messages])})] + (when result + (assoc result + :fresh-api-key fresh-api-key + :provider-auth provider-auth)))))))))))) diff --git a/src/eca/features/login.clj b/src/eca/features/login.clj index 379766239..d6cc545f3 100644 --- a/src/eca/features/login.clj +++ b/src/eca/features/login.clj @@ -91,24 +91,27 @@ (defn maybe-renew-auth-token! [{:keys [provider on-renewing on-error]} ctx] (when-let [expires-at (get-in @(:db* ctx) [:auth provider :expires-at])] - (when (<= (long expires-at) (quot (System/currentTimeMillis) 1000)) + ;; Renew 60 seconds before expiration to avoid race between check and request + (when (<= (long expires-at) (+ 60 (quot (System/currentTimeMillis) 1000))) (when on-renewing (on-renewing)) (renew-auth! provider ctx {:on-error on-error})))) (defn login-done! [{:keys [chat-id db* messenger metrics provider send-msg!]} - & {:keys [silent?] - :or {silent? false}}] + & {:keys [silent? skip-models-sync?] + :or {silent? false + skip-models-sync? false}}] (when (get-in @db* [:auth provider]) (db/update-global-cache! @db* metrics)) - (models/sync-models! db* - (config/all @db*) ;; force get updated config - (fn [new-models] - (messenger/config-updated - messenger - {:chat - {:models (sort (keys new-models))}}))) + (when-not skip-models-sync? + (models/sync-models! db* + (config/all @db*) ;; force get updated config + (fn [new-models] + (messenger/config-updated + messenger + {:chat + {:models (sort (keys new-models))}})))) (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/llm_providers/anthropic.clj b/src/eca/llm_providers/anthropic.clj index a8205f6ff..da66066e0 100644 --- a/src/eca/llm_providers/anthropic.clj +++ b/src/eca/llm_providers/anthropic.clj @@ -446,7 +446,7 @@ :full-name (:name content-block) :arguments (json/parse-string (:input-json content-block))})) (vals @content-block*))] - (when-let [{:keys [new-messages tools]} (on-tools-called tool-calls)] + (when-let [{:keys [new-messages tools fresh-api-key]} (on-tools-called tool-calls)] (let [messages (-> new-messages group-parallel-tool-calls (normalize-messages supports-image?) @@ -460,7 +460,7 @@ :messages messages :tools (->tools tools web-search)) :api-url api-url - :api-key api-key + :api-key (or fresh-api-key api-key) :http-client http-client :extra-headers extra-headers :auth-type auth-type @@ -698,4 +698,4 @@ :refresh-token refresh-token :api-key access-token :expires-at expires-at}) - (f.login/login-done! ctx :silent? true))) + (f.login/login-done! ctx :silent? true :skip-models-sync? true))) diff --git a/src/eca/llm_providers/copilot.clj b/src/eca/llm_providers/copilot.clj index 2290c765a..a2483481c 100644 --- a/src/eca/llm_providers/copilot.clj +++ b/src/eca/llm_providers/copilot.clj @@ -130,4 +130,4 @@ {:keys [api-key expires-at]} (oauth-renew-token access-token)] (swap! db* update-in [:auth provider] merge {:api-key api-key :expires-at expires-at}) - (f.login/login-done! ctx :silent? true))) + (f.login/login-done! ctx :silent? true :skip-models-sync? true))) diff --git a/src/eca/llm_providers/openai.clj b/src/eca/llm_providers/openai.clj index 9b8997ec1..e0cec0b32 100644 --- a/src/eca/llm_providers/openai.clj +++ b/src/eca/llm_providers/openai.clj @@ -276,8 +276,9 @@ :output-tokens (-> response :usage :output_tokens) :input-cache-read-tokens input-cache-read-tokens})) (if (seq tool-calls) - (when-let [{:keys [new-messages tools]} (on-tools-called tool-calls)] - (reset! tool-call-by-item-id* {}) + (when-let [{:keys [new-messages tools fresh-api-key provider-auth]} (on-tools-called tool-calls)] + (doseq [tool-call tool-calls] + (swap! tool-call-by-item-id* dissoc (:item-id tool-call))) (base-responses-request! {:rid (llm-util/gen-rid) :body (assoc body @@ -285,8 +286,8 @@ :tools (->tools tools web-search codex?)) :api-url api-url :url-relative-path url-relative-path - :api-key api-key - :account-id account-id + :api-key (or fresh-api-key api-key) + :account-id (or (:account-id provider-auth) account-id) :http-client http-client :extra-headers extra-headers :auth-type auth-type @@ -490,4 +491,4 @@ :api-key access-token :account-id new-account-id :expires-at expires-at}) - (f.login/login-done! ctx :silent? true))) + (f.login/login-done! ctx :silent? true :skip-models-sync? true))) diff --git a/src/eca/llm_providers/openai_chat.clj b/src/eca/llm_providers/openai_chat.clj index 5b1406313..9853f174a 100644 --- a/src/eca/llm_providers/openai_chat.clj +++ b/src/eca/llm_providers/openai_chat.clj @@ -500,7 +500,7 @@ k)) tool-calls)) on-tools-called-wrapper (fn on-tools-called-wrapper [tools-to-call on-tools-called handle-response] - (when-let [{:keys [new-messages tools]} (on-tools-called tools-to-call)] + (when-let [{:keys [new-messages tools fresh-api-key]} (on-tools-called tools-to-call)] (let [pruned-messages (prune-history new-messages reasoning-history) new-messages-list (vec (concat system-messages @@ -516,7 +516,7 @@ :extra-headers extra-headers :http-client http-client :api-url api-url - :api-key api-key + :api-key (or fresh-api-key api-key) :url-relative-path url-relative-path :on-error wrapped-on-error :on-stream (when stream? (fn [event data] (handle-response event data tool-calls*)))})))) diff --git a/test/eca/features/chat/tool_calls_test.clj b/test/eca/features/chat/tool_calls_test.clj index bc91aad59..db0f5660b 100644 --- a/test/eca/features/chat/tool_calls_test.clj +++ b/test/eca/features/chat/tool_calls_test.clj @@ -1,6 +1,7 @@ (ns eca.features.chat.tool-calls-test (:require [clojure.test :refer [deftest is testing]] + [eca.features.chat.lifecycle :as lifecycle] [eca.features.chat.tool-calls :as tc] [eca.features.hooks :as f.hooks] [eca.features.tools :as f.tools] @@ -276,3 +277,58 @@ :hook-rejected? false :arguments-modified? true} plan))))))) + +(deftest on-tools-called!-returns-provider-auth-test + (testing "returns refreshed provider auth in result after token is renewed during tool execution" + ;; Regression test for: auth captured in LLM provider closure at prompt start. + ;; When a token expires mid-stream, maybe-renew-auth-token updates db*, + ;; and on-tools-called! must return the refreshed auth so provider-specific + ;; continuation metadata (not just the api key) can be reused. + (h/reset-components!) + (let [chat-id "test-chat" + provider "github-copilot" + renewed-provider-auth {:api-key "fresh-token-xyz" + :expires-at 9999999999} + db* (h/db*) + _ (swap! db* #(-> % + (assoc-in [:auth provider :api-key] "stale-token-abc") + (assoc-in [:chats chat-id :status] :running) + (assoc-in [:chats chat-id :messages] []) + (assoc-in [:chats chat-id :tool-calls "call-1" :status] :preparing))) + ;; Pre-set tool call to :preparing state, as it would be after on-prepare-tool-call + ;; fires during the streaming phase before on-tools-called! is invoked. + chat-ctx {:db* db* + :config (h/config) + :chat-id chat-id + :provider provider + :agent :default + :messenger (h/messenger) + :metrics (h/metrics)} + received-msgs* (atom "") + add-to-history! (fn [msg] + (swap! db* update-in [:chats chat-id :messages] (fnil conj []) msg)) + tool-calls [{:id "call-1" + :full-name "eca__test_tool" + :arguments {} + :arguments-text "{}"}] + all-tools [{:name "test_tool" + :full-name "eca__test_tool" + :origin :eca + :server {:name "eca"}}] + expected-provider-auth (merge {:api-key "stale-token-abc"} renewed-provider-auth)] + (with-redefs [f.tools/all-tools (constantly all-tools) + f.tools/approval (constantly :allow) + f.hooks/trigger-if-matches! (fn [_ _ _ _ _] nil) + f.tools/call-tool! (fn [& _] {:contents [{:text "result" :type :text}]}) + f.tools/tool-call-details-before-invocation (constantly nil) + f.tools/tool-call-details-after-invocation (constantly nil) + f.tools/tool-call-summary (constantly "Test tool") + lifecycle/maybe-renew-auth-token + (fn [ctx] + (swap! (:db* ctx) update-in [:auth provider] merge renewed-provider-auth))] + (let [result ((tc/on-tools-called! chat-ctx received-msgs* add-to-history! []) tool-calls)] + (is (= (:api-key expected-provider-auth) (:fresh-api-key result)) + "fresh-api-key must be returned so the provider can use it in the recursive streaming call") + (is (= expected-provider-auth + (:provider-auth result)) + "provider-auth must be returned so providers can reuse refreshed auth metadata")))))) diff --git a/test/eca/llm_providers/openai_test.clj b/test/eca/llm_providers/openai_test.clj index 5f9a9668f..a42658c23 100644 --- a/test/eca/llm_providers/openai_test.clj +++ b/test/eca/llm_providers/openai_test.clj @@ -113,6 +113,55 @@ (is (> (:expires-at result) now-seconds) "expires-at should be computed relative to current time")))))) +(deftest create-response-refreshes-account-id-after-tool-call-test + (testing "uses refreshed provider auth metadata after a long-running tool call" + (let [requests* (atom [])] + (with-redefs [llm-providers.openai/base-responses-request! + (fn [{:keys [api-key account-id on-stream] :as _opts}] + (swap! requests* conj {:api-key api-key + :account-id account-id}) + (when (= 1 (count @requests*)) + (on-stream "response.completed" + {:response {:output [{:type "function_call" + :id "item-1" + :call_id "call-1" + :name "eca__spawn_agent" + :arguments "{}"}] + :usage {:input_tokens 1 + :output_tokens 1}}})) + :ok)] + (llm-providers.openai/create-response! + {:model "gpt-test" + :user-messages [{:role "user" :content [{:type :text :text "hi"}]}] + :instructions "ins" + :reason? false + :supports-image? false + :api-key "stale-token" + :api-url "http://localhost:1" + :past-messages [] + :tools [{:full-name "eca__spawn_agent" :description "spawn" :parameters {:type "object"}}] + :web-search false + :extra-payload {} + :extra-headers nil + :auth-type :auth/oauth + :account-id "old-account"} + {:on-message-received (fn [_]) + :on-error (fn [e] (throw (ex-info "err" e))) + :on-prepare-tool-call (fn [_]) + :on-tools-called (fn [_] + {:new-messages [] + :tools [] + :fresh-api-key "fresh-token" + :provider-auth {:account-id "new-account"}}) + :on-reason (fn [_]) + :on-usage-updated (fn [_]) + :on-server-web-search (fn [_])}) + (is (= [{:api-key "stale-token" + :account-id "old-account"} + {:api-key "fresh-token" + :account-id "new-account"}] + @requests*)))))) + (deftest ->normalize-messages-test (testing "no previous history" (is (match?