From 4c435b025937a1accdb07f03fcc9471bbf4b1f71 Mon Sep 17 00:00:00 2001 From: Jakub Zika Date: Fri, 31 Jul 2026 21:48:35 +0200 Subject: [PATCH] Keep compact tool schema stable accross compaction --- CHANGELOG.md | 2 + src/eca/features/tools/chat.clj | 28 ++++++------ test/eca/features/chat_test.clj | 41 ++++++++++++++++++ test/eca/features/tools/chat_test.clj | 61 +++++++++++++-------------- test/eca/features/tools_test.clj | 11 +++++ 5 files changed, 96 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1641a67c5..e65ba7fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Keep `compact_chat` in the tool schema across normal and compact requests, preserving prompt-cache prefixes while rejecting calls outside active compaction. + ## 0.152.0 - Share chat history across git worktrees of the same repo, and merge workspace cache writes from concurrent servers instead of overwriting. #558 diff --git a/src/eca/features/tools/chat.clj b/src/eca/features/tools/chat.clj index 8d0806318..39a37c11c 100644 --- a/src/eca/features/tools/chat.clj +++ b/src/eca/features/tools/chat.clj @@ -5,27 +5,25 @@ (set! *warn-on-reflection* true) (defn ^:private compact-chat [arguments {:keys [db* chat-id]}] - (let [summary (get arguments "summary")] - ;; Mark chat as not compacting anymore - (swap! db* assoc-in [:chats chat-id :compacting?] false) - - ;; Save summary to replace chat history later - (swap! db* assoc-in [:chats chat-id :last-summary] summary) - - ;; Signal that compact is done so the LLM loop stops - (swap! db* assoc-in [:chats chat-id :compact-done?] true) - - (tools.util/single-text-content "Compacted successfully!"))) + (let [chat (get-in @db* [:chats chat-id])] + (if (or (:compacting? chat) (:auto-compacting? chat)) + (do + (swap! db* update-in [:chats chat-id] + assoc + :compacting? false + :last-summary (get arguments "summary") + :compact-done? true) + (tools.util/single-text-content "Compacted successfully!")) + (tools.util/single-text-content + "Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold." + :error)))) (def definitions {"compact_chat" - {:description "Compact / summarize a chat, cleaning chat history, emptying usage and presenting the summary to user" + {:description "During chat compaction, submit a summary that will become the active conversation context" :parameters {:type "object" :properties {"summary" {:type "string" :description "The summary/compacted text"}} :required ["summary"]} :handler #'compact-chat - :enabled-fn (fn [{:keys [db chat-id]}] - (or (get-in db [:chats chat-id :compacting?] false) - (get-in db [:chats chat-id :auto-compacting?] false))) :summary-fn (constantly "Compacting...")}}) diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index 00808d620..2ea185aa9 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -1238,6 +1238,47 @@ {:role :system :content {:state :finished :type :progress}}])} (h/messages)))))) +(deftest inactive-compact-tool-error-reaches-provider-continuation-test + (testing "inactive compact_chat result is included in the next provider request" + (h/reset-components!) + (let [continuation* (atom nil) + original-all-tools f.tools/all-tools + original-call-tool! f.tools/call-tool! + expected-error "Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold." + {:keys [chat-id]} + (prompt! + {:message "Continue normally"} + {:all-tools-mock original-all-tools + :call-tool-mock original-call-tool! + :api-mock + (fn [{:keys [on-first-response-received on-message-received + on-prepare-tool-call on-tools-called]}] + (on-first-response-received) + (on-prepare-tool-call {:id "compact-call-1" + :full-name "eca__compact_chat" + :arguments-text "{\"summary\":\"Must not be stored\"}"}) + (reset! continuation* + (on-tools-called [{:id "compact-call-1" + :full-name "eca__compact_chat" + :arguments {"summary" "Must not be stored"}}])) + (on-message-received {:type :text :text "Understood"}) + (on-message-received {:type :finish}))}) + tool-output (->> (:new-messages @continuation*) + (filter #(= "tool_call_output" (:role %))) + last + :content)] + (is (match? {:id "compact-call-1" + :full-name "eca__compact_chat" + :error true + :output {:error true + :contents [{:type :text :text expected-error}]}} + tool-output) + "the provider continuation must contain the exact inactive-compaction error for the LLM") + (is (nil? (get-in (h/db) [:chats chat-id :last-summary]))) + (is (nil? (get-in (h/db) [:chats chat-id :compact-done?]))) + (is (not (true? (get-in (h/db) [:chats chat-id :compacting?]))) + "the accidental call must not activate or complete compaction")))) + (deftest concurrent-tool-calls-test (testing "Running three calls simultaneously" (h/reset-components!) diff --git a/test/eca/features/tools/chat_test.clj b/test/eca/features/tools/chat_test.clj index 2ecca0a51..3dc99f391 100644 --- a/test/eca/features/tools/chat_test.clj +++ b/test/eca/features/tools/chat_test.clj @@ -32,6 +32,19 @@ (is (= true (:compact-done? chat-state)) "Should set compact-done? to true")))))) + (testing "Successfully compacts an auto-compacting chat" + (let [db* (h/db*) + chat-id "test-chat-auto-compacting" + test-summary "Auto-compacted summary"] + (swap! db* assoc-in [:chats chat-id :auto-compacting?] true) + (let [result ((get-in f.tools.chat/definitions ["compact_chat" :handler]) + {"summary" test-summary} + {:db* db* :chat-id chat-id}) + chat-state (get-in @db* [:chats chat-id])] + (is (false? (:error result))) + (is (= test-summary (:last-summary chat-state))) + (is (true? (:compact-done? chat-state)))))) + (testing "Handles empty summary" (let [db* (h/db*) chat-id "test-chat-456" @@ -50,37 +63,20 @@ (is (= empty-summary (:last-summary chat-state))) (is (= true (:compact-done? chat-state)))))))) -(deftest compact-chat-enabled-test - (testing "Tool is enabled when chat is compacting" - (let [db* (h/db*) - chat-id "test-chat-compacting"] - (swap! db* assoc-in [:chats chat-id :compacting?] true) - - (is (true? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn]) - {:db @db* :chat-id chat-id}))))) - - (testing "Tool is disabled when chat is not compacting" - (let [db* (h/db*) - chat-id "test-chat-not-compacting"] - (swap! db* assoc-in [:chats chat-id :compacting?] false) - - (is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn]) - {:db @db* :chat-id chat-id}))))) - - (testing "Tool is disabled when compacting? is not set (defaults to false)" - (let [db* (h/db*) - chat-id "test-chat-no-compacting-key"] - ;; Don't set compacting? at all - - (is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn]) - {:db @db* :chat-id chat-id}))))) - - (testing "Tool is disabled when chat doesn't exist" - (let [db* (h/db*) - chat-id "non-existent-chat"] - - (is (false? ((get-in f.tools.chat/definitions ["compact_chat" :enabled-fn]) - {:db @db* :chat-id chat-id})))))) +(deftest compact-chat-requires-active-compaction-test + (let [db* (h/db*) + chat-id "test-chat-not-compacting" + handler (get-in f.tools.chat/definitions ["compact_chat" :handler])] + (swap! db* assoc-in [:chats chat-id] {:id chat-id}) + (let [before @db* + result (handler {"summary" "Must not be stored"} + {:db* db* :chat-id chat-id})] + (is (match? {:error true + :contents [{:type :text + :text "Chat compaction is not active for this request. This tool is available only while chat compaction is in progress. To compact manually, the user must use the `/compact` command; compaction may also start automatically when context usage reaches the configured threshold."}]} + result)) + (is (= before @db*) + "Inactive compact tool calls must not mutate chat state")))) (deftest compact-chat-summary-fn-test (testing "Summary function returns constant string" @@ -93,7 +89,8 @@ (is (string? (:description tool-def)) "Should have a description") (is (map? (:parameters tool-def)) "Should have parameters") (is (or (fn? (:handler tool-def)) (var? (:handler tool-def))) "Should have a handler function or var") - (is (fn? (:enabled-fn tool-def)) "Should have an enabled-fn") + (is (not (contains? tool-def :enabled-fn)) + "Tool availability must not change the provider tool schema") (is (fn? (:summary-fn tool-def)) "Should have a summary-fn"))) (testing "Tool parameters schema is correct" diff --git a/test/eca/features/tools_test.clj b/test/eca/features/tools_test.clj index f72d9dbdd..336a08280 100644 --- a/test/eca/features/tools_test.clj +++ b/test/eca/features/tools_test.clj @@ -38,6 +38,17 @@ :origin :native}]) (f.tools/all-tools "123" "code" {} {})))) + (testing "Compact tool schema is stable across compaction state" + (let [db {:chats {"123" {}}} + wire-tools (fn [db] + (mapv #(select-keys % [:full-name :description :parameters]) + (f.tools/all-tools "123" "code" db {}))) + normal-tools (wire-tools db) + manual-tools (wire-tools (assoc-in db [:chats "123" :compacting?] true)) + auto-tools (wire-tools (assoc-in db [:chats "123" :auto-compacting?] true))] + (is (some #(= "eca__compact_chat" (:full-name %)) normal-tools)) + (is (= normal-tools manual-tools auto-tools)))) + (testing "Subagent excludes spawn_agent, task, git, and ask_user tools" (let [db {:chats {"sub-1" {:subagent {:name "explorer"}}}} tools (f.tools/all-tools "sub-1" "code" db {})