Skip to content

Commit d2c437a

Browse files
committed
Fix tool call approval ignoring configs for mcp servers
1 parent 7a4f497 commit d2c437a

File tree

4 files changed

+16
-13
lines changed

4 files changed

+16
-13
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+
- Fix tool call approval ignoring configs for mcp servers.
6+
57
## 0.73.2
68

79
- Fix tool call approval thread lock.

src/eca/features/tools.clj

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,13 +216,13 @@
216216
remember-to-approve?
217217
:allow
218218

219-
(some #(approval-matches? % server tool-call-name args) deny)
219+
(some #(approval-matches? % (:name server) tool-call-name args) deny)
220220
:deny
221221

222-
(some #(approval-matches? % server tool-call-name args) ask)
222+
(some #(approval-matches? % (:name server) tool-call-name args) ask)
223223
:ask
224224

225-
(some #(approval-matches? % server tool-call-name args) allow)
225+
(some #(approval-matches? % (:name server) tool-call-name args) allow)
226226
:allow
227227

228228
(legacy-manual-approval? config tool-call-name)

test/eca/features/tools/shell_test.clj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@
148148
"env")))
149149

150150
(deftest plan-mode-approval-restrictions-test
151-
(let [all-tools [{:name "eca_shell_command" :server "eca"}]
151+
(let [all-tools [{:name "eca_shell_command" :server {:name "eca"}}]
152152
config config/initial-config]
153153

154154
(testing "dangerous commands blocked in plan mode via approval"

test/eca/features/tools_test.clj

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,12 @@
9292
(#'f.tools/get-disabled-tools config "plan"))))))
9393

9494
(deftest approval-test
95-
(let [all-tools [{:name "eca_read" :server "eca"}
96-
{:name "eca_write" :server "eca"}
97-
{:name "eca_shell" :server "eca" :require-approval-fn (constantly true)}
98-
{:name "eca_plan" :server "eca" :require-approval-fn (constantly false)}
99-
{:name "request" :server "web"}
100-
{:name "download" :server "web"}]]
95+
(let [all-tools [{:name "eca_read" :server {:name "eca"}}
96+
{:name "eca_write" :server {:name "eca"}}
97+
{:name "eca_shell" :server {:name "eca"} :require-approval-fn (constantly true)}
98+
{:name "eca_plan" :server {:name "eca"} :require-approval-fn (constantly false)}
99+
{:name "request" :server {:name "web"}}
100+
{:name "download" :server {:name "web"}}]]
101101
(testing "tool has require-approval-fn which returns true"
102102
(is (= :ask (f.tools/approval all-tools "eca_shell" {} {} {} nil))))
103103
(testing "tool has require-approval-fn which returns false we ignore it"
@@ -110,7 +110,8 @@
110110
(testing "when matches allow config"
111111
(is (= :allow (f.tools/approval all-tools "request" {} {} {:toolCall {:approval {:allow {"web__request" {}}}}} nil)))
112112
(is (= :allow (f.tools/approval all-tools "eca_read" {} {} {:toolCall {:approval {:allow {"eca_read" {}}}}} nil)))
113-
(is (= :allow (f.tools/approval all-tools "request" {} {} {:toolCall {:approval {:allow {"web" {}}}}} nil))))
113+
(is (= :allow (f.tools/approval all-tools "request" {} {} {:toolCall {:approval {:allow {"web" {}}}}} nil)))
114+
(is (= :allow (f.tools/approval all-tools "eca_read" {} {} {:toolCall {:approval {:allow {"eca" {}}}}} nil))))
114115
(testing "when matches ask config"
115116
(is (= :ask (f.tools/approval all-tools "request" {} {} {:toolCall {:approval {:ask {"web__request" {}}}}} nil)))
116117
(is (= :ask (f.tools/approval all-tools "eca_read" {} {} {:toolCall {:approval {:ask {"eca_read" {}}}}} nil)))
@@ -146,8 +147,8 @@
146147
(is (= :ask (f.tools/approval all-tools "request" {} {} {} nil)))))))
147148

148149
(deftest behavior-specific-approval-test
149-
(let [all-tools [{:name "eca_shell_command" :server "eca"}
150-
{:name "eca_read_file" :server "eca"}]]
150+
(let [all-tools [{:name "eca_shell_command" :server {:name "eca"}}
151+
{:name "eca_read_file" :server {:name "eca"}}]]
151152
(testing "behavior-specific approval overrides global rules"
152153
(let [config {:toolCall {:approval {:byDefault "allow"}}
153154
:behavior {"plan" {:toolCall {:approval {:deny {"eca_shell_command" {:argsMatchers {"command" [".*rm.*"]}}}

0 commit comments

Comments
 (0)