Merge remote-tracking branch 'origin/master' into rules_improvement_and_tool

zikajk · zikajk · commit c6b82a5710ab · 2026-04-26T18:40:57.000+02:00
# Conflicts:
#	CHANGELOG.md
#	src/eca/features/chat.clj
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,7 +3,12 @@
 ## Unreleased
 
 - Improve rules with frontmatter filters, condition variables, path-scoped loading, enforcement support, and clearer documentation. #222
+- `preToolCall` hooks now receive `approval: "ask"` for the native `ask_user` tool so notification hooks (e.g. matching `.approval == "ask"`) also fire when the chat is blocked waiting for a user answer, regardless of trust mode.
+
+## 0.129.2
+
 - Add support for gpt-5.5 variants
+- Restore trust mode on chat resume: `chat/open` and the `/resume` slash command now emit `config/updated` with `selectTrust` reflecting the resumed chat's persisted trust toggle, so the client indicator stays in sync with the server's auto-approval behavior. #426
 
 ## 0.129.1
 
diff --git a/docs/config/hooks.md b/docs/config/hooks.md
@@ -102,6 +102,12 @@ To reject a tool call, either output `{"approval": "deny"}` or exit with code `2
     }
     ```
 
+    The same hook also fires for the native `ask_user` tool: it always blocks
+    waiting for a user answer (regardless of trust mode), so `preToolCall`
+    receives `approval: "ask"` for it. This means a single `'.approval == "ask"'`
+    hook covers both "tool call needs approval" and "assistant is waiting on a
+    user question".
+
 === "Inject context on chat start"
 
     ```javascript title="~/.config/eca/config.json"
diff --git a/docs/protocol.md b/docs/protocol.md
@@ -1824,9 +1824,11 @@ in the UI. The server replays the chat by emitting `chat/cleared` (messages),
 the persisted messages. When the persisted chat has a stored model the server
 additionally emits a `config/updated` notification to realign the client's
 selected model (and available variants) with the resumed chat, so the next
-prompt keeps using the chat's original provider/model. Typically used after
-`chat/list` when the user selects a chat that has not been opened in the current
-client session.
+prompt keeps using the chat's original provider/model. The same notification
+also carries `selectTrust` reflecting the resumed chat's trust toggle, so the
+client indicator stays in sync with the auto-approval behavior the server will
+apply. Typically used after `chat/list` when the user selects a chat that has
+not been opened in the current client session.
 
 _Request:_
 
@@ -2300,6 +2302,17 @@ interface ConfigUpdatedParams {
          */
         selectVariant?: string | null;
 
+        /**
+         * The trust toggle state for the active chat. When present clients
+         * should forcefully update the chat trust indicator (and any
+         * derived UI like a shield/flame icon) to match this value.
+         *
+         * Server returns this on chat resume (`chat/open`, `/resume`) so the
+         * client's indicator matches the auto-approval behavior the server
+         * will apply for subsequent tool calls in the resumed chat.
+         */
+        selectTrust?: boolean;
+
         /**
         * Message to show when starting a new chat.
         */
diff --git a/resources/ECA_VERSION b/resources/ECA_VERSION
@@ -1 +1 @@
-0.129.1
+0.129.2
diff --git a/src/eca/config.clj b/src/eca/config.clj
@@ -682,6 +682,19 @@
        messenger
        db*))))
 
+(defn notify-selected-trust-changed!
+  "Server-initiated equivalent of a client-side trust toggle: aligns the
+   client-side trust indicator with the chat's persisted `:trust` value.
+   Emits via `notify-fields-changed-only!`, so it is a no-op when nothing
+   changed. Used by chat resume flows (`chat/open`, `/resume`) so the icon
+   the client shows matches the auto-approval behavior the server is about
+   to apply (#426)."
+  [trust db* messenger]
+  (notify-fields-changed-only!
+   {:chat {:select-trust (boolean trust)}}
+   messenger
+   db*))
+
 (def ^:private config-schema-url "https://eca.dev/config.json")
 
 (defn ^:private flatten-to-paths
diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj
@@ -1480,4 +1480,5 @@
         (send-chat-contents! messages chat-ctx)
         (lifecycle/send-content! chat-ctx :system (assoc-some {:type :metadata} :title title))
         (config/notify-selected-model-changed! (:model chat) db* messenger config)
+        (config/notify-selected-trust-changed! (:trust chat) db* messenger)
         {:found? true :chat-id chat-id :title title}))))
diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj
@@ -546,6 +546,16 @@
         trusted? (= :trust/allow approval)
         effective-approval (if trusted? :allow approval)
 
+        ;; ask_user always blocks waiting for the user regardless of trust/auto-allow.
+        ;; Surface that as :ask to preToolCall hooks so notification hooks
+        ;; (e.g. matching .approval == "ask") also fire when the chat is blocked on
+        ;; a user question. Only override when allowed; explicit deny is preserved.
+        hook-approval (if (and (= "eca" server-name)
+                               (= "ask_user" name)
+                               (= :allow effective-approval))
+                        :ask
+                        effective-approval)
+
         ;; 2. Run hooks to collect modifications and approval overrides
         hook-state* (atom {:hook-results []
                            :approval-override nil
@@ -560,7 +570,7 @@
                   {:tool-name name
                    :server server-name
                    :tool-input arguments
-                   :approval effective-approval})
+                   :approval hook-approval})
            {:on-before-action on-before-hook-action
             :on-after-action (fn [result]
                                (on-after-hook-action result)
diff --git a/src/eca/features/commands.clj b/src/eca/features/commands.clj
@@ -516,6 +516,10 @@
                      ;; Align the client's selected model with the resumed chat
                      ;; so the LLM call keeps using the chat's original model. #417
                      (config/notify-selected-model-changed! (:model chat) db* messenger config)
+                     ;; Align the client's trust indicator with the resumed
+                     ;; chat's persisted :trust so the icon matches the
+                     ;; auto-approval behavior the server will apply. #426
+                     (config/notify-selected-trust-changed! (:trust chat) db* messenger)
                      {:type :chat-messages
                       :clear-before? true
                       :chats {chat-id {:title (:title chat)
diff --git a/src/eca/features/completion.clj b/src/eca/features/completion.clj
@@ -29,8 +29,6 @@
     (->> (assoc lines line-idx updated-line)
          (string/join "\n"))))
 
-
-
 (defn complete [{:keys [doc-text doc-version position]} db* config messenger metrics]
   (let [full-model (get-in config [:completion :model])
         [provider model] (shared/full-model->provider+model full-model)
diff --git a/src/eca/features/tools/agent.clj b/src/eca/features/tools/agent.clj
@@ -91,23 +91,6 @@
            sort
            vec))
 
-(defn ^:private available-variant-names
-  "Returns a sorted union of all variant names across all available models."
-  [config db]
-  (let [model-keys (keys (:models db))]
-    (when (seq model-keys)
-      (let [all-variants (->> model-keys
-                              (mapcat (fn [^String full-model]
-                                        (let [idx (.indexOf full-model "/")]
-                                          (when (pos? idx)
-                                            (let [provider (subs full-model 0 idx)
-                                                  model (subs full-model (inc idx))
-                                                  user-variants (get-in config [:providers provider :models model :variants])]
-                                              (config/selectable-variant-names (config/effective-model-variants config provider model user-variants)))))))
-                              (into (sorted-set)))]
-        (when (seq all-variants)
-          (vec all-variants))))))
-
 (defn ^:private model-variant-names
   "Returns sorted variant names for a specific full model string (e.g. \"anthropic/claude-sonnet-4-6\")."
   [config ^String full-model]
@@ -270,7 +253,7 @@
     (str base-description agents-section)))
 
 (defn definitions
-  [config db]
+  [config _db]
   {"spawn_agent"
    {:description (build-description config)
     :parameters  {:type       "object"
diff --git a/src/eca/features/tools/git.clj b/src/eca/features/tools/git.clj
@@ -96,7 +96,7 @@
                                            :description "The git operation being performed."
                                            :enum ["status" "diff" "log" "add" "commit" "push"
                                                   "branch" "checkout" "merge" "rebase" "stash"
-                                                  "pr_create" "pr_view" "pr_comment" "gh_other"]}
+                                                  "pr_create" "pr_view" "pr_comment" "gh"]}
                               "summary" {:type "string"
                                          :description "Concise 2-3 word description of what is being done (e.g. \"staged changes\", \"all test files\")."}}
                  :required ["command" "operation"]}
diff --git a/test/eca/features/chat/tool_calls_test.clj b/test/eca/features/chat/tool_calls_test.clj
@@ -276,7 +276,78 @@
                        :approval-override nil
                        :hook-rejected? false
                        :arguments-modified? true}
-                      plan)))))))
+                      plan))))))
+
+  (testing "ask_user with allowed config sends :ask to preToolCall hook"
+    (h/reset-components!)
+    (let [tool-call {:id "call-1"
+                     :full-name "eca__ask_user"
+                     :arguments {"question" "Why?"}}
+          all-tools [{:name "ask_user"
+                      :full-name "eca__ask_user"
+                      :origin :eca
+                      :server {:name "eca"}}]
+          db (h/db)
+          config (h/config)
+          agent-name :default
+          chat-id "test-chat"
+          hook-data* (atom nil)]
+      (with-redefs [f.tools/approval (constantly :allow)
+                    f.hooks/trigger-if-matches! (fn [event data _ _ _]
+                                                  (when (= event :preToolCall)
+                                                    (reset! hook-data* data)))]
+        (let [plan (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id)]
+          (is (= :ask (:approval @hook-data*))
+              "preToolCall hook receives :ask so notification hooks fire while waiting on user")
+          (is (match? {:decision :allow
+                       :hook-rejected? false}
+                      plan)
+              "tool actually executes (:allow) so the question still reaches the user")))))
+
+  (testing "ask_user with denied config keeps :deny in preToolCall hook"
+    (h/reset-components!)
+    (let [tool-call {:id "call-1"
+                     :full-name "eca__ask_user"
+                     :arguments {"question" "Why?"}}
+          all-tools [{:name "ask_user"
+                      :full-name "eca__ask_user"
+                      :origin :eca
+                      :server {:name "eca"}}]
+          db (h/db)
+          config (h/config)
+          agent-name :default
+          chat-id "test-chat"
+          hook-data* (atom nil)]
+      (with-redefs [f.tools/approval (constantly :deny)
+                    f.hooks/trigger-if-matches! (fn [event data _ _ _]
+                                                  (when (= event :preToolCall)
+                                                    (reset! hook-data* data)))]
+        (let [plan (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id)]
+          (is (= :deny (:approval @hook-data*))
+              "explicit :deny is preserved, not overridden to :ask")
+          (is (= :deny (:decision plan)))))))
+
+  (testing "non ask_user tool keeps :allow in preToolCall hook (regression)"
+    (h/reset-components!)
+    (let [tool-call {:id "call-1"
+                     :full-name "eca__test_tool"
+                     :arguments {:foo "bar"}}
+          all-tools [{:name "test_tool"
+                      :full-name "eca__test_tool"
+                      :origin :eca
+                      :server {:name "eca"}}]
+          db (h/db)
+          config (h/config)
+          agent-name :default
+          chat-id "test-chat"
+          hook-data* (atom nil)]
+      (with-redefs [f.tools/approval (constantly :allow)
+                    f.hooks/trigger-if-matches! (fn [event data _ _ _]
+                                                  (when (= event :preToolCall)
+                                                    (reset! hook-data* data)))]
+        (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id)
+        (is (= :allow (:approval @hook-data*))
+            "ask_user override does not leak to other tools")))))
 
 (deftest on-tools-called!-returns-provider-auth-test
   (testing "returns refreshed provider auth in result after token is renewed during tool execution"
diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj
