Fix error field on tool call outputs

Author: ericdallo

docs/protocol.md

@@ -576,6 +576,11 @@ interface ToolCalledContent {
      */
     arguments: string[];
 
+    /**
+     * Whether it was a error
+     */
+    error: boolean;
+    
     /**
      * the result of the tool call.
      */
@@ -588,12 +593,7 @@ interface ToolCalledContent {
         /**
          * The content of this output
          */
-        content: string;
-        
-        /**
-         * Whether it was a error
-         */
-        error: boolean;
+        content: string; 
     }];
 }
 

src/eca/features/chat.clj

@@ -213,17 +213,18 @@
                             ;; Otherwise auto approve
                             (deliver approved?* true))
                           (if @approved?*
-                            (let [output (f.tools/call-tool! name arguments @db* config)]
+                            (let [result (f.tools/call-tool! name arguments @db* config)]
                               (add-to-history! {:role "tool_call" :content tool-call})
-                              (add-to-history! {:role "tool_call_output" :content (assoc tool-call :output output)})
+                              (add-to-history! {:role "tool_call_output" :content (assoc tool-call :output result)})
                               (swap! tool-call-args-by-id* dissoc id)
                               (send-content! chat-ctx :assistant
                                              {:type :toolCalled
                                               :origin (tool-name->origin name all-tools)
                                               :name name
                                               :arguments arguments
+                                              :error (:error result)
                                               :id id
-                                              :outputs (:contents output)})
+                                              :outputs (:contents result)})
                               {:new-messages (get-in @db* [:chats chat-id :messages])})
                             (do
                               (add-to-history! {:role "tool_call" :content tool-call})

src/eca/features/tools/mcp.clj

@@ -14,6 +14,10 @@
     McpSchema$CallToolRequest
     McpSchema$ClientCapabilities
     McpSchema$Content
+    McpSchema$GetPromptRequest
+    McpSchema$Prompt
+    McpSchema$PromptArgument
+    McpSchema$PromptMessage
     McpSchema$Root
     McpSchema$TextContent
     McpSchema$Tool
@@ -72,6 +76,26 @@
    :tools (get-in db [:mcp-clients mcp-name :tools])
    :status status})
 
