Merge pull request #565 from editor-code-assistant/fix/compact-tool-cache-invalidation

Keep compact tool schema stable accross compaction

Author: ericdallo

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

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...")}})

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!)

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"

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 {})