diff --git a/CHANGELOG.md b/CHANGELOG.md index c939e87eb..35caf7e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add `rename_chat_title`, a native tool that lets an approved agent rename the current chat title. + ## 0.161.1 - Add Claude Opus 5.5 support. diff --git a/docs/config/tools.md b/docs/config/tools.md index 100c306b9..29ab2f042 100644 --- a/docs/config/tools.md +++ b/docs/config/tools.md @@ -632,6 +632,8 @@ Globally ECA allows its read-only builtin tools and asks for everything else: } ``` +The `rename_chat_title` builtin changes chat metadata, so it is not in the default `allow` list. It asks for approval unless you add an explicit rule or enable trust mode. + The builtin `plan` and `explorer` agents replace these with stricter rules: `allow` only covers the read-only builtin tools plus read-only shell commands (`pwd`, `git diff/log/show`, `find`, `ls`), and `deny` blocks dangerous shell patterns (file mutations like `rm`/`mv`/`cp`/`touch`/`mkdir`, output redirections, pipes to `tee`/`dd`/`xargs`, in-place `sed`/`awk`/`perl`, `git add/commit/push`, `npm install`). Check the up-to-date values in [config.clj](https://github.com/editor-code-assistant/eca/blob/master/src/eca/config.clj). ### Debugging approval rules diff --git a/docs/features.md b/docs/features.md index ea0d59a16..a407ee822 100644 --- a/docs/features.md +++ b/docs/features.md @@ -81,6 +81,13 @@ ECA support built-in tools to avoid user extra installation and configuration, t - `editor_definition`: Ask client for the definition locations of a symbol (like LSP definition). Requires client capability, can be disabled via `toolCall.editorNav.enabled` config. - `editor_references`: Ask client for the references of a symbol (like LSP references). Requires client capability, can be disabled via `toolCall.editorNav.enabled` config. +=== "Chat" + + Provides access to chat metadata and lifecycle actions. + + - `compact_chat`: submit a summary during chat compaction. + - `rename_chat_title`: rename the current chat title after the user explicitly asks for it. This tool asks for approval by default. + !!! info "Custom Tools" Besides the built-in native tools, ECA allows you to define your own tools by wrapping any command-line executable. This feature enables you to extend ECA's capabilities to match your specific workflows, such as running custom scripts, interacting with internal services, or using your favorite CLI tools. diff --git a/docs/protocol.md b/docs/protocol.md index de7e96493..a0d41a210 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -3025,13 +3025,16 @@ interface EcaServerUpdatedParams { /** * The built-in tools supported by eca. * - * Built-in tools include: read_file, view_image, write_file, edit_file, move_file, - * directory_tree, shell_command, editor_diagnostics, editor_definition, - * editor_references, compact_chat, skill, spawn_agent, and task. + * Built-in tools include: ask_user, bg_job, compact_chat, directory_tree, + * edit_file, editor_definition, editor_diagnostics, editor_references, + * fetch_rule, git, grep, move_file, preview_file_change, read_file, + * rename_chat_title, search_tools, shell_command, skill, spawn_agent, + * task, view_image, and write_file. * - * Note: `spawn_agent` and `task` are excluded from subagent tool sets. - * `spawn_agent` is excluded to prevent nesting, and `task` because - * task list state is chat-local and should be managed by the parent agent. + * Note: `spawn_agent`, `task`, `git`, `ask_user`, and `rename_chat_title` + * are excluded from subagent tool sets. `spawn_agent` is excluded to prevent + * nesting. `task`, `git`, `ask_user`, and `rename_chat_title` require + * parent-agent state, user interaction, or visible parent chat ownership. */ tools: ServerTool[]; } diff --git a/integration-test/integration/chat/commands_test.clj b/integration-test/integration/chat/commands_test.clj index dcb8f360a..dfab6dc95 100644 --- a/integration-test/integration/chat/commands_test.clj +++ b/integration-test/integration/chat/commands_test.clj @@ -52,8 +52,10 @@ {:name "plugins" :arguments []} {:name "plugin-install" :arguments [{:name "plugin" :description "Plugin name or plugin@marketplace" :required true}]} + {:name "plugin-update" + :arguments [{:name "marketplace" :description "Configured marketplace source name" :required true}]} {:name "plugin-uninstall" - :arguments [{:name "plugin" :description "Plugin name" :required true}]} + :arguments [{:name "plugin" :description "Plugin name or plugin@marketplace" :required true}]} {:name "hooks" :arguments []} {:name "eca-info" :arguments nil}]} resp)))) diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index 1d9d59a12..b3c18568b 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -10,6 +10,8 @@ [eca.features.background-tasks :as bg] [eca.features.chat.history :as history] [eca.features.chat.lifecycle :as lifecycle] + [eca.features.chat.persistence :as chat.persistence] + [eca.features.chat.title :as chat.title] [eca.features.chat.tool-calls :as tc] [eca.features.commands :as f.commands] [eca.features.context :as f.context] @@ -955,32 +957,7 @@ (pos? m) (format "%dm%02ds" m s) :else (format "%ds" s)))) -(defn ^:private sanitize-title - "Clean up a chat title: take first meaningful line, strip control chars, - markdown header prefixes, collapse whitespace, and truncate to 40 chars. - - If the first non-blank line is a bare markdown header with nothing else - (e.g. '## Understand' — a planning-mode section the title model sometimes - mimics), fall through to the next non-blank line when one exists." - [^String s] - (when s - (let [lines (->> (string/split s #"\n") - (map string/trim) - (remove string/blank?)) - bare-header? (fn [^String line] - (boolean (re-matches #"#+\s+\S.*" line))) - picked (or (when-let [first-line (first lines)] - (if (and (bare-header? first-line) - (seq (rest lines))) - (first (rest lines)) - first-line)) - "")] - (-> picked - (string/replace #"[\x00-\x1f\x7f]" " ") - (string/replace #"^#+\s*" "") - (string/replace #"\s+" " ") - (string/trim) - (as-> t (subs t 0 (min (count t) 40))))))) +(def ^:private sanitize-title chat.title/sanitize-title) (defn ^:private prompt-messages! "Send user messages to LLM with hook processing. @@ -1084,7 +1061,7 @@ ;; *_result entry appended right after, which ;; triggers the save in their place. (when-not (#{"tool_call" "server_tool_use"} role) - (db/save-chat! @db* chat-id metrics)))) + (chat.persistence/save-chat-current! db* chat-id metrics)))) on-usage-updated (fn [usage] (when-let [usage (shared/usage-msg->usage usage full-model chat-ctx)] ;; Never let the context-breakdown (a display-only @@ -1145,11 +1122,14 @@ :provider-auth provider-auth :subagent? true})] (when output-text - (let [title (sanitize-title output-text)] - (swap! db* assoc-in [:chats chat-id :title] title) - (lifecycle/send-content! chat-ctx :system (assoc-some {:type :metadata} :title title)) - (when (= :idle (get-in @db* [:chats chat-id :status])) - (db/save-chat! @db* chat-id metrics)))))))) + (chat.title/update-generated-chat-title! + db* chat-id output-text + {:messenger messenger + :metrics metrics + :parent-chat-id (:parent-chat-id chat-ctx) + :role :system + :expected-prompt-id prompt-id + :expected-user-prompt-count prompt-count})))))) (lifecycle/send-content! chat-ctx :system {:type :progress :state :running :text "Waiting model"}) (if (and (lifecycle/auto-compact? chat-id agent full-model config @db*) (not (:auto-compacted? chat-ctx))) @@ -1634,7 +1614,7 @@ (swap! db* assoc-in [:chats chat-id :prompt-error] (prompt-error-data error-data error-type)) (lifecycle/send-content! chat-ctx :system {:type :text :text text}) - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks chat-ctx))))) :else @@ -1729,7 +1709,7 @@ ;; :prompt-finished? was already set or the prompt-id rotated, ;; which would leave a chat that hit an error without a save. ;; Persist explicitly so users can always /resume an errored chat. - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks chat-ctx))))))))}) (catch Exception e (when-not (:silent? (ex-data e)) @@ -1743,7 +1723,7 @@ (lifecycle/send-content! chat-ctx :system {:type :text :text (str "\n\n" "Error: " (or (ex-message e) (.getName (class e))))}) ;; Belt-and-suspenders: persist before finish-chat-prompt!, ;; which may short-circuit. See note above in :on-error. - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks chat-ctx)))) (finally (when (and (= prompt-id (get-in @db* [:chats chat-id :prompt-id])) @@ -1754,7 +1734,7 @@ (when-not (get-in @db* [:chats chat-id :prompt-finished?]) (messenger/chat-status-changed (:messenger chat-ctx) {:chat-id chat-id :status :idle}) (lifecycle/trigger-chat-status-hook! chat-ctx)) - (db/save-chat! @db* chat-id metrics)))))))))) + (chat.persistence/save-chat-current! db* chat-id metrics)))))))))) (defn ^:private send-mcp-prompt! [{:keys [prompt args] :as _decision} @@ -2270,7 +2250,7 @@ (swap-vals! db* update-in [:chats chat-id] #(or % new-chat))) created? (and new-chat (nil? (get-in old-db [:chats chat-id]))) _ (when created? - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (messenger/chat-opened messenger {:chat-id chat-id :title (:title new-chat)}) (when (:trust new-chat) (config/notify-fields-changed-only! {:chat {:select-trust true}} messenger db* chat-id))) @@ -2501,7 +2481,7 @@ (dissoc :tool-calls :last-api :usage :task :prompt-cache :last-editor-state))))) (messenger/chat-cleared messenger {:chat-id chat-id :messages messages}) - (db/save-chat! @db* chat-id metrics))) + (chat.persistence/save-chat-current! db* chat-id metrics))) (defn update-chat "Update chat metadata like title and trust. @@ -2513,14 +2493,7 @@ (when (some? trust) (swap! db* assoc-in [:chats chat-id :trust] trust)) (when title - (let [title (sanitize-title title)] - (swap! db* assoc-in [:chats chat-id :title] title) - (swap! db* assoc-in [:chats chat-id :title-custom?] true) - (messenger/chat-content-received messenger - {:chat-id chat-id - :role "system" - :content {:type :metadata :title title}}) - (db/save-chat! @db* chat-id metrics)))) + (chat.title/update-chat-title! db* chat-id title messenger metrics))) {}) (defn rollback-chat @@ -2553,7 +2526,7 @@ ;; Rollback is the user's recovery tool for a chat that got into a bad ;; state. Persist immediately so the cleaned-up history survives a ;; restart instead of relying on the next unrelated save. - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (messenger/chat-cleared messenger {:chat-id chat-id @@ -2593,7 +2566,7 @@ new-messages (into (subvec messages 0 insert-after) (cons flag-msg (subvec messages insert-after)))] (swap! db* assoc-in [:chats chat-id :messages] new-messages) - (db/save-chat! @db* chat-id metrics) + (chat.persistence/save-chat-current! db* chat-id metrics) (messenger/chat-cleared messenger {:chat-id chat-id :messages true}) (send-chat-contents! new-messages {:chat-id chat-id :db* db* :messenger messenger}))) {})) @@ -2608,7 +2581,7 @@ messages))] (when (not= (count new-messages) (count messages)) (swap! db* assoc-in [:chats chat-id :messages] new-messages) - (db/save-chat! @db* chat-id metrics)))) + (chat.persistence/save-chat-current! db* chat-id metrics)))) {}) (defn fork-chat @@ -2636,7 +2609,7 @@ :prompt-finished? true}] (swap! db* assoc-in [:chats new-id] new-chat) (mark-editor-open! db* new-id) - (db/save-chat! @db* new-id metrics) + (chat.persistence/save-chat-current! db* new-id metrics) (messenger/chat-opened messenger {:chat-id new-id :title new-title}) (send-chat-contents! kept-messages {:chat-id new-id :db* db* :messenger messenger}) (lifecycle/send-content! {:messenger messenger :chat-id new-id} diff --git a/src/eca/features/chat/lifecycle.clj b/src/eca/features/chat/lifecycle.clj index 16ea26cdb..0aa7fdcf4 100644 --- a/src/eca/features/chat/lifecycle.clj +++ b/src/eca/features/chat/lifecycle.clj @@ -2,6 +2,7 @@ (:require [clojure.string :as string] [eca.db :as db] + [eca.features.chat.persistence :as chat.persistence] [eca.features.hooks :as f.hooks] [eca.features.login :as f.login] [eca.logger :as logger] @@ -508,7 +509,7 @@ (dispatch-finish-callbacks! chat-ctx {:follow-up-text follow-up-text :stop-turn? stop-turn? :stopping? stopping?}) - (db/save-chat! @db* chat-id metrics))))) + (chat.persistence/save-chat-current! db* chat-id metrics))))) (defn finish-chat-prompt-stopped! "Finish a turn that was halted by a hook (continue:false) or otherwise aborted. diff --git a/src/eca/features/chat/persistence.clj b/src/eca/features/chat/persistence.clj new file mode 100644 index 000000000..abf99cf5b --- /dev/null +++ b/src/eca/features/chat/persistence.clj @@ -0,0 +1,19 @@ +(ns eca.features.chat.persistence + (:require + [eca.db :as db])) + +(set! *warn-on-reflection* true) + +(defonce ^:private chat-save-lock (Object.)) + +(defn with-save-lock! + "Run F while holding the chat save lock." + [f] + (locking chat-save-lock + (f))) + +(defn save-chat-current! + "Persist CHAT-ID from the current DB atom snapshot." + [db* chat-id metrics] + (with-save-lock! + #(db/save-chat! @db* chat-id metrics))) diff --git a/src/eca/features/chat/title.clj b/src/eca/features/chat/title.clj new file mode 100644 index 000000000..e34ef040a --- /dev/null +++ b/src/eca/features/chat/title.clj @@ -0,0 +1,113 @@ +(ns eca.features.chat.title + (:require + [clojure.string :as string] + [eca.features.chat.persistence :as chat.persistence] + [eca.messenger :as messenger] + [eca.shared :refer [assoc-some]])) + +(set! *warn-on-reflection* true) + +(defn sanitize-title + "Clean up a chat title: take first meaningful line, strip control chars, + markdown header prefixes, collapse whitespace, and truncate to 40 chars. + + If the first non-blank line is a bare markdown header with nothing else + (e.g. '## Understand' - a planning-mode section the title model sometimes + mimics), fall through to the next non-blank line when one exists." + [^String s] + (when s + (let [lines (->> (string/split s #"\n") + (map string/trim) + (remove string/blank?)) + bare-header? (fn [^String line] + (boolean (re-matches #"#+\s+\S.*" line))) + picked (or (when-let [first-line (first lines)] + (if (and (bare-header? first-line) + (seq (rest lines))) + (first (rest lines)) + first-line)) + "")] + (-> picked + (string/replace #"[\x00-\x1f\x7f]" " ") + (string/replace #"^#+\s*" "") + (string/replace #"\s+" " ") + (string/trim) + (as-> t (subs t 0 (min (count t) 40))))))) + +(defn- commit-title-update! + [db* chat-id title update-chat] + (loop [] + (let [db @db* + chat (get-in db [:chats chat-id])] + (if-not chat + nil + (let [title (sanitize-title title) + updated-chat (update-chat chat title)] + (if-not updated-chat + nil + (let [new-db (assoc-in db [:chats chat-id] updated-chat)] + (if (compare-and-set! db* db new-db) + {:db new-db + :chat updated-chat + :title title} + (recur))))))))) + +(defn- notify-title! [messenger chat-id parent-chat-id role title] + (messenger/chat-content-received messenger + (assoc-some {:chat-id chat-id + :role role + :content {:type :metadata :title title}} + :parent-chat-id parent-chat-id))) + +(defn- update-title-with-side-effects! + [db* chat-id title update-chat {:keys [messenger metrics parent-chat-id role save?] + :or {role "system" + save? (constantly true)}}] + (chat.persistence/with-save-lock! + (fn [] + (when-let [{:keys [title] :as result} + (commit-title-update! db* chat-id title update-chat)] + (let [db @db* + chat (get-in db [:chats chat-id])] + (when (and chat (= title (:title chat))) + (when messenger + (notify-title! messenger chat-id parent-chat-id role title)) + (when (and metrics (save? chat)) + (chat.persistence/save-chat-current! db* chat-id metrics)) + (assoc result :db @db* :chat (get-in @db* [:chats chat-id])))))))) + +(defn update-chat-title! + "Set CHAT-ID's title to TITLE, mark it custom, notify clients, and save it." + [db* chat-id title messenger metrics] + (when-let [{:keys [title]} + (update-title-with-side-effects! + db* chat-id title + (fn [chat title] + (assoc chat + :title title + :title-custom? true + :updated-at (System/currentTimeMillis))) + {:messenger messenger + :metrics metrics})] + title)) + +(defn- expected-chat-state? + [chat opts] + (and (or (not (contains? opts :expected-prompt-id)) + (= (:expected-prompt-id opts) (:prompt-id chat))) + (or (not (contains? opts :expected-user-prompt-count)) + (= (:expected-user-prompt-count opts) (:user-prompt-count chat))))) + +(defn update-generated-chat-title! + "Set CHAT-ID's generated title when no custom title won the race." + ([db* chat-id title] + (update-generated-chat-title! db* chat-id title nil)) + ([db* chat-id title opts] + (let [opts (or opts {})] + (update-title-with-side-effects! + db* chat-id title + (fn [chat title] + (when (and (not (:title-custom? chat)) + (expected-chat-state? chat opts)) + (assoc chat :title title))) + (assoc opts :save? #(= :idle (:status %))))))) diff --git a/src/eca/features/tools.clj b/src/eca/features/tools.clj index 19a4e60d3..ce32f140b 100644 --- a/src/eca/features/tools.clj +++ b/src/eca/features/tools.clj @@ -258,9 +258,10 @@ - Excludes spawn_agent to prevent nesting. - Excludes task because task list state is currently chat-local; it should be managed by the parent agent. - Excludes git because subagents don't perform git operations. - - Excludes ask_user because subagents run non-interactively and cannot prompt the user." + - Excludes ask_user because subagents run non-interactively and cannot prompt the user. + - Excludes rename_chat_title because subagents do not own the visible parent chat title." [tools] - (filterv #(not (contains? #{"spawn_agent" "task" "git" "ask_user"} (:name %))) tools)) + (filterv #(not (contains? #{"spawn_agent" "task" "git" "ask_user" "rename_chat_title"} (:name %))) tools)) (defn ^:private get-defer-all-percent "Percentage of the model context window the MCP tool definitions may take diff --git a/src/eca/features/tools/chat.clj b/src/eca/features/tools/chat.clj index 39a37c11c..1400a6e57 100644 --- a/src/eca/features/tools/chat.clj +++ b/src/eca/features/tools/chat.clj @@ -1,5 +1,7 @@ (ns eca.features.tools.chat (:require + [clojure.string :as string] + [eca.features.chat.title :as chat.title] [eca.features.tools.util :as tools.util])) (set! *warn-on-reflection* true) @@ -18,12 +20,47 @@ "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)))) +(defn ^:private sanitized-title-arg [args] + (let [title (get args "title")] + (when (string? title) + (chat.title/sanitize-title title)))) + +(defn ^:private rename-chat-title [arguments {:keys [db* chat-id messenger metrics]}] + (let [title (sanitized-title-arg arguments)] + (cond + (string/blank? title) + (tools.util/single-text-content + "INVALID_ARGS: title is required and must not be blank." + :error) + + (not (get-in @db* [:chats chat-id])) + (tools.util/single-text-content "Chat not found." :error) + + :else + (if-let [title (chat.title/update-chat-title! db* chat-id title messenger metrics)] + (tools.util/single-text-content + (format "Chat title renamed to: %s" title)) + (tools.util/single-text-content "Chat not found." :error))))) + (def definitions {"compact_chat" {: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"}} + :description "The summary/compacted text"}} :required ["summary"]} :handler #'compact-chat - :summary-fn (constantly "Compacting...")}}) + :summary-fn (constantly "Compacting...")} + + "rename_chat_title" + {:description "Rename the current chat title. Use this only after the user explicitly asks to rename the chat title." + :parameters {:type "object" + :properties {"title" {:type "string" + :description "The new title for the current chat"}} + :required ["title"]} + :handler #'rename-chat-title + :summary-fn (fn [{:keys [args]}] + (let [title (sanitized-title-arg args)] + (if (string/blank? title) + "Renaming chat" + (format "Renaming chat: %s" title))))}}) diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index 57bb6c96a..03ce086cb 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -7,6 +7,7 @@ [eca.db :as db] [eca.features.chat :as f.chat] [eca.features.chat.lifecycle :as lifecycle] + [eca.features.chat.title :as chat.title] [eca.features.context :as f.context] [eca.features.index :as f.index] [eca.features.prompt :as f.prompt] @@ -18,6 +19,7 @@ [eca.llm-api :as llm-api] [eca.llm-util :as llm-util] [eca.logger :as logger] + [eca.messenger :as messenger] [eca.test-helper :as h] [matcher-combinators.matchers :as m] [matcher-combinators.test :refer [match?]])) @@ -643,6 +645,212 @@ (testing "returns nil for nil input" (is (nil? (#'f.chat/sanitize-title nil))))) +(defn ^:private call-in-thread [f] + (let [result (promise) + thread (Thread. (fn [] + (try + (deliver result {:value (f)}) + (catch Throwable e + (deliver result {:error e})))))] + (.start thread) + result)) + +(defn ^:private blocking-title-messenger + [blocked-title started release events*] + (reify messenger/IMessenger + (chat-content-received [_ msg] + (let [title (get-in msg [:content :title])] + (when (= blocked-title title) + (deliver started true) + @release) + (swap! events* conj [:metadata title]))))) + +(deftest generated-title-update-test + (testing "writes generated title for a chat without a custom title" + (h/reset-components!) + (let [db* (h/db*) + chat-id "generated-title-chat" + result (do + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title"}) + (chat.title/update-generated-chat-title! db* chat-id "Generated title\nignored"))] + (is (= "Generated title" (:title result))) + (is (= "Generated title" (get-in @db* [:chats chat-id :title]))) + (is (not (get-in @db* [:chats chat-id :title-custom?]))) + (is (= "Generated title" (get-in result [:db :chats chat-id :title]))))) + + (testing "does not overwrite a custom title after a CAS retry" + (h/reset-components!) + (let [db* (h/db*) + chat-id "generated-title-race-chat" + sanitize-calls* (atom 0)] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title"}) + (with-redefs [chat.title/sanitize-title + (fn [_title] + (when (= 1 (swap! sanitize-calls* inc)) + (swap! db* update-in [:chats chat-id] + assoc + :title "Custom during CAS" + :title-custom? true)) + "Generated after race")] + (is (nil? (chat.title/update-generated-chat-title! db* chat-id "Generated after race")))) + (is (= "Custom during CAS" (get-in @db* [:chats chat-id :title]))) + (is (true? (get-in @db* [:chats chat-id :title-custom?]))))) + + (testing "rejects generated title from stale prompt state" + (h/reset-components!) + (let [db* (h/db*) + chat-id "generated-title-stale-state-chat" + saves* (atom [])] + (swap! db* assoc-in [:chats chat-id] + {:id chat-id + :title "Newer generated title" + :status :idle + :prompt-id "prompt-2" + :user-prompt-count 2}) + (with-redefs [db/save-chat! + (fn [db chat-id _metrics] + (swap! saves* conj (get-in db [:chats chat-id :title])))] + (is (nil? (chat.title/update-generated-chat-title! + db* chat-id "Stale generated title" + {:messenger (h/messenger) + :metrics (h/metrics) + :expected-prompt-id "prompt-1" + :expected-user-prompt-count 1})))) + (is (= "Newer generated title" (get-in @db* [:chats chat-id :title]))) + (is (empty? (:chat-content-received (h/messages)))) + (is (empty? @saves*)))) + + (testing "serializes generated title side effects before a later custom title" + (h/reset-components!) + (let [db* (h/db*) + chat-id "generated-title-side-effects-chat" + generated-started (promise) + release-generated (promise) + events* (atom []) + messenger (blocking-title-messenger "Generated title" generated-started release-generated events*)] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title" :status :idle}) + (with-redefs [db/save-chat! + (fn [db chat-id _metrics] + (swap! events* conj [:save (get-in db [:chats chat-id :title])]))] + (let [generated-result (call-in-thread + #(chat.title/update-generated-chat-title! + db* chat-id "Generated title" + {:messenger messenger + :metrics (h/metrics) + :role :system})) + generated-started? (deref generated-started 2000 false) + custom-result (when generated-started? + (call-in-thread + #(chat.title/update-chat-title! db* chat-id "Custom title" + messenger + (h/metrics))))] + (is generated-started? + "Generated title metadata should start before the custom update is released") + (when generated-started? + (deliver release-generated true) + (is (not= ::timeout (deref generated-result 1000 ::timeout))) + (is (not= ::timeout (deref custom-result 1000 ::timeout))) + (is (= [[:metadata "Generated title"] + [:save "Generated title"] + [:metadata "Custom title"] + [:save "Custom title"]] + @events*)) + (is (= "Custom title" (get-in @db* [:chats chat-id :title]))) + (is (true? (get-in @db* [:chats chat-id :title-custom?]))))))))) + +(deftest explicit-title-update-side-effects-test + (testing "serializes explicit title side effects in commit order" + (h/reset-components!) + (let [db* (h/db*) + chat-id "explicit-title-side-effects-chat" + first-started (promise) + release-first (promise) + events* (atom []) + messenger (blocking-title-messenger "First title" first-started release-first events*)] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title" :updated-at 1}) + (with-redefs [db/save-chat! + (fn [db chat-id _metrics] + (swap! events* conj [:save (get-in db [:chats chat-id :title])]))] + (let [first-result (call-in-thread + #(chat.title/update-chat-title! db* chat-id "First title" + messenger + (h/metrics))) + first-started? (deref first-started 2000 false) + second-result (when first-started? + (call-in-thread + #(chat.title/update-chat-title! db* chat-id "Second title" + messenger + (h/metrics))))] + (is first-started? + "First title metadata should start before the second update is released") + (when-not first-started? + (is (nil? (:error (deref first-result 1000 {}))) + "First title update should not fail before metadata")) + (when first-started? + (deliver release-first true) + (is (not= ::timeout (deref first-result 1000 ::timeout))) + (is (not= ::timeout (deref second-result 1000 ::timeout))) + (is (= [[:metadata "First title"] + [:save "First title"] + [:metadata "Second title"] + [:save "Second title"]] + @events*)) + (is (= "Second title" (get-in @db* [:chats chat-id :title]))) + (is (true? (get-in @db* [:chats chat-id :title-custom?]))) + (is (< 1 (get-in @db* [:chats chat-id :updated-at]))))))))) + +(deftest prompt-save-title-persistence-race-test + (testing "stale prompt save cannot persist after a custom title save" + (h/reset-components!) + (let [db* (h/db*) + chat-id "prompt-save-title-race-chat" + first-save-started (promise) + second-save-started (promise) + release-first-save (promise) + save-calls* (atom 0) + saved-titles* (atom [])] + (swap! db* assoc-in [:chats chat-id] + {:id chat-id + :title "Old title" + :status :running + :prompt-id "prompt-1"}) + (with-redefs [db/save-chat! + (fn [db chat-id _metrics] + (let [call (swap! save-calls* inc) + title (get-in db [:chats chat-id :title])] + (if (= 1 call) + (do + (deliver first-save-started true) + @release-first-save + (swap! saved-titles* conj title)) + (do + (deliver second-save-started true) + (swap! saved-titles* conj title)))))] + (let [prompt-result (call-in-thread + #(lifecycle/finish-chat-prompt! + :idle + {:chat-id chat-id + :db* db* + :metrics (h/metrics) + :messenger (h/messenger) + :prompt-id "prompt-1" + :skip-post-request-hooks? true})) + first-started? (deref first-save-started 2000 false)] + (is first-started? + "Prompt save should start before the custom rename") + (when first-started? + (let [title-result (call-in-thread + #(chat.title/update-chat-title! db* chat-id "Custom title" + (h/messenger) + (h/metrics)))] + (deref second-save-started 1000 ::not-started) + (deliver release-first-save true) + (is (not= ::timeout (deref prompt-result 1000 ::timeout))) + (is (not= ::timeout (deref title-result 1000 ::timeout))) + (is (= "Custom title" (last @saved-titles*))) + (is (= "Custom title" (get-in @db* [:chats chat-id :title]))) + (is (true? (get-in @db* [:chats chat-id :title-custom?])))))))))) + (deftest conversation-title-transcript-test (testing "renders user/assistant messages as a plain-text transcript" (is (= "user: hi\n\nassistant: hello" @@ -790,6 +998,20 @@ "Should NOT re-title after manual rename") (is (= "My Custom Title" (get-in (h/db) [:chats chat-id :title]))))) + (testing "automatic title does not overwrite a custom title set while generating" + (h/reset-components!) + (let [chat-id "title-race-chat" + sync-mock (fn [_params] + (f.chat/update-chat {:chat-id chat-id :title "Custom During Title"} + (h/db*) + (h/messenger) + (h/metrics)) + {:output-text "Auto Later"})] + (prompt-with-title! {:message "Help me debug" :chat-id chat-id} + {:sync-prompt-mock sync-mock}) + (is (= "Custom During Title" (get-in (h/db) [:chats chat-id :title]))) + (is (true? (get-in (h/db) [:chats chat-id :title-custom?]))))) + (testing "title disabled in config skips all generation" (h/reset-components!) (let [sync-prompt-calls* (atom []) diff --git a/test/eca/features/tools/chat_test.clj b/test/eca/features/tools/chat_test.clj index 3dc99f391..ced2a9ada 100644 --- a/test/eca/features/tools/chat_test.clj +++ b/test/eca/features/tools/chat_test.clj @@ -1,6 +1,10 @@ (ns eca.features.tools.chat-test (:require [clojure.test :refer [deftest is testing]] + [eca.config :as config] + [eca.db :as db] + [eca.features.chat.title :as chat.title] + [eca.features.tools :as f.tools] [eca.features.tools.chat :as f.tools.chat] [eca.test-helper :as h] [matcher-combinators.test :refer [match?]])) @@ -99,3 +103,121 @@ (is (contains? (:properties params) "summary")) (is (= "string" (get-in params [:properties "summary" :type]))) (is (= ["summary"] (:required params)))))) + +(deftest rename-chat-title-test + (testing "renames the current chat and notifies clients" + (let [db* (h/db*) + chat-id "rename-chat" + saved* (atom nil) + handler (get-in f.tools.chat/definitions ["rename_chat_title" :handler])] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title"}) + (with-redefs [db/save-chat! (fn [db chat-id metrics] + (reset! saved* {:db db + :chat-id chat-id + :metrics metrics}))] + (let [result (handler {"title" "New title\nignored"} + {:db* db* + :chat-id chat-id + :messenger (h/messenger) + :metrics (h/metrics)})] + (is (match? {:error false + :contents [{:type :text + :text "Chat title renamed to: New title"}]} + result)) + (is (= "New title" (get-in @db* [:chats chat-id :title]))) + (is (true? (get-in @db* [:chats chat-id :title-custom?]))) + (is (match? {:chat-id chat-id + :role "system" + :content {:type :metadata + :title "New title"}} + (-> (h/messages) :chat-content-received first))) + (is (= chat-id (:chat-id @saved*))) + (is (= "New title" (get-in @saved* [:db :chats chat-id :title]))) + (is (true? (get-in @saved* [:db :chats chat-id :title-custom?]))))))) + + (testing "rejects missing or blank titles without mutating state" + (let [handler (get-in f.tools.chat/definitions ["rename_chat_title" :handler]) + cases [{"title" ""} {"title" " \n "} {"title" 42} {}]] + (doseq [arguments cases] + (h/reset-components!) + (let [db* (h/db*) + chat-id "rename-chat"] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title"}) + (let [before @db* + result (handler arguments + {:db* db* + :chat-id chat-id + :messenger (h/messenger) + :metrics (h/metrics)})] + (is (match? {:error true + :contents [{:type :text + :text "INVALID_ARGS: title is required and must not be blank."}]} + result)) + (is (= before @db*)) + (is (nil? (:chat-content-received (h/messages))))))))) + + (testing "returns an error when the chat is missing" + (let [db* (h/db*) + handler (get-in f.tools.chat/definitions ["rename_chat_title" :handler]) + result (handler {"title" "New title"} + {:db* db* + :chat-id "missing-chat" + :messenger (h/messenger) + :metrics (h/metrics)})] + (is (match? {:error true + :contents [{:type :text + :text "Chat not found."}]} + result)) + (is (nil? (:chat-content-received (h/messages)))))) + + (testing "returns an error when the chat disappears during rename" + (let [db* (h/db*) + chat-id "rename-chat" + handler (get-in f.tools.chat/definitions ["rename_chat_title" :handler])] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :title "Old title"}) + (with-redefs [chat.title/update-chat-title! (fn [& _] nil)] + (let [result (handler {"title" "New title"} + {:db* db* + :chat-id chat-id + :messenger (h/messenger) + :metrics (h/metrics)})] + (is (match? {:error true + :contents [{:type :text + :text "Chat not found."}]} + result))))))) + +(deftest rename-chat-title-tool-definition-test + (testing "Tool definition has correct structure" + (let [tool-def (get f.tools.chat/definitions "rename_chat_title")] + (is (some? tool-def) "Tool definition should exist") + (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 (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") + (is (= "Renaming chat" + ((:summary-fn tool-def) {:args {"title" 42}}))))) + + (testing "Tool parameters schema is correct" + (let [params (get-in f.tools.chat/definitions ["rename_chat_title" :parameters])] + (is (= "object" (:type params))) + (is (contains? (:properties params) "title")) + (is (= "string" (get-in params [:properties "title" :type]))) + (is (= ["title"] (:required params)))))) + +(deftest rename-chat-title-asks-by-default-test + (let [db {:chats {"rename-chat" {:id "rename-chat"}}} + all-tools (f.tools/all-tools "rename-chat" "code" db config/initial-config) + tool (f.tools/resolve-tool "eca__rename_chat_title" all-tools)] + (is (some? tool)) + (is (= :ask + (f.tools/approval all-tools tool {"title" "New title"} + db config/initial-config "code"))))) + +(deftest rename-chat-title-hidden-from-subagents-test + (let [db {:chats {"subagent-chat" {:id "subagent-chat" + :parent-chat-id "parent-chat" + :subagent {}}}} + all-tools (f.tools/all-tools "subagent-chat" "general" db config/initial-config)] + (is (nil? (f.tools/resolve-tool "eca__rename_chat_title" all-tools)))))