+(defn ^:private list-server-tools [^ObjectMapper obj-mapper ^McpSyncClient client]
+  (mapv (fn [^McpSchema$Tool tool-client]
+          {:name (.name tool-client)
+           :description (.description tool-client)
+           ;; We convert to json to then read so we have a clojure map
+           ;; TODO avoid this converting to clojure map directly
+           :parameters (json/parse-string (.writeValueAsString obj-mapper (.inputSchema tool-client)) true)})
+        (.tools (.listTools client))))
+
+(defn ^:private list-server-prompts [^McpSyncClient client]
+  (mapv (fn [^McpSchema$Prompt prompt-client]
+          {:name (.name prompt-client)
+           :description (.description prompt-client)
+           :arguments (mapv (fn [^McpSchema$PromptArgument content]
+                              {:name (.name content)
+                               :description (.description content)
+                               :required (.required content)})
+                            (.arguments prompt-client))})
+        (.prompts (.listPrompts client))))
+
 (defn initialize-servers-async! [{:keys [on-server-updated]} db* config]
   (let [workspaces (:workspace-folders @db*)
         db @db*
@@ -89,14 +113,8 @@
                 (doseq [{:keys [name uri]} workspaces]
                   (.addRoot client (McpSchema$Root. uri name)))
                 (.initialize client)
-                (let [tools (mapv (fn [^McpSchema$Tool tool-client]
-                                    {:name (.name tool-client)
-                                     :description (.description tool-client)
-                                     ;; We convert to json to then read so we have a clojure map
-                                     ;; TODO avoid this converting to clojure map directly
-                                     :parameters (json/parse-string (.writeValueAsString obj-mapper (.inputSchema tool-client)) true)})
-                                  (.tools (.listTools client)))]
-                  (swap! db* assoc-in [:mcp-clients name :tools] tools))
+                (swap! db* assoc-in [:mcp-clients name :tools] (list-server-tools obj-mapper client))
+                (swap! db* assoc-in [:mcp-clients name :prompts] (list-server-prompts client))
                 (on-server-updated (->server name server-config :running @db*)))
               (catch Exception e
                 (logger/warn logger-tag (format "Could not initialize MCP server %s. Error: %s" name (.getMessage e)))
@@ -118,20 +136,43 @@
       (let [result (.callTool ^McpSyncClient mcp-client
                               (McpSchema$CallToolRequest. name arguments))]
         (logger/debug logger-tag "ToolCall result: " result)
-        {:contents (map (fn [content]
+        {:error (.isError result)
+         :contents (map (fn [content]
                           (case (.type ^McpSchema$Content content)
                             "text" {:type :text
-                                    :error (.isError result)
                                     :content (.text ^McpSchema$TextContent content)}
                             nil))
                         (.content result))})
       (catch Exception e
-        {:contents [{:type :text
-                     :error true
+        {:error true
+         :contents [{:type :text
                      :content (.getMessage e)}]}))))
 
+(defn get-prompt! [^String name ^Map arguments db]
+  (let [mcp-client (->> (vals (:mcp-clients db))
+                        (keep (fn [{:keys [client prompts]}]
+                                (when (some #(= name (:name %)) prompts)
+                                  client)))
+                        first)
+        prompt (.getPrompt ^McpSyncClient mcp-client (McpSchema$GetPromptRequest. name arguments))]
+    {:description (.description prompt)
+     :messages (mapv (fn [^McpSchema$PromptMessage message]
+                       {:role (.role message)
+                        :content (.content message)})
+                     (.messages prompt))}))
+
 (defn shutdown! [db*]
   (doseq [[_name {:keys [_client]}] (:mcp-clients @db*)]
     ;; TODO NoClassDefFound being thrown for some reason
     #_(.closeGracefully ^McpSyncClient client))
   (swap! db* assoc :mcp-clients {}))
+
+(comment
+  (def db* (atom user/*db*))
+  (user/with-workspace-root "/home/greg/dev/eca/eca"
+    (initialize-servers-async! {:on-server-updated println}
+                               db*
+                               {:mcpTimeoutSeconds 10
+                                :mcpServers {"fetch" {:command "docker" :args ["run" "-i" "--rm" "mcp/fetch"]}}}))
+  (:prompts (second (first (:mcp-clients @db*))))
+  (get-prompt! "fetch" {"url" "https://eca.dev"} @db*))

src/eca/features/tools/shell.clj

@@ -38,18 +38,16 @@
           (logger/debug logger-tag "Command executed:" result)
           (if (zero? (:exit result))
             (tools.util/single-text-content (:out result))
-            {:contents (remove nil?
+            {:error true
+             :contents (remove nil?
                                (concat [{:type :text
-                                         :content (str "Exit code " (:exit result))
-                                         :error true}]
+                                         :content (str "Exit code " (:exit result))}]
                                        (when-not (string/blank? err)
                                          [{:type :text
-                                           :content (str "Stderr:\n" err)
-                                           :error true}])
+                                           :content (str "Stderr:\n" err)}])
                                        (when-not (string/blank? out)
                                          [{:type :text
-                                           :content (str "Stdout:\n" out)
-                                           :error true}])))})))))
+                                           :content (str "Stdout:\n" out)}])))})))))
 
 (def definitions
   {"eca_shell_command"

src/eca/features/tools/util.clj

@@ -5,9 +5,9 @@
    [eca.shared :as shared]))
 
 (defn single-text-content [text & [error]]
-  {:contents [{:type :text
-               :content text
-               :error (boolean error)}]})
+  {:error (boolean error)
+   :contents [{:type :text
+               :content text}]})
 
 (defn workspace-roots-strs [db]
   (->> (:workspace-folders db)

test/eca/features/chat_test.clj

@@ -218,15 +218,16 @@ for allowed directories and then list files"
               (on-message-received {:type :text :text "/foo/bar"})
               (on-message-received {:type :finish}))
             :call-tool-mock
-            (constantly {:contents [{:type :text :error false :content "Allowed directories: /foo/bar"}]})})]
+            (constantly {:error false
+                         :contents [{:type :text :content "Allowed directories: /foo/bar"}]})})]
       (is (match?
            {chat-id {:id chat-id
                      :messages [{:role "user" :content "List the files you are allowed to see"}
                                 {:role "assistant" :content "Ok, working on it"}
                                 {:role "tool_call" :content {:id "call-1" :name "list_allowed_directories" :arguments {}}}
                                 {:role "tool_call_output" :content {:id "call-1" :name "list_allowed_directories" :arguments {}
-                                                                    :output {:contents [{:content "Allowed directories: /foo/bar"
-                                                                                         :error false
+                                                                    :output {:error false
+                                                                             :contents [{:content "Allowed directories: /foo/bar"
                                                                                          :type :text}]}}}
                                 {:role "assistant" :content "I can see: \n/foo/bar"}]}}
            (:chats (h/db))))
@@ -239,7 +240,7 @@ for allowed directories and then list files"
              {:role :assistant :content {:type :text :text " working on it"}}
              {:role :assistant :content {:type :toolCallPrepare :id "call-1" :name "list_allowed_directories" :arguments-text "" :manual-approval false}}
              {:role :assistant :content {:type :toolCallRun :id "call-1" :name "list_allowed_directories" :arguments {} :manual-approval false}}
-             {:role :assistant :content {:type :toolCalled :id "call-1" :name "list_allowed_directories" :arguments {} :outputs [{:content "Allowed directories: /foo/bar" :error false :type :text}]}}
+             {:role :assistant :content {:type :toolCalled :id "call-1" :name "list_allowed_directories" :arguments {} :outputs [{:content "Allowed directories: /foo/bar" :type :text}]}}
              {:role :assistant :content {:type :text :text "I can see: \n"}}
              {:role :assistant :content {:type :text :text "/foo/bar"}}
              {:role :system :content {:state :finished :type :progress}}]}

test/eca/features/tools/shell_test.clj

@@ -10,8 +10,8 @@
 (deftest shell-command-test
   (testing "inexistent working_directory"
     (is (match?
-         {:contents [{:type :text
-                      :error true
+         {:error true
+          :contents [{:type :text
                       :content (format "working directory %s does not exist" (h/file-path "/baz"))}]}
          (with-redefs [fs/exists? (constantly false)]
            ((get-in f.tools.shell/definitions ["eca_shell_command" :handler])
@@ -20,11 +20,10 @@
             {:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
   (testing "command exited with non-zero exit code"
     (is (match?
-         {:contents [{:type :text
-                      :error true
+         {:error true
+          :contents [{:type :text
                       :content "Exit code 1"}
                      {:type :text
-                      :error true
                       :content "Stderr:\nSome error"}]}
          (with-redefs [fs/exists? (constantly true)
                        shell/sh (constantly {:exit 1 :err "Some error"})]
@@ -33,8 +32,8 @@
             {:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
   (testing "command succeeds"
     (is (match?
-         {:contents [{:type :text
-                      :error false
+         {:error false
+          :contents [{:type :text
                       :content "Some text"}]}
          (with-redefs [fs/exists? (constantly true)
                        shell/sh (constantly {:exit 0 :out "Some text" :err "Other text"})]
@@ -43,8 +42,8 @@
             {:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
   (testing "command succeeds with different working directory"
     (is (match?
-         {:contents [{:type :text
-                      :error false
+         {:error false
+          :contents [{:type :text
                       :content "Some text"}]}
          (with-redefs [fs/exists? (constantly true)
                        shell/sh (constantly {:exit 0 :out "Some text" :err "Other text"})]
@@ -54,8 +53,8 @@
             {:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
   (testing "command does not fails if not in excluded config"
     (is (match?
-         {:contents [{:type :text
-                      :error false
+         {:error false
+          :contents [{:type :text
                       :content "Some text"}]}
          (with-redefs [fs/exists? (constantly true)
                        shell/sh (constantly {:exit 0 :out "Some text" :err "Other text"})]
@@ -66,8 +65,8 @@
                                             :excludeCommands ["ls" "cd"]}}}})))))
   (testing "command fails if in excluded config"
     (is (match?
-         {:contents [{:type :text
-                      :error true
+         {:error true
+          :contents [{:type :text
                       :content "Cannot run command 'rm' because it is excluded by eca config."}]}
          (with-redefs [fs/exists? (constantly true)]
            ((get-in f.tools.shell/definitions ["eca_shell_command" :handler])

test/eca/main_test.clj

@@ -31,4 +31,14 @@
   (testing "final options"
     (is (string? (:exit-message (#'main/parse ["--help"]))))
     (is (string? (:exit-message (#'main/parse ["-h"]))))
-    (is (string? (:exit-message (#'main/parse ["--version"]))))))
+    (is (string? (:exit-message (#'main/parse ["--version"])))))
+  (testing "options + commands"
+    (is (match?
+         {:action "server"
+          :options {:log-level "debug"}}
+         (#'main/parse ["--log-level" "debug" "server"]))))
+  (testing "commands + options"
+    (is (match?
+         {:action "server"
+          :options {:log-level "debug"}}
+         (#'main/parse ["server" "--log-level" "debug"])))))