Skip to content

Added ability to cancel tool calls. #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ericdallo merged 5 commits into from
Oct 6, 2025
Merged

Added ability to cancel tool calls. #145

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd929da
Added ability to cancel tool calls.
bombaywalla Oct 3, 2025
93a518a
Updated CHANGELOG.
bombaywalla Oct 3, 2025
7473ce8
Improve message when not starting a shell process.
bombaywalla Oct 3, 2025
20d261f
Merged master
bombaywalla Oct 4, 2025
1fe5aac
Merge master
bombaywalla Oct 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- Added ability to cancel tool calls. Only the shell tool
currently. #145

## 0.63.3

- Fix last word going after tool call for openai-chat API.
Expand Down
169 changes: 124 additions & 45 deletions src/eca/features/chat.clj
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions src/eca/features/tools.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@
[name arguments details result]
(tools.util/tool-call-details-after-invocation name arguments details result))

(defn tool-call-destroy-resource!
"Destroy the resource in the tool call named `name`."
[name resource-kwd resource]
(tools.util/tool-call-destroy-resource! name resource-kwd resource))

(defn refresh-tool-servers!
"Updates all tool servers (native and MCP) with new behavior status."
[tool-status-fn db* messenger config]
Expand Down
46 changes: 37 additions & 9 deletions src/eca/features/tools/shell.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
(def ^:private default-timeout 60000)
(def ^:private max-timeout (* 60000 10))

(defn ^:private shell-command [arguments {:keys [db]}]
(defn ^:private shell-command [arguments {:keys [db tool-call-id call-state-fn state-transition-fn]}]
(let [command-args (get arguments "command")
user-work-dir (get arguments "working_directory")
timeout (min (or (get arguments "timeout") default-timeout) max-timeout)]
Expand All @@ -29,15 +29,36 @@
(config/get-property "user.home"))
_ (logger/debug logger-tag "Running command:" command-args)
result (try
(deref (p/process {:dir work-dir
:out :string
:err :string
:timeout timeout
:continue true} "bash -c" command-args)
timeout
::timeout)
(let [proc (when-not (= :stopping (:status (call-state-fn)))
(p/process {:dir work-dir
:out :string
:err :string
:timeout timeout
:continue true} "bash -c" command-args))]
(when proc
Copy link
Member

@ericdallo ericdallo Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will happen when this comes nil? I believe result will be nil as well? is that expected?

Copy link
Contributor Author

@bombaywalla bombaywalla Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.

When proc is nil, that would mean that the tool call is in the :stopping status and so we don't want to start another process. As you say, result will be nil. The :else clause of the cond will trigger and an error will be reported. Admittedly, the error message will be less than useful.

Would it be better to have result be set to {:exit 1 :err "Tool call is :stopping, so shell process not started"}? Or would you prefer a different way to handle this?

Copy link
Member

@ericdallo ericdallo Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum, not sure, we certainly don't want to respond to LLM right? so it doesn't need to have a response with the exit/err format, maybe we should just throw a exception?

Copy link
Contributor Author

@bombaywalla bombaywalla Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I don't follow. If we throw an Exception, it will be caught right there and will return an exit/err anyways. What am I missing?

Why should we not respond to the LLM? It asked for a tool call, so should we not at least send a response saying why we are not running the call? I'm not familiar enough with what the LLMs would expect in such a case, but my guess would be that some feedback would be helpful.

My guess is that what I suggested with the exit/err is a reasonable way to proceed. But let me know if you'd prefer a different approach.

Copy link
Member

@ericdallo ericdallo Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are stopping, we don't want to request LLM again right? because the LLM loop are just requests and responses, if we are :stopping, we don't want to continure this loop (answer LLM), does that makes sense?

Copy link
Contributor Author

@bombaywalla bombaywalla Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code was meant to be defensive in the following situation.

The LLM has requested a call. However, before the process was started, there was a stop request and so the call has transitioned to :stopping. Now, in the tool call future, just before spawning the process, the call checks to see what its status is. If it is :stopping, it will not spawn off the process.

Currently, the only way to request a stop is via stopping the whole prompt and the whole LLM loop.
We have also talked about the situation where the user might want to stop a particular tool call and continue with the rest. In that case, I would guess that we do want to continue the LLM loop with the remaining calls, right?

I was trying to be consistent with what happens if, for whatever reason (say reaching a process limit), the process cannot be spawned.

Perhaps you can explain more what you meant by throwing an Exception. In particular, where you expect this Exception would be handled.

Copy link
Member

@ericdallo ericdallo Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would guess that we do want to continue the LLM loop with the remaining calls, right?

Good question, currently we don't have the option to stop one tool call so I don't know what would happen if 3 tool calls are running and user stop 1, what we should do? but yeah, maybe you are right, we want to respond that user stopped the tool call and respond to LLM , so I think it's ok to return that err/exit structure you mentioned.

Perhaps you can explain more what you meant by throwing an Exception. In particular, where you expect this Exception would be handled.

Nvm, I was thinking in a case user stop the prompt, but not stopping a single tool call, I just think that:

  • if there is a single tool call and user stop it, we should stop the loop and user types what LLM should do differently.
  • but if there are multiple tool calls, not sure we want to stop the loop.

(state-transition-fn :resources-created {:resources {:process proc}})
(try (deref proc
timeout
::timeout)
(catch InterruptedException e
(let [msg (or (.getMessage e) "Shell tool call was interrupted")]
(logger/debug logger-tag "Shell tool call was interrupted" {:tool-call-id tool-call-id :message msg})
(tools.util/tool-call-destroy-resource! "eca_shell_command" :process proc)
(state-transition-fn :resources-destroyed {:resources [:process]})
{:exit 1 :err msg})))))
(catch Exception e
{:exit 1 :err (.getMessage e)}))
;; Process did not start, or had an Exception (other than InterruptedException) during execution.
(let [msg (or (.getMessage e) "Caught an Exception during execution of the shell tool")]
(logger/warn logger-tag "Got an Exception during execution" {:message msg})
{:exit 1 :err msg}))
(finally
;; If any resources remain, destroy them.
(let [state (call-state-fn)]
(when-let [resources (:resources state)]
(doseq [[res-kwd res] resources]
(tools.util/tool-call-destroy-resource! "eca_shell_command" res-kwd res))
(when (#{:executing :stopping} (:status state))
(state-transition-fn :resources-destroyed {:resources (keys resources)}))))))
err (some-> (:err result) string/trim)
out (some-> (:out result) string/trim)]
(cond
Expand Down Expand Up @@ -90,3 +111,10 @@
(let [workspace-roots (mapv (comp shared/uri->filename :uri) (:workspace-folders db))]
(not-any? #(fs/starts-with? wd %) workspace-roots)))))
:summary-fn #'shell-command-summary}})

(defmethod tools.util/tool-call-destroy-resource! :eca_shell_command [name resource-kwd resource]
(logger/debug logger-tag "About to destroy resource" {:resource-kwd resource-kwd})
(case resource-kwd
:process (p/destroy-tree resource)
(logger/warn logger-tag "Unknown resource keyword" {:tool-name name
:resource-kwd resource-kwd})))
8 changes: 8 additions & 0 deletions src/eca/features/tools/util.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
(defmethod tool-call-details-after-invocation :default [_name _arguments details _result]
details)

(defmulti tool-call-destroy-resource!
Copy link
Member

@ericdallo ericdallo Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

"Destroy the tool call resource."
(fn [name _resource-kwd _resource] (keyword name)))

(defmethod tool-call-destroy-resource! :default [_name _resource-kwd _resource]
;; By default, do nothing
)

(defn single-text-content [text & [error]]
{:error (boolean error)
:contents [{:type :text
Expand Down
108 changes: 55 additions & 53 deletions test/eca/features/chat_tool_call_state_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@
"Expected state machine to contain [:execution-approved :stop-requested] transition")
(is (contains? state-machine [:executing :stop-requested])
"Expected state machine to contain [:executing :stop-requested] transition")
(is (contains? state-machine [:stopping :stop-requested])
"Expected state machine to contain [:stopping :stop-requested] transition")
(is (contains? state-machine [:cleanup :stop-requested])
"Expected state machine to contain [:cleanup :stop-requested] transition")

;; Note: :rejected, :completed and :stopped are terminal states, so no stop transitions.
(is (not (contains? state-machine [:stopped :stop-requested]))
"Expected :rejected state to not have stop transition defined, since it is a terminal state")
;; Note: :rejected and :completed are terminal states, so no stop transitions.
(is (not (contains? state-machine [:rejected :stop-requested]))
"Expected :rejected state to not have stop transition defined, since it is a terminal state")
(is (not (contains? state-machine [:completed :stop-requested]))
Expand Down Expand Up @@ -259,7 +261,7 @@
(let [result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :tool-run run-event-data)]

(is (match? {:status :check-approval
:actions [:init-arguments :init-approval-promise :send-toolCallRun]}
:actions [:init-arguments :init-approval-promise :init-future-cleanup-promise :send-toolCallRun]}
result)
"Expected next state to be :check-approval with actions of :init-approval-promise and :send-toolCallRun")

Expand Down Expand Up @@ -314,7 +316,7 @@
;; Step 2: :preparing -> :check-approval
(let [result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :tool-run run-event-data)]
(is (match? {:status :check-approval
:actions [:init-arguments :init-approval-promise :send-toolCallRun]}
:actions [:init-arguments :init-approval-promise :init-future-cleanup-promise :send-toolCallRun]}
result)
"Expected transition to :check-approval with init promise and send run actions")

Expand Down Expand Up @@ -449,31 +451,31 @@
(let [result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :execution-start
{:name "list_files" :origin "filesystem" :arguments {:path "/tmp"}})]
(is (match? {:status :executing
:actions [:set-start-time :set-call-future :send-toolCallRunning :send-progress]}
:actions [:set-start-time :add-future :send-toolCallRunning :send-progress]}
result)
"Expected transition to :executing status with no additional actions")

(let [tool-state (#'f.chat/get-tool-call-state @db* chat-id tool-call-id)]
(is (= :executing (:status tool-state))
"Expected tool call state to be in :executing status"))))

;; Step 2: :executing -> :completed
(testing ":executing -> :completed transition"
;; Step 2: :executing -> :cleanup
(testing ":executing -> :cleanup transition"
(let [result-data {:outputs "file1.txt\nfile2.txt\nfile3.txt"
:error nil
:name "list_files"
:origin "filesystem"
:arguments {:path "/tmp"}}
result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :execution-end result-data)]

(is (match? {:status :completed
:actions [:send-toolCalled :log-metrics :send-progress]}
(is (match? {:status :cleanup
:actions [:deliver-future-cleanup-completed :send-toolCalled :log-metrics :send-progress]}
result)
"Expected transition to :completed with send toolCalled and record metrics actions")
"Expected transition to :cleanup with send toolCalled and record metrics actions")

(let [tool-state (#'f.chat/get-tool-call-state @db* chat-id tool-call-id)]
(is (= :completed (:status tool-state))
"Expected tool call state to be in :completed status"))
(is (= :cleanup (:status tool-state))
"Expected tool call state to be in :cleanup status"))

(let [messages (h/messages)
chat-messages (:chat-content-received messages)
Expand All @@ -499,15 +501,15 @@
chat-id "test-chat"
chat-ctx {:chat-id chat-id :request-id "req-1" :messenger (h/messenger)}]

;; Test :initial -> :stopped
(testing ":initial -> :stopped"
;; Test :initial -> :cleanup
(testing ":initial -> :cleanup"
(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-initial" :stop-requested)]
(is (match? {:status :stopped :actions []} result)
"Expected transition from :initial to :stopped with no actions")))
(is (match? {:status :cleanup :actions []} result)
"Expected transition from :initial to :cleanup with no actions")))

;; Test :preparing -> :stopped (already covered in transition-tool-call-stop-transitions-test)
;; Test :check-approval -> :stopped
(testing ":check-approval -> :stopped"
;; Test :preparing -> :cleanup (already covered in transition-tool-call-stop-transitions-test)
;; Test :check-approval -> :cleanup
(testing ":check-approval -> :cleanup"
(let [approved?* (promise)]
(#'f.chat/transition-tool-call! db* chat-ctx "tool-check" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})
Expand All @@ -520,8 +522,8 @@
result)
"Expected transition from :check-approval to :rejected with relevant actions"))))

;; Test :executing -> :stopping -> :stopped
(testing ":executing -> :stopping -> :stopped"
;; Test :executing -> :stopping -> :cleanup
(testing ":executing -> :stopping -> :cleanup"
(let [approved?* (promise)]
(#'f.chat/transition-tool-call! db* chat-ctx "tool-executing" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})
Expand All @@ -533,18 +535,18 @@

(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-executing" :stop-requested)]
(is (match? {:status :stopping
:actions []}
:actions [:cancel-future]}
result)
"Expected transition from :executing to :stopping with relevant actions"))
(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-executing" :stop-attempted)]
(is (match? {:status :stopped
:actions [:send-toolCallRejected]}
(is (match? {:status :cleanup
:actions [:deliver-future-cleanup-completed :send-toolCallRejected]}
result)
"Expected transition from :stopping to :stopped with relevant actions"))))
"Expected transition from :stopping to :cleanup with relevant actions"))))


;; Test :completed -> :stopped (should be no-op or error)
(testing ":completed -> :stopped (should handle gracefully)"
;; Test :cleanup -> :cleanup (should be no-op or error)
(testing ":cleanup -> :cleanup (should handle gracefully)"
(let [approved?* (promise)]
(#'f.chat/transition-tool-call! db* chat-ctx "tool-completed" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})
Expand All @@ -556,15 +558,15 @@
(#'f.chat/transition-tool-call! db* chat-ctx "tool-completed" :execution-end
{:outputs "success" :error nil :name "test" :origin "test" :arguments {}})

;; Now try to stop a completed tool call
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"Invalid state transition"
(#'f.chat/transition-tool-call! db* chat-ctx "tool-completed" :stop-requested))
"Expected exception as completed tool calls cannot be stopped")))
;; Now try to stop a tool call in :cleanup
(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-completed" :stop-requested)]
(is (match? {:status :cleanup
:actions []}
result)
"Expected a transition from :cleanup on :stop-requested to be a nop"))))

;; Test :rejected -> :stopped
(testing ":rejected -> :stopped (should handle gracefully)"
;; Test :rejected -> :cleanup
(testing ":rejected -> :cleanup (should handle gracefully)"
(let [approved?* (promise)]
(#'f.chat/transition-tool-call! db* chat-ctx "tool-rejected" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})
Expand All @@ -589,17 +591,17 @@
chat-id "test-chat"
chat-ctx {:chat-id chat-id :request-id "req-1" :messenger (h/messenger)}]

(testing ":preparing -> :stopped"
(testing ":preparing -> :cleanup"
(#'f.chat/transition-tool-call! db* chat-ctx "tool-1" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})

(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-1" :stop-requested)]
(is (match? {:status :stopped
(is (match? {:status :cleanup
:actions [:set-decision-reason :send-toolCallRejected]}
result)
"Expected transition to :stopped with send toolCallRejected action")
(is (= :stopped (:status (#'f.chat/get-tool-call-state @db* chat-id "tool-1")))
"Expected tool call state to be in :stopped status")))
"Expected transition to :cleanup with send toolCallRejected action")
(is (= :cleanup (:status (#'f.chat/get-tool-call-state @db* chat-id "tool-1")))
"Expected tool call state to be in :cleanup status")))

(testing ":waiting-approval -> :rejected"
(let [approved?* (promise)]
Expand All @@ -620,7 +622,7 @@
(is (= false (deref approved?* 100 :timeout))
"Expected promise to be delivered with false value"))))

(testing ":execution-approved -> :stopped"
(testing ":execution-approved -> :cleanup"
(let [approved?* (promise)]
(#'f.chat/transition-tool-call! db* chat-ctx "tool-3" :tool-prepare
{:name "test" :origin "test" :arguments-text "{}"})
Expand All @@ -629,12 +631,12 @@
(#'f.chat/transition-tool-call! db* chat-ctx "tool-3" :approval-allow)

(let [result (#'f.chat/transition-tool-call! db* chat-ctx "tool-3" :stop-requested)]
(is (match? {:status :stopped
(is (match? {:status :cleanup
:actions [:send-toolCallRejected]}
result)
"Expected transition to :stopped with send toolCallRejected action")
(is (= :stopped (:status (#'f.chat/get-tool-call-state @db* chat-id "tool-3")))
"Expected tool call state to be in :stopped status")))))))
"Expected transition to :cleanup with send toolCallRejected action")
(is (= :cleanup (:status (#'f.chat/get-tool-call-state @db* chat-id "tool-3")))
"Expected tool call state to be in :cleanup status")))))))

(deftest test-stop-prompt-messages
;; Test what messages are sent when stop-prompt is called
Expand Down Expand Up @@ -827,7 +829,7 @@

(deftest transition-tool-call-stop-during-execution-test
;; Test stopping a tool call during execution
(testing ":executing -> :stopping -> :stopped transition"
(testing ":executing -> :stopping -> :cleanup transition"
(h/reset-components!)
(let [db* (h/db*)
chat-id "test-chat"
Expand Down Expand Up @@ -856,8 +858,8 @@
result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :stop-attempted)]
(is (= :stopping (:status tool-state))
"Expected tool call state to be in :stopping status")
(is (= :stopped (:status result))
"Expected tool call state to be :stopped after :stop-requested"))
(is (= :cleanup (:status result))
"Expected tool call state to be :cleanup after :stop-requested"))
)))

(deftest transition-tool-call-nonexistent-tool-call-operations-test
Expand Down Expand Up @@ -894,18 +896,18 @@
(#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :execution-start
{:name "test" :origin "test" :arguments {}})

(testing ":executing -> :completed with error"
(testing ":executing -> :cleanup with error"
(let [error-result {:outputs nil
:error "File not found: /nonexistent/path"
:name "test"
:origin "test"
:arguments {}}
result (#'f.chat/transition-tool-call! db* chat-ctx tool-call-id :execution-end error-result)]

(is (match? {:status :completed
:actions [:send-toolCalled :log-metrics :send-progress]}
(is (match? {:status :cleanup
:actions [:deliver-future-cleanup-completed :send-toolCalled :log-metrics :send-progress]}
result)
"Expected transition to :completed with send toolCalled and record metrics actions")
"Expected transition to :cleanup with send toolCalled and record metrics actions")

(let [messages (h/messages)
chat-messages (:chat-content-received messages)
Expand Down
Loading
Loading