diff --git a/CHANGELOG.md b/CHANGELOG.md index 54f7e19f0..f8c8d2e46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Add unit and integration tests covering parent↔subagent end-to-end communication so regressions like the v0.133.1 spawn-agent breakage are caught automatically. - Improve `editor_diagnostics` tool summary to show the target filename (e.g. `Checking diagnostics: foo.clj`) or `Checking all diagnostics` when no path is provided. - Bugfix: preserve the chat's selected variant when changing model on an existing chat (regression from per-chat scoping in v0.133.1). `chat/selectedModelChanged` now keeps the chat's persisted `:variant` if it is still supported by the new model, before falling back to the chat's agent variant. Resume flows (`chat/open`, `/resume`) align the same way. +- Native ECA tools now auto-resolve bare tool names like `write_file` to their canonical `eca__...` form, avoiding repeated failed retries when an LLM omits the native server prefix. ## 0.133.2 diff --git a/integration-test/integration/chat/commands_test.clj b/integration-test/integration/chat/commands_test.clj index ba254e0f0..eed2d5c83 100644 --- a/integration-test/integration/chat/commands_test.clj +++ b/integration-test/integration/chat/commands_test.clj @@ -26,7 +26,9 @@ {:name "login" :arguments [{:name "provider-id"}]} {:name "model" :arguments [{:name "full-model"}]} {:name "skills" :arguments []} - {:name "skill-create" :arguments [{:name "name"} {:name "prompt"}]} + {:name "skill-create" + :arguments [{:name "name" :description "The skill name" :required true} + {:name "prompt" :description "What to consider as this skill content" :required true}]} {:name "costs" :arguments []} {:name "compact" :arguments [{:name "additional-input"}]} {:name "fork" :arguments []} @@ -39,8 +41,10 @@ {:name "prompt-show" :arguments [{:name "optional-prompt"}]} {:name "subagents" :arguments []} {:name "plugins" :arguments []} - {:name "plugin-install" :arguments [{:name "plugin"}]} - {:name "plugin-uninstall" :arguments [{:name "plugin"}]} + {:name "plugin-install" + :arguments [{:name "plugin" :description "Plugin name or plugin@marketplace" :required true}]} + {:name "plugin-uninstall" + :arguments [{:name "plugin" :description "Plugin name" :required true}]} {:name "eca-info" :arguments nil}]} resp)))) @@ -50,7 +54,9 @@ (is (match? {:chatId nil :commands [{:name "login" :arguments [{:name "provider-id"}]} - {:name "skill-create" :arguments [{:name "name"} {:name "prompt"}]} + {:name "skill-create" + :arguments [{:name "name" :description "The skill name" :required true} + {:name "prompt" :description "What to consider as this skill content" :required true}]} {:name "costs" :arguments []} {:name "compact" :arguments [{:name "additional-input"}]} {:name "remote" :arguments []} diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index eba337860..d8bdbfa8e 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -822,7 +822,8 @@ :on-prepare-tool-call (fn [{:keys [id full-name arguments-text]}] (lifecycle/assert-chat-not-stopped! chat-ctx) (let [all-tools (f.tools/all-tools chat-id agent @db* config) - tool (tc/tool-by-full-name full-name all-tools)] + tool (f.tools/resolve-tool full-name all-tools) + resolved-full-name (or (:full-name tool) full-name)] (when-not tool (logger/warn logger-tag "Tool not found for prepare" {:full-name full-name @@ -830,10 +831,10 @@ (tc/transition-tool-call! db* chat-ctx id :tool-prepare {:name (or (:name tool) full-name) :server (:name (:server tool)) - :full-name full-name + :full-name resolved-full-name :origin (or (:origin tool) :unknown) :arguments-text arguments-text - :summary (f.tools/tool-call-summary all-tools full-name nil config @db*)}))) + :summary (f.tools/tool-call-summary all-tools resolved-full-name nil config @db*)}))) :on-tools-called (tc/on-tools-called! (assoc chat-ctx :continue-fn (fn [tc-all-tools tc-user-messages] diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj index 5add6deea..2bb604078 100644 --- a/src/eca/features/chat/tool_calls.clj +++ b/src/eca/features/chat/tool_calls.clj @@ -517,9 +517,6 @@ {:status status :actions actions})) -(defn tool-by-full-name [full-name all-tools] - (first (filter #(= full-name (:full-name %)) all-tools))) - (defn ^:private process-pre-tool-call-hook-result "Pure function: fold a single hook result into accumulated state. @@ -561,9 +558,7 @@ & [{:keys [on-before-hook-action on-after-hook-action trust] :or {on-before-hook-action (fn [_] nil) on-after-hook-action (fn [_] nil)}}]] - (let [tool (tool-by-full-name full-name all-tools) - name (:name tool) - server (:server tool) + (let [{:keys [name server] :as tool} (f.tools/resolve-tool full-name all-tools) server-name (:name server) native-tools (filter #(= :native (:origin %)) all-tools) @@ -666,12 +661,15 @@ (let [rejected-tool-call-info* (atom nil)] (run! (fn do-tool-call [{:keys [id full-name] :as tool-call}] (let [approved?* (promise) - {:keys [origin name server parameters]} (tool-by-full-name full-name all-tools) + {:keys [origin name server parameters] + :as resolved-tool} (f.tools/resolve-tool full-name all-tools) + full-name (or (:full-name resolved-tool) full-name) server-name (:name server) - tool-call (update tool-call :arguments - #(if parameters - (tools.util/omit-optional-empty-string-args parameters %) - %)) + tool-call (assoc tool-call + :full-name full-name + :arguments (if parameters + (tools.util/omit-optional-empty-string-args parameters (:arguments tool-call)) + (:arguments tool-call))) decision-plan (decide-tool-call-action tool-call all-tools @db* config agent chat-id {:on-before-hook-action (partial lifecycle/notify-before-hook-action! chat-ctx) diff --git a/src/eca/features/tools.clj b/src/eca/features/tools.clj index f7dee4cd7..c308e7092 100644 --- a/src/eca/features/tools.clj +++ b/src/eca/features/tools.clj @@ -133,26 +133,26 @@ (defn ^:private native-definitions [chat-id agent-name db config] (into - {} - (map (fn [[name tool]] - [name (-> tool - (assoc :name name) - (replace-string-values-with-vars - {:workspaceRoots (tools.util/workspace-roots-strs db) - :readFileMaxLines (get-in config [:toolCall :readFile :maxLines])}))])) - (merge {} - f.tools.filesystem/definitions - f.tools.shell/definitions - f.tools.git/definitions - f.tools.editor/definitions - f.tools.chat/definitions - f.tools.skill/definitions - f.tools.task/definitions - f.tools.background/definitions - f.tools.ask-user/definitions - (f.tools.agent/definitions config db) - (f.tools.custom/definitions config) - (f.tools.fetch-rule/definitions config db chat-id agent-name)))) + {} + (map (fn [[name tool]] + [name (-> tool + (assoc :name name) + (replace-string-values-with-vars + {:workspaceRoots (tools.util/workspace-roots-strs db) + :readFileMaxLines (get-in config [:toolCall :readFile :maxLines])}))])) + (merge {} + f.tools.filesystem/definitions + f.tools.shell/definitions + f.tools.git/definitions + f.tools.editor/definitions + f.tools.chat/definitions + f.tools.skill/definitions + f.tools.task/definitions + f.tools.background/definitions + f.tools.ask-user/definitions + (f.tools.agent/definitions config db) + (f.tools.custom/definitions config) + (f.tools.fetch-rule/definitions config db chat-id agent-name)))) (defn native-tools ([db config] @@ -171,6 +171,18 @@ [tools] (filterv #(not (contains? #{"spawn_agent" "task" "git" "ask_user"} (:name %))) tools)) +(defn resolve-tool + [tool-name all-tools] + (or (some #(when (= tool-name (:full-name %)) %) all-tools) + (when-not (string/includes? tool-name "__") + (when-let [resolved-tool (some #(when (and (= :native (:origin %)) + (= tool-name (:name %))) + %) all-tools)] + (logger/info logger-tag "Auto-resolved bare native tool name" + {:requested-name tool-name + :resolved-name (:full-name resolved-tool)}) + resolved-tool)))) + (defn all-tools "Returns all available tools, including both native ECA tools (like filesystem and shell tools) and tools provided by MCP servers. @@ -211,11 +223,13 @@ state-transition-fn ; params: event & event-data {:keys [trust]}] (logger/info logger-tag (format "Calling tool '%s' with args '%s'" full-name arguments)) - (let [[server-name tool-name] (string/split full-name #"__") - arguments (update-keys arguments clojure.core/name) + (let [arguments (update-keys arguments clojure.core/name) db @db* all-tools (all-tools chat-id agent-name db config) - tool-meta (some #(when (= full-name (:full-name %)) %) all-tools) + tool-meta (resolve-tool full-name all-tools) + resolved-full-name (:full-name tool-meta full-name) + server-name (get-in tool-meta [:server :name]) + tool-name (:name tool-meta) arguments (if-let [parameters (:parameters tool-meta)] (tools.util/omit-optional-empty-string-args parameters arguments) arguments) @@ -224,7 +238,6 @@ (try (when-not tool-meta (throw (ex-info (format "Tool '%s' not found" full-name) {:full-name full-name - :server-name server-name :arguments arguments :all-tools (mapv :full-name all-tools)}))) (let [result (-> (if required-args-error @@ -249,7 +262,7 @@ :metrics metrics}))) (tools.util/maybe-truncate-output config tool-call-id))] (logger/debug logger-tag "Tool call result: " result) - (metrics/count-up! "tool-called" {:name full-name :error (:error result)} metrics) + (metrics/count-up! "tool-called" {:name resolved-full-name :error (:error result)} metrics) (if-let [r (:rollback-changes result)] (do (swap! db* assoc-in [:chats chat-id :tool-calls tool-call-id :rollback-changes] r) @@ -376,8 +389,7 @@ :on-server-removed (partial notify-server-removed metrics messenger)}))) (defn tool-call-summary [all-tools full-name args config db] - (when-let [summary-fn (:summary-fn (first (filter #(= full-name (:full-name %)) - all-tools)))] + (when-let [summary-fn (:summary-fn (resolve-tool full-name all-tools))] (try (summary-fn {:args args :config config diff --git a/test/eca/features/tools_test.clj b/test/eca/features/tools_test.clj index 79809a750..527b5dffa 100644 --- a/test/eca/features/tools_test.clj +++ b/test/eca/features/tools_test.clj @@ -354,6 +354,18 @@ "git add ." "python --version")))) +(deftest resolve-tool-test + (let [native-tool {:name "write_file" :full-name "eca__write_file" :server {:name "eca"} :origin :native} + mcp-tool {:name "write_file" :full-name "vendor__write_file" :server {:name "vendor"} :origin :mcp} + all-tools [native-tool mcp-tool]] + (testing "resolves exact full name" + (is (= native-tool (f.tools/resolve-tool "eca__write_file" all-tools))) + (is (= mcp-tool (f.tools/resolve-tool "vendor__write_file" all-tools)))) + (testing "resolves bare native ECA tool names" + (is (= native-tool (f.tools/resolve-tool "write_file" all-tools)))) + (testing "does not resolve bare names to MCP tools" + (is (nil? (f.tools/resolve-tool "missing_tool" all-tools)))))) + (deftest call-tool!-test (testing "INVALID_ARGS for missing required param on native tool" (is (match? @@ -381,7 +393,38 @@ (h/metrics) identity identity - nil)))))) + nil))))) + (testing "bare native tool names dispatch to native ECA tools" + (let [called-args* (atom nil)] + (is (match? + {:error false + :contents [{:type :text :text "OK"}]} + (with-redefs [f.tools.filesystem/definitions + {"test_native_tool" + {:description "Test tool" + :parameters {"type" "object" + :properties {"path" {:type "string"}} + :required ["path"]} + :handler (fn [args _ctx] + (reset! called-args* args) + {:error false + :contents [{:type :text :text "OK"}]})}} + f.mcp/call-tool! (fn [& _] + (throw (ex-info "MCP should not be called" {})))] + (f.tools/call-tool! + "test_native_tool" + {"path" "/tmp/foo"} + "chat-1" + "call-1" + "code" + (h/db*) + (h/config) + (h/messenger) + (h/metrics) + identity + identity + nil)))) + (is (= {"path" "/tmp/foo"} @called-args*))))) (deftest call-tool!-mcp-missing-required-test (testing "INVALID_ARGS for missing required param on MCP tool"