diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ce30af16..a39d79cf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Remote REST API now surfaces tool calls awaiting approval: `GET /api/v1/chats/:id` returns `pendingToolCalls` and session/list-chats entries include `pendingApprovalCount`. + ## 0.136.2 - Add `chat.defaultTrust` config to start new chats in trust mode (auto-accepting tool calls) across all editors. diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj index 77ca5e90a..ef7ca0806 100644 --- a/src/eca/features/chat/tool_calls.clj +++ b/src/eca/features/chat/tool_calls.clj @@ -397,7 +397,8 @@ ;; :status (keyword) is initialized by the state transition machinery ;; :approved?* (promise) is initialized by the :init-approval-promise action ;; :future-cleanup-complete?* (promise) is initialized by the :init-future-cleanup-promise action - ;; :arguments (map) is initialized by the :init-arguments action + ;; :arguments (map), :summary, :details and :manual-approval are + ;; initialized by the :init-arguments action ;; :start-time (long) is initialized by the :set-start-time action ;; :future (future) is initialized by the :add-future action ;; :resources (map) is updated by the :add-resources and remove-resources actions @@ -419,8 +420,12 @@ (:future-cleanup-complete?* event-data)) :init-arguments - (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :arguments] - (:arguments event-data)) + (swap! db* update-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id] + #(-> % + (assoc :arguments (:arguments event-data)) + (assoc-some :summary (:summary event-data) + :details (:details event-data) + :manual-approval (:manual-approval event-data)))) :set-decision-reason (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :decision-reason] diff --git a/src/eca/remote/handlers.clj b/src/eca/remote/handlers.clj index 60f38efaf..4e763fe6f 100644 --- a/src/eca/remote/handlers.clj +++ b/src/eca/remote/handlers.clj @@ -47,6 +47,38 @@ (defn ^:private camel-keys [m] (shared/map->camel-cased-map m)) +(defn ^:private waiting-approval? [[_ tc]] + (= :waiting-approval (:status tc))) + +(defn ^:private pending-tool-calls + "Tool calls in :waiting-approval, shaped for the REST API so clients that + missed the toolCallRun SSE event can still render the approval card." + [chat] + (->> (:tool-calls chat) + (filter waiting-approval?) + (mapv (fn [[id tc]] + (shared/assoc-some + {:id id + :name (:name tc) + :server (:server tc) + :origin (:origin tc) + :arguments (:arguments tc) + :manual-approval (boolean (:manual-approval tc))} + :summary (:summary tc) + :details (:details tc)))))) + +(defn ^:private pending-approval-count [chat] + (->> (:tool-calls chat) (filter waiting-approval?) count)) + +(defn ^:private chat-summary [chat] + (camel-keys + {:id (:id chat) + :title (:title chat) + :status (or (:status chat) :idle) + :created-at (:created-at chat) + :updated-at (:updated-at chat) + :pending-approval-count (pending-approval-count chat)})) + (defn ^:private session-state "Builds the session state map used for both GET /session and SSE session:connected." [db config] @@ -74,13 +106,7 @@ (->> (vals (:chats db)) (remove :subagent) (filter #(get editor-open (:id %))) - (mapv (fn [chat] - (camel-keys - {:id (:id chat) - :title (:title chat) - :status (or (:status chat) :idle) - :created-at (:created-at chat) - :updated-at (:updated-at chat)}))))) + (mapv chat-summary))) :startedAt (when-let [ms (:started-at db)] (.toString (Instant/ofEpochMilli ^long ms))) :welcomeMessage (handlers/welcome-message config) @@ -112,12 +138,7 @@ chats (->> (vals (:chats db)) (remove :subagent) (filter #(get editor-open (:id %))) - (mapv (fn [{:keys [id title status created-at updated-at]}] - {:id id - :title title - :status (or status :idle) - :createdAt created-at - :updatedAt updated-at})))] + (mapv chat-summary))] (json-response chats))) (defn handle-get-chat [{:keys [db*]} _request chat-id] @@ -130,7 +151,8 @@ :created-at (:created-at chat) :updated-at (:updated-at chat) :messages (or (:messages chat) []) - :task (:task chat)})) + :task (:task chat) + :pending-tool-calls (pending-tool-calls chat)})) (error-response 404 "chat_not_found" (str "Chat " chat-id " does not exist")))) (defn handle-prompt [{:keys [db*] :as components} request chat-id] diff --git a/test/eca/remote/handlers_test.clj b/test/eca/remote/handlers_test.clj index 4137e526a..cb1ae1b7f 100644 --- a/test/eca/remote/handlers_test.clj +++ b/test/eca/remote/handlers_test.clj @@ -51,7 +51,26 @@ (let [response (handlers/handle-list-chats (components) nil) body (json/parse-string (:body response) true)] (is (= 1 (count body))) - (is (= "c1" (:id (first body))))))) + (is (= "c1" (:id (first body)))) + (is (= 0 (:pendingApprovalCount (first body)))))) + + (testing "pendingApprovalCount reflects only :waiting-approval tool calls" + (swap! (h/db*) assoc + :chats {"c1" {:id "c1" + :title "Has pending" + :status :running + :tool-calls {"tc-1" {:status :waiting-approval} + "tc-2" {:status :waiting-approval} + "tc-3" {:status :executing} + "tc-4" {:status :completed}}} + "c2" {:id "c2" :title "No pending" :status :idle}} + :chat-start-fired #{"c1" "c2"}) + (let [response (handlers/handle-list-chats (components) nil) + body (json/parse-string (:body response) true) + by-id (into {} (map (juxt :id identity) body))] + (is (= 2 (count body))) + (is (= 2 (get-in by-id ["c1" :pendingApprovalCount]))) + (is (= 0 (get-in by-id ["c2" :pendingApprovalCount])))))) (deftest handle-get-chat-test (testing "returns 404 for missing chat" @@ -66,7 +85,58 @@ body (json/parse-string (:body response) true)] (is (= 200 (:status response))) (is (= "c1" (:id body))) - (is (= "My Chat" (:title body)))))) + (is (= "My Chat" (:title body))))) + + (testing "pendingToolCalls is an empty array when no tool calls are waiting" + (swap! (h/db*) assoc-in [:chats "c1"] {:id "c1" :title "T" :status :idle}) + (let [response (handlers/handle-get-chat (components) nil "c1") + body (json/parse-string (:body response) true)] + (is (= 200 (:status response))) + (is (= [] (:pendingToolCalls body))))) + + (testing "pendingToolCalls surfaces tool calls in :waiting-approval, excluding other statuses" + (swap! (h/db*) assoc-in [:chats "c1"] + {:id "c1" + :title "T" + :status :running + :tool-calls {"tc-1" {:status :waiting-approval + :name "shell_command" + :server "eca" + :origin :native + :arguments {:command "ls"} + :manual-approval true + :summary "Run ls" + :details {:type "shellCommand"}} + "tc-2" {:status :executing + :name "read_file" + :server "eca" + :origin :native + :arguments {:path "/tmp/x"}} + "tc-3" {:status :completed + :name "write_file"}}}) + (let [response (handlers/handle-get-chat (components) nil "c1") + body (json/parse-string (:body response) true) + pending (:pendingToolCalls body)] + (is (= 200 (:status response))) + (is (= 1 (count pending))) + (is (= {:id "tc-1" + :name "shell_command" + :server "eca" + :origin "native" + :arguments {:command "ls"} + :manualApproval true + :summary "Run ls" + :details {:type "shellCommand"}} + (first pending))))) + + (testing "approved/rejected tool calls drop out of pendingToolCalls" + (swap! (h/db*) assoc-in [:chats "c1"] + {:id "c1" + :tool-calls {"tc-1" {:status :rejected :name "x"} + "tc-2" {:status :completed :name "y"}}}) + (let [response (handlers/handle-get-chat (components) nil "c1") + body (json/parse-string (:body response) true)] + (is (= [] (:pendingToolCalls body)))))) (deftest handle-stop-test (testing "returns 404 for missing chat" @@ -90,7 +160,19 @@ body (json/parse-string (:body response) true)] (is (= 200 (:status response))) (is (string? (:version body))) - (is (= "1.0" (:protocolVersion body)))))) + (is (= "1.0" (:protocolVersion body))))) + + (testing "chat entries include pendingApprovalCount" + (swap! (h/db*) assoc + :chats {"c1" {:id "c1" :title "Has pending" :status :running + :tool-calls {"tc-1" {:status :waiting-approval} + "tc-2" {:status :executing}}}} + :chat-start-fired #{"c1"}) + (let [response (handlers/handle-session (components) nil) + body (json/parse-string (:body response) true) + chat (first (:chats body))] + (is (= "c1" (:id chat))) + (is (= 1 (:pendingApprovalCount chat)))))) (deftest handle-answer-question-test (let [inner (h/messenger)