Skip to content

Commit 51ce4af

Browse files
ericdalloeca-agent
ericdallo
and
eca-agent
committed
Fix Copilot provider retry loop from stale tool-call atom
Three root causes combined to produce an infinite retry loop exclusively with the Copilot provider: 1. response.output_item.done used item_id for atom lookup, but Copilot encrypts IDs differently across SSE events. This created orphan atom entries ({:arguments "..."} with no :id/:full-name). 2. The doseq/dissoc cleanup used response.completed's item_id which never matched the streaming item_id, so stale entries accumulated. 3. The catch block in base-responses-request! labeled ALL exceptions as "Connection error:", causing internal state-machine errors to be classified as :overloaded and retried up to 10 times. Fixes: - response.output_item.done now looks up existing atom entries by call_id (consistent across Copilot events) instead of item_id - Replace doseq/dissoc with reset! to clear the entire atom before recursive requests - Catch block distinguishes ex-info (Internal error) from IO exceptions (Connection error) to prevent misclassification Adds 4 unit tests covering: primary path, fallback path, phantom tool call prevention, and Copilot-style mismatched item IDs. Refs #398 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev>
1 parent ab1b377 commit 51ce4af

File tree

2 files changed

+242
-6
lines changed

2 files changed

+242
-6
lines changed

‎src/eca/llm_providers/openai.clj‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,10 @@
9696
(llm-util/log-response logger-tag rid "response" body)
9797
(response-body->result body)))))
9898
(catch Exception e
99-
(on-error {:exception e
100-
:message (format "Connection error: %s" (or (ex-message e) (.getName (class e))))})))))
99+
(let [msg (or (ex-message e) (.getName (class e)))
100+
prefix (if (ex-data e) "Internal error" "Connection error")]
101+
(on-error {:exception e
102+
:message (format "%s: %s" prefix msg)}))))))
101103

