Skip to content

Commit b1a15fe

Browse files
ericdalloericdallo
committed
Support timeout on eca_shell_command with default to 1min.
1 parent eb9aeef commit b1a15fe

File tree

3 files changed

+69
-38
lines changed

3 files changed

+69
-38
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
- Support timeout on `eca_shell_command` with default to 1min.
6+
57
## 0.50.2
68

79
- Fix setting the `web-search` capability in the relevant models

‎src/eca/features/tools/shell.clj‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@
1212

1313
(def ^:private logger-tag "[TOOLS-SHELL]")
1414

15+
(def ^:private default-timeout 60000)
16+
(def ^:private max-timeout (* 60000 10))
17+
1518
(defn ^:private shell-command [arguments {:keys [db]}]
1619
(let [command-args (get arguments "command")
17-
user-work-dir (get arguments "working_directory")]
20+
user-work-dir (get arguments "working_directory")
21+
timeout (min (or (get arguments "timeout") default-timeout) max-timeout)]
1822
(or (tools.util/invalid-arguments arguments [["working_directory" #(or (nil? %)
1923
(fs/exists? %)) "working directory $working_directory does not exist"]])
2024
(let [work-dir (or (some-> user-work-dir fs/canonicalize str)
@@ -25,27 +29,40 @@
2529
(config/get-property "user.home"))
2630
_ (logger/debug logger-tag "Running command:" command-args)
2731
result (try
28-
(p/shell {:dir work-dir
29-
:out :string
30-
:err :string
31-
:continue true} "bash -c" command-args)
32+
(deref (p/process {:dir work-dir
33+
:out :string
34+
:err :string
35+
:timeout timeout
36+
:continue true} "bash -c" command-args)
37+
timeout
38+
::timeout)
3239
(catch Exception e
3340
{:exit 1 :err (.getMessage e)}))
3441
err (some-> (:err result) string/trim)
3542
out (some-> (:out result) string/trim)]
36-
(logger/debug logger-tag "Command executed:" result)
37-
(if (zero? (:exit result))
38-
(tools.util/single-text-content (:out result))
39-
{:error true
40-
:contents (remove nil?
41-
(concat [{:type :text
42-
:text (str "Exit code " (:exit result))}]
43-
(when-not (string/blank? err)
44-
[{:type :text
45-
:text (str "Stderr:\n" err)}])
46-
(when-not (string/blank? out)
47-
[{:type :text
48-
:text (str "Stdout:\n" out)}])))})))))
43+
(cond
44+
(= result ::timeout)
45+
(do
46+
(logger/debug logger-tag "Command timed out after " timeout " ms")
47+
(tools.util/single-text-content (str "Command timed out after " timeout " ms") true))
48+
49+
(zero? (:exit result))
50+
(do (logger/debug logger-tag "Command executed:" result)
51+
(tools.util/single-text-content (:out result)))
52+
53+
:else
54+
(do
55+
(logger/debug logger-tag "Command executed:" result)
56+
{:error true
57+
:contents (remove nil?
58+
(concat [{:type :text
59+
:text (str "Exit code " (:exit result))}]
60+
(when-not (string/blank? err)
61+
[{:type :text
62+
:text (str "Stderr:\n" err)}])
63+
(when-not (string/blank? out)
64+
[{:type :text
65+
:text (str "Stdout:\n" out)}])))}))))))
4966

