Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Guard subagent activity labels against overly long model-generated text.
- 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.

Expand Down
18 changes: 13 additions & 5 deletions src/eca/features/chat/tool_calls.clj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
[eca.features.chat.lifecycle :as lifecycle]
[eca.features.hooks :as f.hooks]
[eca.features.tools :as f.tools]
[eca.features.tools.agent :as f.tools.agent]
[eca.features.tools.mcp :as f.tools.mcp]
[eca.features.tools.util :as tools.util]
[eca.llm-util :as llm-util]
Expand Down Expand Up @@ -666,19 +667,26 @@
(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)
tool (tool-by-full-name full-name all-tools)
{:keys [origin name server parameters]} tool
server-name (:name server)
spawn-agent? (and (= :native origin)
(= "eca" server-name)
(= "spawn_agent" name))
tool-call (update tool-call :arguments
#(if parameters
(tools.util/omit-optional-empty-string-args parameters %)
%))
#(cond-> (if parameters
(tools.util/omit-optional-empty-string-args parameters %)
%)
spawn-agent? f.tools.agent/normalize-arguments))
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)
:on-after-hook-action (partial lifecycle/notify-after-hook-action! chat-ctx)
:trust (get-in @db* [:chats chat-id :trust])})
{:keys [decision arguments hook-rejected? reason hook-continue
{:keys [decision hook-rejected? reason hook-continue
hook-stop-reason arguments-modified?]} decision-plan
arguments (cond-> (:arguments decision-plan)
spawn-agent? f.tools.agent/normalize-arguments)
_ (when arguments-modified?
(lifecycle/send-content! chat-ctx :system {:type :hookActionFinished
:action-type "shell"
Expand Down
30 changes: 24 additions & 6 deletions src/eca/features/tools/agent.clj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,20 @@
(set! *warn-on-reflection* true)

(def ^:private logger-tag "[AGENT-TOOL]")
(def ^:private activity-summary-max-length 40)

(defn normalize-arguments
"Normalize spawn_agent arguments before display, history, and invocation."
[arguments]
(let [activity (when (string? (get arguments "activity"))
(-> (get arguments "activity")
str/trim
(str/replace #"\s+" " ")))]
(if (str/blank? activity)
(dissoc arguments "activity")
(assoc arguments "activity" (if (> (count activity) activity-summary-max-length)
(str (subs activity 0 activity-summary-max-length) "...")
activity)))))

(defn ^:private all-agents
[config]
Expand Down Expand Up @@ -64,7 +78,9 @@
:name "spawn_agent"
:server "eca"
:origin "native"
:summary (format "%s: %s" agent-name activity)
:summary (if activity
(format "%s: %s" agent-name activity)
agent-name)
:arguments arguments
:details (cond-> {:type :subagent
:subagent-chat-id subagent-chat-id
Expand Down Expand Up @@ -107,9 +123,10 @@
"Handler for the spawn_agent tool.
Spawns a subagent to perform a focused task and returns the result."
[arguments {:keys [db* config messenger metrics chat-id tool-call-id call-state-fn trust]}]
(let [agent-name (get arguments "agent")
(let [arguments (normalize-arguments arguments)
agent-name (get arguments "agent")
task (get arguments "task")
activity (get arguments "activity" "working")
activity (get arguments "activity")
db @db*

;; Check for nesting - prevent subagents from spawning other subagents
Expand Down Expand Up @@ -267,12 +284,13 @@
:description "Optional sub-agent model override. Reserved for explicit user override only. Omit unless the user explicitly named a model."}
"variant" {:type "string"
:description "Optional sub-agent model variant override. Reserved for explicit user override only. Omit unless the user explicitly named a variant."}}
:required ["agent" "task" "activity"]}
:required ["agent" "task"]}
:handler #'spawn-agent
:summary-fn (fn [{:keys [args]}]
(if-let [agent-name (get args "agent")]
(let [activity (get args "activity" "working")]
(format "%s: %s" agent-name activity))
(if-let [activity (get (normalize-arguments args) "activity")]
(format "%s: %s" agent-name activity)
agent-name)
"Spawning agent"))}})

(defmethod tools.util/tool-call-details-before-invocation :spawn_agent
Expand Down
53 changes: 50 additions & 3 deletions test/eca/features/tools/agent_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,48 @@
(defn ^:private spawn-handler []
(get-in (f.tools.agent/definitions test-config test-db) ["spawn_agent" :handler]))

(defn ^:private spawn-summary [args]
((get-in (f.tools.agent/definitions test-config test-db) ["spawn_agent" :summary-fn]) {:args args}))

(deftest spawn-agent-activity-summary-test
(testing "normal activity label is unchanged"
(is (= "explorer: searching files"
(spawn-summary {"agent" "explorer" "activity" "searching files"}))))

(testing "whitespace and newlines are collapsed"
(is (= "explorer: searching files"
(spawn-summary {"agent" "explorer" "activity" " searching\n\t files "}))))

(testing "long activity label is truncated"
(let [long-label (apply str (repeat 80 "a"))]
(is (= (str "explorer: " (apply str (repeat 40 "a")) "...")
(spawn-summary {"agent" "explorer" "activity" long-label})))))

(testing "blank activity omits summary suffix"
(is (= "explorer"
(spawn-summary {"agent" "explorer" "activity" " \n "})))
(is (= "explorer"
(spawn-summary {"agent" "explorer"})))))

(deftest spawn-agent-normalize-arguments-test
(is (= {"agent" "explorer" "task" "find" "activity" "searching files"}
(f.tools.agent/normalize-arguments {"agent" "explorer"
"task" "find"
"activity" " searching\nfiles "})))
(is (= {"agent" "explorer" "task" "find"}
(f.tools.agent/normalize-arguments {"agent" "explorer"
"task" "find"
"activity" ""})))
(is (= {"agent" "explorer" "task" "find"}
(f.tools.agent/normalize-arguments {"agent" "explorer"
"task" "find"
"activity" ["not" "string"]})))
(is (= {"agent" "explorer" "task" "find" "activity" "searching files"}
(f.tools.agent/normalize-arguments
(f.tools.agent/normalize-arguments {"agent" "explorer"
"task" "find"
"activity" " searching\nfiles "})))))

(deftest spawn-agent-not-found-test
(testing "throws when agent is not found"
(let [db* (atom {:chats {"chat-1" {:id "chat-1"}}})
Expand Down Expand Up @@ -589,7 +631,7 @@
"activity" {:type "string"}
"model" {:type "string"}
"variant" {:type "string"}}
:required ["agent" "task" "activity"]}
:required ["agent" "task"]}
(:parameters tool)))))

(testing "model and variant enums are absent when no models in db"
Expand Down Expand Up @@ -663,12 +705,17 @@
:chat-id "parent-1"
:tool-call-id "tc-1"
:call-state-fn (constantly {:status :executing})}))
result (deref result-fut 5000 ::timeout)]
result (deref result-fut 30000 ::timeout)
timeout-details (when (identical? ::timeout result)
(pr-str {:parent-chat (get-in @(h/db*) [:chats "parent-1"])
:subagent-chat (get-in @(h/db*) [:chats "subagent-tc-1"])
:messages (h/messages)}))]
(when (identical? ::timeout result)
(future-cancel result-fut))
(testing "spawn handler completes (regression would hang the polling loop)"
(is (not (identical? ::timeout result))
"spawn handler did not complete in 5s — chat/prompt likely rejected the subagent chat-id"))
(str "spawn handler did not complete in 30s — chat/prompt likely rejected the subagent chat-id. "
timeout-details)))
(when (map? result)
(testing "spawn handler returns success (would be :error true under v0.133.1)"
(is (match? {:error false
Expand Down
Loading