102104
(defn ^:private normalize-messages [messages supports-image?]
103105
(keep (fn [{:keys [role content] :as msg}]
@@ -199,8 +201,17 @@
199201
(on-reason {:status :finished
200202
:id (-> data :item :id)
201203
:external-id (-> data :item :encrypted_content)}))
202-
"function_call" (swap! tool-call-by-item-id* update (-> data :item :id)
203-
assoc :arguments (-> data :item :arguments))
204+
"function_call" (let [done-item-id (-> data :item :id)
205+
done-call-id (-> data :item :call_id)
206+
args (-> data :item :arguments)]
207+
(swap! tool-call-by-item-id*
208+
(fn [m]
209+
(if-let [existing-key (or (when (contains? m done-item-id) done-item-id)
210+
(->> m
211+
(some (fn [[k v]]
212+
(when (= done-call-id (:id v)) k)))))]
213+
(assoc-in m [existing-key :arguments] args)
214+
(assoc m done-item-id {:arguments args})))))
204215
"web_search_call" (on-server-web-search {:status :finished
205216
:id (-> data :item :id)
206217
:output nil})
@@ -277,8 +288,7 @@
277288
:input-cache-read-tokens input-cache-read-tokens}))
278289
(if (seq tool-calls)
279290
(when-let [{:keys [new-messages tools fresh-api-key provider-auth]} (on-tools-called tool-calls)]
280-
(doseq [tool-call tool-calls]
281-
(swap! tool-call-by-item-id* dissoc (:item-id tool-call)))
291+
(reset! tool-call-by-item-id* {})
282292
(base-responses-request!
283293
{:rid (llm-util/gen-rid)
284294
:body (assoc body

‎test/eca/llm_providers/openai_test.clj‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,229 @@
211211
[{:role "user" :content [{:type :text :text "Check diagnostics"}]}
212212
{:role "tool_call" :content {:id "call-1" :full-name "eca__editor_diagnostics" :arguments nil}}]
213213
true)))))
214+
215+
(defn- base-provider-params []
216+
{:model "gpt-test"
217+
:user-messages [{:role "user" :content [{:type :text :text "hi"}]}]
218+
:instructions "test"
219+
:reason? false
220+
:supports-image? false
221+
:api-key "fake-key"
222+
:api-url "http://localhost:1"
223+
:past-messages []
224+
:tools [{:full-name "eca__shell_command" :description "run" :parameters {:type "object"}}]
225+
:web-search false
226+
:extra-payload {}
227+
:extra-headers nil
228+
:auth-type :auth/api-key
229+
:account-id nil})
230+
231+
(defn- base-callbacks [{:keys [on-prepare-tool-call on-tools-called on-message-received on-error]
232+
:or {on-prepare-tool-call (fn [_])
233+
on-tools-called (fn [_] {:new-messages [] :tools []})
234+
on-message-received (fn [_])
235+
on-error (fn [e] (throw (ex-info "unexpected error in test" e)))}}]
236+
{:on-message-received on-message-received
237+
:on-error on-error
238+
:on-prepare-tool-call on-prepare-tool-call
239+
:on-tools-called on-tools-called
240+
:on-reason (fn [_])
241+
:on-usage-updated (fn [_])
242+
:on-server-web-search (fn [_])})
243+
244+
(deftest create-response-tool-calls-via-output-test
245+
(testing "tool calls in response.completed output trigger callbacks correctly"
246+
(let [prepare-calls* (atom [])
247+
tools-called* (atom [])
248+
requests* (atom [])]
249+
(with-redefs [llm-providers.openai/base-responses-request!
250+
(fn [{:keys [on-stream] :as opts}]
251+
(swap! requests* conj opts)
252+
(when (= 1 (count @requests*))
253+
(on-stream "response.output_item.added"
254+
{:item {:type "function_call"
255+
:id "item-1"
256+
:call_id "call-1"
257+
:name "eca__shell_command"
258+
:arguments ""}})
259+
(on-stream "response.function_call_arguments.delta"
260+
{:item_id "item-1"
261+
:delta "{\"command\":\"ls\"}"})
262+
(on-stream "response.output_item.done"
263+
{:item {:type "function_call"
264+
:id "item-1"
265+
:call_id "call-1"
266+
:name "eca__shell_command"
267+
:arguments "{\"command\":\"ls\"}"}})
268+
(on-stream "response.completed"
269+
{:response {:output [{:type "function_call"
270+
:id "item-1"
271+
:call_id "call-1"
272+
:name "eca__shell_command"
273+
:arguments "{\"command\":\"ls\"}"}]
274+
:usage {:input_tokens 10
275+
:output_tokens 5}
276+
:status "completed"}})))]
277+
(llm-providers.openai/create-response!
278+
(base-provider-params)
279+
(base-callbacks
280+
{:on-prepare-tool-call (fn [data] (swap! prepare-calls* conj data))
281+
:on-tools-called (fn [tool-calls]
282+
(swap! tools-called* conj tool-calls)
283+
{:new-messages [] :tools []})}))
284+
(is (pos? (count @prepare-calls*)))
285+
(is (= "call-1" (:id (first @prepare-calls*))))
286+
(is (= "eca__shell_command" (:full-name (first @prepare-calls*))))
287+
(is (= 1 (count @tools-called*)))
288+
(is (match? [{:id "call-1"
289+
:full-name "eca__shell_command"
290+
:arguments {"command" "ls"}}]
291+
(first @tools-called*)))
292+
(is (= 2 (count @requests*)))))))
293+
294+
(deftest create-response-tool-calls-fallback-via-atom-test
295+
(testing "empty output in response.completed still triggers on-tools-called via atom fallback"
296+
(let [tools-called* (atom [])
297+
requests* (atom [])]
298+
(with-redefs [llm-providers.openai/base-responses-request!
299+
(fn [{:keys [on-stream] :as opts}]
300+
(swap! requests* conj opts)
301+
(when (= 1 (count @requests*))
302+
(on-stream "response.output_item.added"
303+
{:item {:type "function_call"
304+
:id "item-1"
305+
:call_id "call-1"
306+
:name "eca__shell_command"
307+
:arguments ""}})
308+
(on-stream "response.function_call_arguments.delta"
309+
{:item_id "item-1"
310+
:delta "{\"command\":\"ls\"}"})
311+
(on-stream "response.output_item.done"
312+
{:item {:type "function_call"
313+
:id "item-1"
314+
:call_id "call-1"
315+
:name "eca__shell_command"
316+
:arguments "{\"command\":\"ls\"}"}})
317+
;; response.completed with EMPTY output — fallback must kick in
318+
(on-stream "response.completed"
319+
{:response {:output []
320+
:usage {:input_tokens 10
321+
:output_tokens 5}
322+
:status "completed"}})))]
323+
(llm-providers.openai/create-response!
324+
(base-provider-params)
325+
(base-callbacks
326+
{:on-tools-called (fn [tool-calls]
327+
(swap! tools-called* conj tool-calls)
328+
{:new-messages [] :tools []})}))
329+
(is (= 1 (count @tools-called*)))
330+
(is (match? [{:id "call-1"
331+
:full-name "eca__shell_command"
332+
:arguments {"command" "ls"}}]
333+
(first @tools-called*)))
334+
(is (= 2 (count @requests*)))))))
335+
336+
(deftest create-response-text-only-no-phantom-calls-test
337+
(testing "text-only final response doesn't produce phantom tool calls from stale atom entries"
338+
(let [tools-called* (atom [])
339+
finish-received* (atom false)
340+
requests* (atom [])]
341+
(with-redefs [llm-providers.openai/base-responses-request!
342+
(fn [{:keys [on-stream] :as opts}]
343+
(swap! requests* conj opts)
344+
(case (count @requests*)
345+
;; First call: tool call with Copilot-style mismatched item IDs
346+
1 (do
347+
(on-stream "response.output_item.added"
348+
{:item {:type "function_call"
349+
:id "stream-added-id"
350+
:call_id "call-1"
351+
:name "eca__shell_command"
352+
:arguments ""}})
353+
(on-stream "response.function_call_arguments.delta"
354+
{:item_id "stream-added-id"
355+
:delta "{\"command\":\"ls\"}"})
356+
(on-stream "response.output_item.done"
357+
{:item {:type "function_call"
358+
:id "stream-done-id"
359+
:call_id "call-1"
360+
:name "eca__shell_command"
361+
:arguments "{\"command\":\"ls\"}"}})
362+
(on-stream "response.completed"
363+
{:response {:output [{:type "function_call"
364+
:id "output-id"
365+
:call_id "call-1"
366+
:name "eca__shell_command"
367+
:arguments "{\"command\":\"ls\"}"}]
368+
:usage {:input_tokens 10 :output_tokens 5}
369+
:status "completed"}}))
370+
;; Second call: text-only response (no tool calls)
371+
2 (on-stream "response.completed"
372+
{:response {:output [{:type "message"
373+
:id "msg-1"
374+
:content [{:text "Done."}]}]
375+
:usage {:input_tokens 5 :output_tokens 3}
376+
:status "completed"}})
377+
nil))]
378+
(llm-providers.openai/create-response!
379+
(base-provider-params)
380+
(base-callbacks
381+
{:on-message-received (fn [msg]
382+
(when (= :finish (:type msg))
383+
(reset! finish-received* true)))
384+
:on-tools-called (fn [tool-calls]
385+
(swap! tools-called* conj tool-calls)
386+
{:new-messages [] :tools []})}))
387+
(is (= 1 (count @tools-called*))
388+
"on-tools-called should fire exactly once, not for phantom calls")
389+
(is (true? @finish-received*)
390+
"text-only response should trigger :finish")
391+
(is (= 2 (count @requests*))
392+
"should make exactly 2 requests, no retry loop")))))
393+
394+
(deftest create-response-mismatched-item-ids-test
395+
(testing "different item IDs across streaming events still produce correct tool calls"
396+
(let [tools-called* (atom [])
397+
requests* (atom [])]
398+
(with-redefs [llm-providers.openai/base-responses-request!
399+
(fn [{:keys [on-stream] :as opts}]
400+
(swap! requests* conj opts)
401+
(when (= 1 (count @requests*))
402+
;; Copilot-style: different encrypted IDs for the same tool call
403+
(on-stream "response.output_item.added"
404+
{:item {:type "function_call"
405+
:id "encrypted-added-id"
406+
:call_id "call-1"
407+
:name "eca__shell_command"
408+
:arguments ""}})
409+
(on-stream "response.function_call_arguments.delta"
410+
{:item_id "encrypted-added-id"
411+
:delta "{\"command\":\"ls\"}"})
412+
;; output_item.done uses a DIFFERENT encrypted id
413+
(on-stream "response.output_item.done"
414+
{:item {:type "function_call"
415+
:id "encrypted-done-id"
416+
:call_id "call-1"
417+
:name "eca__shell_command"
418+
:arguments "{\"command\":\"ls\"}"}})
419+
;; response.completed uses yet ANOTHER encrypted id
420+
(on-stream "response.completed"
421+
{:response {:output [{:type "function_call"
422+
:id "encrypted-output-id"
423+
:call_id "call-1"
424+
:name "eca__shell_command"
425+
:arguments "{\"command\":\"ls\"}"}]
426+
:usage {:input_tokens 10 :output_tokens 5}
427+
:status "completed"}})))]
428+
(llm-providers.openai/create-response!
429+
(base-provider-params)
430+
(base-callbacks
431+
{:on-tools-called (fn [tool-calls]
432+
(swap! tools-called* conj tool-calls)
433+
{:new-messages [] :tools []})}))
434+
(is (= 1 (count @tools-called*)))
435+
(is (match? [{:id "call-1"
436+
:full-name "eca__shell_command"
437+
:arguments {"command" "ls"}}]
438+
(first @tools-called*)))
439+
(is (= 2 (count @requests*)))))))

0 commit comments

Comments
 (0)