5067
(defn shell-command-summary [args]
5168
(if-let [command (get args "command")]
@@ -61,7 +78,9 @@
6178
:properties {"command" {:type "string"
6279
:description "The shell command to execute."}
6380
"working_directory" {:type "string"
64-
:description "The directory to run the command in. Default to the first workspace root."}}
81+
:description "The directory to run the command in. (Default: first workspace-root)"}
82+
"timeout" {:type "integer"
83+
:description (format "Optional timeout in milliseconds (Default: %s)" default-timeout)}}
6584
:required ["command"]}
6685
:handler #'shell-command
6786
:require-approval-fn (fn [args {:keys [db]}]

‎test/eca/features/tools/shell_test.clj‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
(:require
33
[babashka.fs :as fs]
44
[babashka.process :as p]
5-
[clojure.test :refer [deftest is testing are]]
5+
[clojure.test :refer [are deftest is testing]]
66
[eca.config :as config]
77
[eca.features.tools :as f.tools]
88
[eca.features.tools.shell :as f.tools.shell]
@@ -28,7 +28,7 @@
2828
{:type :text
2929
:text "Stderr:\nSome error"}]}
3030
(with-redefs [fs/exists? (constantly true)
31-
p/shell (constantly {:exit 1 :err "Some error"})]
31+
p/process (constantly (future {:exit 1 :err "Some error"}))]
3232
((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
3333
{"command" "ls -lh"}
3434
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
@@ -38,7 +38,7 @@
3838
:contents [{:type :text
3939
:text "Some text"}]}
4040
(with-redefs [fs/exists? (constantly true)
41-
p/shell (constantly {:exit 0 :out "Some text" :err "Other text"})]
41+
p/process (constantly (future {:exit 0 :out "Some text" :err "Other text"}))]
4242
((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
4343
{"command" "ls -lh"}
4444
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
@@ -48,12 +48,22 @@
4848
:contents [{:type :text
4949
:text "Some text"}]}
5050
(with-redefs [fs/exists? (constantly true)
51-
p/shell (constantly {:exit 0 :out "Some text" :err "Other text"})]
51+
p/process (constantly (future {:exit 0 :out "Some text" :err "Other text"}))]
5252
((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
5353
{"command" "ls -lh"
5454
"working_directory" (h/file-path "/project/foo/src")}
5555
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
56-
)
56+
(testing "command exceeds timeout"
57+
(is (match?
58+
{:error true
59+
:contents [{:type :text
60+
:text "Command timed out after 50 ms"}]}
61+
(with-redefs [fs/exists? (constantly true)
62+
p/process (constantly (future (Thread/sleep 100) {:exit 0 :err "ok"}))]
63+
((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
64+
{"command" "ls -lh"
65+
"timeout" 50}
66+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))))
5767

5868
(deftest shell-require-approval-fn-test
5969
(let [approval-fn (get-in f.tools.shell/definitions ["eca_shell_command" :require-approval-fn])
@@ -84,7 +94,7 @@
8494
:contents [{:type :text
8595
:text "Some output"}]}
8696
(with-redefs [fs/exists? (constantly true)
87-
p/shell (constantly {:exit 0 :out "Some output"})]
97+
p/process (constantly (future {:exit 0 :out "Some output"}))]
8898
((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
8999
{"command" command}
90100
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}
@@ -102,11 +112,11 @@
102112
(deftest plan-mode-approval-restrictions-test
103113
(let [all-tools [{:name "eca_shell_command" :server "eca"}]
104114
config config/initial-config]
105-
115+
106116
(testing "dangerous commands blocked in plan mode via approval"
107-
(are [command] (= :deny
108-
(f.tools/approval all-tools "eca_shell_command"
109-
{"command" command} {} config "plan"))
117+
(are [command] (= :deny
118+
(f.tools/approval all-tools "eca_shell_command"
119+
{"command" command} {} config "plan"))
110120
"echo 'test' > file.txt"
111121
"cat file.txt > output.txt"
112122
"ls >> log.txt"
@@ -121,19 +131,19 @@
121131
"npm install package"
122132
"python -c \"open('file.txt','w').write('test')\""
123133
"bash -c 'echo test > file.txt'"))
124-
134+
125135
(testing "non-dangerous commands default to ask in plan mode"
126136
(are [command] (= :ask
127-
(f.tools/approval all-tools "eca_shell_command"
128-
{"command" command} {} config "plan"))
137+
(f.tools/approval all-tools "eca_shell_command"
138+
{"command" command} {} config "plan"))
129139
"python --version" ; not matching dangerous patterns, defaults to ask
130140
"node script.js" ; not matching dangerous patterns, defaults to ask
131141
"clojure -M:test")) ; not matching dangerous patterns, defaults to ask
132-
142+
133143
(testing "safe commands not denied in plan mode"
134144
(are [command] (not= :deny
135-
(f.tools/approval all-tools "eca_shell_command"
136-
{"command" command} {} config "plan"))
145+
(f.tools/approval all-tools "eca_shell_command"
146+
{"command" command} {} config "plan"))
137147
"git status"
138148
"ls -la"
139149
"find . -name '*.clj'"
@@ -143,11 +153,11 @@
143153
"pwd"
144154
"date"
145155
"env"))
146-
156+
147157
(testing "same commands work fine in agent mode (not denied)"
148158
(are [command] (not= :deny
149-
(f.tools/approval all-tools "eca_shell_command"
150-
{"command" command} {} config "agent"))
159+
(f.tools/approval all-tools "eca_shell_command"
160+
{"command" command} {} config "agent"))
151161
"echo 'test' > file.txt"
152162
"rm file.txt"
153163
"git add ."

0 commit comments

Comments
 (0)