From 2bd8b7d4a759fd1042702f48456f67a2f868b587 Mon Sep 17 00:00:00 2001 From: Jakub Zika Date: Thu, 11 Jun 2026 11:45:10 +0200 Subject: [PATCH] Add chat status changed hook Add an advisory chatStatusChanged hook that emits aggregate chat status snapshots, including pending approvals, user questions, and running tool calls. Document the new hook behavior and add coverage for deduping, advisory hook output handling, prompt stopping, and tool-call state transitions. --- CHANGELOG.md | 1 + docs/config.json | 1 + docs/config/hooks.md | 43 ++++- src/eca/db.clj | 2 +- src/eca/features/chat.clj | 61 +++---- src/eca/features/chat/lifecycle.clj | 67 +++++++- src/eca/features/chat/tool_calls.clj | 2 + src/eca/features/commands.clj | 4 +- src/eca/features/hooks.clj | 2 +- src/eca/features/tools/agent.clj | 6 +- src/eca/handlers.clj | 12 +- test/eca/features/chat/lifecycle_test.clj | 158 ++++++++++++++++++ test/eca/features/chat_test.clj | 6 +- .../features/chat_tool_call_state_test.clj | 129 +++++++++++++- test/eca/features/tools/agent_test.clj | 2 +- 15 files changed, 438 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7888ff90..db97b13c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - MCP: recover remote servers with stale connections (e.g. after suspend): tool-call timeouts now probe and re-initialize; add `mcpKeepAliveSeconds` pings; 404 triggers re-init. - Update `github-copilot` default model to `gpt-5.5`. +- Add advisory `chatStatusChanged` hook reporting aggregate chat status (running/idle/stopping, awaiting-user-input, pending/running tool calls). ## 0.140.0 diff --git a/docs/config.json b/docs/config.json index 572713291..33bc3d1b2 100644 --- a/docs/config.json +++ b/docs/config.json @@ -861,6 +861,7 @@ "sessionEnd", "chatStart", "chatEnd", + "chatStatusChanged", "subagentStart", "subagentPostRequest", "preRequest", diff --git a/docs/config/hooks.md b/docs/config/hooks.md index 073eb8df4..f9a89773b 100644 --- a/docs/config/hooks.md +++ b/docs/config/hooks.md @@ -18,6 +18,7 @@ This page first explains the mechanics shared by **all** hooks, then provides a | [`sessionEnd`](#sessionend) | Server shutting down | Side effects | | [`chatStart`](#chatstart) | New or resumed chat | Inject context, stop the turn | | [`chatEnd`](#chatend) | Chat deleted | Side effects (advisory) | +| [`chatStatusChanged`](#chatstatuschanged) | Chat aggregate status changed | Side effects (advisory) | | [`subagentStart`](#subagentstart) | Before a subagent's first prompt | Inject context, stop the turn | | [`preRequest`](#prerequest) | Before prompt sent to LLM | Rewrite prompt, inject context, block | | [`postRequest`](#postrequest) | After a primary-agent prompt finished | Trigger a follow-up turn | @@ -125,8 +126,8 @@ These behave the same wherever they are supported: | Field | Type | Effect | |-------|------|--------| -| `systemMessage` | string | A standalone user-facing message, shown as `Hook '': ` on exit `0`, independent of `visible`/`suppressOutput`. No effect on `sessionStart`/`sessionEnd`/`chatEnd` (no chat UI). | -| `suppressOutput` | boolean | Hide only the execution block's body (stdout/stderr and effect lines). No effect on `sessionStart`/`sessionEnd`/`chatEnd` (no chat UI). | +| `systemMessage` | string | A standalone user-facing message, shown as `Hook '': ` on exit `0`, independent of `visible`/`suppressOutput`. No effect on `sessionStart`/`sessionEnd`/`chatEnd`/`chatStatusChanged` (no chat UI). | +| `suppressOutput` | boolean | Hide only the execution block's body (stdout/stderr and effect lines). No effect on `sessionStart`/`sessionEnd`/`chatEnd`/`chatStatusChanged` (no chat UI). | | `continue` | boolean | `false` stops the remaining hooks for the event; for turn-scoped hooks it also stops the turn. | | `stopReason` | string | User-only explanation shown when `continue: false` stops a turn. **Never sent to the LLM.** | @@ -139,7 +140,7 @@ The two ways a hook intervenes are **not** interchangeable: - **`continue: false`** (exit `0`) always stops ECA from running the **remaining hooks** of the same event. For **turn-scoped** hooks it *additionally* stops the current turn and surfaces `Turn stopped by hook '': ` (or `Turn stopped by hook ''.` with no reason). - **`exit 2`** is a surgical, hook-specific intervention (see each hook). It **never** stops the hook chain. -**Turn-scoped hooks** are `chatStart`, `subagentStart`, `preRequest`, `postRequest`, `subagentPostRequest`, `preCompact`, `postCompact`, `preToolCall`, and `postToolCall`. The rest — `sessionStart`, `sessionEnd`, `chatEnd` — only skip their remaining peers. +**Turn-scoped hooks** are `chatStart`, `subagentStart`, `preRequest`, `postRequest`, `subagentPostRequest`, `preCompact`, `postCompact`, `preToolCall`, and `postToolCall`. The rest — `sessionStart`, `sessionEnd`, `chatEnd`, `chatStatusChanged` — only skip their remaining peers. When a hook stops the turn it ends immediately: remaining `postRequest`/`subagentPostRequest` hooks do not run and no `followUp`/continuation fires. @@ -157,7 +158,7 @@ Two controls gate the block (`systemMessage` is unaffected by both): | `visible: false` | Hides the **whole** block — the hook run is fully silent. | | `suppressOutput: true` | Hides only the block **body**; the block header still appears so you can see the hook ran. | -Effect-only fields (`updatedInput`, `approval`, `replacedOutput`) change behavior without creating standalone messages, but a visible block still surfaces them as effect lines so you can see what changed. Hooks without a UI (`sessionStart`, `sessionEnd`, `chatEnd`) ignore `systemMessage`/`suppressOutput`. +Effect-only fields (`updatedInput`, `approval`, `replacedOutput`) change behavior without creating standalone messages, but a visible block still surfaces them as effect lines so you can see what changed. Hooks without a UI (`sessionStart`, `sessionEnd`, `chatEnd`, `chatStatusChanged`) ignore `systemMessage`/`suppressOutput`. ### Base input @@ -208,6 +209,40 @@ Fires when a chat is deleted. **Advisory and best-effort** — side effects only - **Honored output** — none. No `additionalContext`/`stopReason`/`suppressOutput` (no UI). `continue: false` only stops later `chatEnd` hooks; it does not affect deletion. - **Exit 2** — non-blocking error. +### `chatStatusChanged` + +Fires whenever the aggregate chat status snapshot changes. **Advisory and best-effort** — side effects only (notifications, dashboards, telemetry). + +- **Input adds** — `status`, `awaiting_user_input`, `pending_approval_tool_call_ids`, `pending_question_tool_call_ids`, `running_tool_call_ids`, and, only while blocked on the user, `waiting_reason` (`toolApproval` or `userQuestion`). Subagent chats also include `parent_chat_id`. +- **Honored output** — none. No `additionalContext`/`updatedInput`/`approval`/`replacedOutput`/`stopReason`/`systemMessage`/`suppressOutput` (no UI). `continue: false` only stops later `chatStatusChanged` hooks for the same event; it does not affect the chat lifecycle and cannot suppress later status emissions. +- **Exit 2** — non-blocking error. + +Each payload is a full snapshot, never a delta, and the hook fires only when the snapshot actually changed — it never receives a duplicate of the previous snapshot. A consumer that attaches mid-turn may see nothing until the next transition. + +This hook can fire many times per turn (every tool start/end changes the aggregate). Keep the script fast: + +```bash +#!/usr/bin/env bash +payload=$(cat) +( slow-notify "$payload" & ) # detach; exit immediately +exit 0 +``` + +Example configuration: + +```json +{ + "hooks": { + "notify-status": { + "type": "chatStatusChanged", + "actions": [ + {"type": "shell", "file": "~/.config/eca/hooks/chat-status.sh"} + ] + } + } +} +``` + ### `subagentStart` Fires before a subagent's first prompt. Use to inject context scoped to the subagent. diff --git a/src/eca/db.clj b/src/eca/db.clj index ad20d9e51..acb179f94 100644 --- a/src/eca/db.clj +++ b/src/eca/db.clj @@ -339,7 +339,7 @@ ;; error before any token arrived. Cleanup of ;; stale chats is handled by ;; cleanup-old-chats! instead. - (-> (update-vals chats #(dissoc % :tool-calls)) + (-> (update-vals chats #(dissoc % :tool-calls :last-status-payload)) stamp-chat-ids))))) (defn ^:private normalize-db-for-global-write [db] diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index 0f787af2f..9ca9fa71e 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -746,6 +746,7 @@ (swap! db* update-in [:chats chat-id] dissoc :prompt-finished?) (swap! db* assoc-in [:chats chat-id :updated-at] (System/currentTimeMillis)) (messenger/chat-status-changed messenger {:chat-id chat-id :status :running}) + (lifecycle/trigger-chat-status-hook! chat-ctx) (swap! db* assoc-in [:chats chat-id :prompt-id] prompt-id) (swap! db* assoc-in [:chats chat-id :model] full-model) ;; Persist the variant the chat is currently using so subsequent @@ -1245,7 +1246,8 @@ ;; Only notify client if finish-chat-prompt! hasn't already run, ;; otherwise the belated statusChanged causes duplicate finished handling. (when-not (get-in @db* [:chats chat-id :prompt-finished?]) - (messenger/chat-status-changed (:messenger chat-ctx) {:chat-id chat-id :status :idle})) + (messenger/chat-status-changed (:messenger chat-ctx) {:chat-id chat-id :status :idle}) + (lifecycle/trigger-chat-status-hook! chat-ctx)) (db/update-workspaces-cache! @db* metrics)))))))))) (defn ^:private send-mcp-prompt! @@ -1583,13 +1585,15 @@ :model "error" :status :error}))))))) -(defn tool-call-approve [{:keys [chat-id tool-call-id save]} db* messenger metrics] +(defn tool-call-approve + [{:keys [chat-id tool-call-id save]} db* messenger config metrics] (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) (if-not (get-in @db* [:chats chat-id :tool-calls tool-call-id]) (logger/warn logger-tag "tool-call-approve ignored: unknown chat or tool-call" {:chat-id chat-id :tool-call-id tool-call-id}) (let [chat-ctx {:chat-id chat-id :db* db* + :config config :metrics metrics :messenger messenger}] (tc/transition-tool-call! db* chat-ctx tool-call-id :user-approve @@ -1599,13 +1603,15 @@ (let [tool-call-name (get-in @db* [:chats chat-id :tool-calls tool-call-id :name])] (swap! db* assoc-in [:tool-calls tool-call-name :remember-to-approve?] true))))))) -(defn tool-call-reject [{:keys [chat-id tool-call-id]} db* messenger metrics] +(defn tool-call-reject + [{:keys [chat-id tool-call-id]} db* messenger config metrics] (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) (if-not (get-in @db* [:chats chat-id :tool-calls tool-call-id]) (logger/warn logger-tag "tool-call-reject ignored: unknown chat or tool-call" {:chat-id chat-id :tool-call-id tool-call-id}) (let [chat-ctx {:chat-id chat-id :db* db* + :config config :metrics metrics :messenger messenger}] (tc/transition-tool-call! db* chat-ctx tool-call-id :user-reject @@ -1667,31 +1673,30 @@ (logger/info logger-tag "Steer message removed" {:chat-id chat-id}))))) (defn prompt-stop - ([params db* messenger metrics] - (prompt-stop params db* messenger metrics {})) - ([{:keys [chat-id]} db* messenger metrics {:keys [silent?]}] - (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) - (when (identical? :running (get-in @db* [:chats chat-id :status])) - ;; Set :stopping immediately to prevent race with stream callbacks - ;; that check status via assert-chat-not-stopped! or cancelled? - (swap! db* assoc-in [:chats chat-id :status] :stopping) - (let [chat-ctx {:chat-id chat-id - :db* db* - :metrics metrics - :messenger messenger - :parent-chat-id (db/parent-chat-id @db* chat-id)}] - (when-not silent? - (lifecycle/send-content! chat-ctx :system {:type :text - :text "\nPrompt stopped"})) - - ;; Handle each active tool call - (doseq [[tool-call-id _] (tc/get-active-tool-calls @db* chat-id)] - (tc/transition-tool-call! db* chat-ctx tool-call-id :stop-requested - {:reason {:code :user-prompt-stop - :text "Tool call rejected because of user prompt stop"}})) - ;; Clear compacting flags so finish-chat-prompt! isn't blocked - (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?) - (lifecycle/finish-chat-prompt! :stopping (lifecycle/strip-hook-callbacks chat-ctx))))))) + [{:keys [chat-id]} db* messenger config metrics & {:keys [silent?]}] + (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) + (when (identical? :running (get-in @db* [:chats chat-id :status])) + ;; Set :stopping immediately to prevent race with stream callbacks + ;; that check status via assert-chat-not-stopped! or cancelled? + (swap! db* assoc-in [:chats chat-id :status] :stopping) + (let [chat-ctx {:chat-id chat-id + :db* db* + :config config + :metrics metrics + :messenger messenger + :parent-chat-id (db/parent-chat-id @db* chat-id)}] + (when-not silent? + (lifecycle/send-content! chat-ctx :system {:type :text + :text "\nPrompt stopped"})) + + ;; Handle each active tool call + (doseq [[tool-call-id _] (tc/get-active-tool-calls @db* chat-id)] + (tc/transition-tool-call! db* chat-ctx tool-call-id :stop-requested + {:reason {:code :user-prompt-stop + :text "Tool call rejected because of user prompt stop"}})) + ;; Clear compacting flags so finish-chat-prompt! isn't blocked + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?) + (lifecycle/finish-chat-prompt! :stopping (lifecycle/strip-hook-callbacks chat-ctx)))))) (defn delete-chat [{:keys [chat-id]} db* messenger config metrics] diff --git a/src/eca/features/chat/lifecycle.clj b/src/eca/features/chat/lifecycle.clj index a42cf5c75..8437f6102 100644 --- a/src/eca/features/chat/lifecycle.clj +++ b/src/eca/features/chat/lifecycle.clj @@ -104,9 +104,9 @@ :id id}))) (defn notify-after-hook-action! [chat-ctx {:keys [id name parsed raw-output raw-error exit type hook-type visible? tool-call-id]}] - (let [advisory-chat-end? (= :chatEnd hook-type) - ;; JSON output fields are only honored on exit 0 (and never for chatEnd). - parsed-effects (when (and (not advisory-chat-end?) (zero? exit)) parsed) + (let [advisory-chat-hook? (contains? #{:chatEnd :chatStatusChanged} hook-type) + ;; JSON output fields are only honored on exit 0 (and never for advisory chat hooks). + parsed-effects (when (and (not advisory-chat-hook?) (zero? exit)) parsed) suppress? (boolean (get parsed-effects "suppressOutput"))] ;; Two independent channels: ;; 1. Execution block, gated by `visible?`. If a started event was sent we must @@ -362,11 +362,72 @@ :stop-reason (:stop-reason stop-result) :stop-hook-name (:stop-hook-name stop-result)})) +(defn ^:private chat-status-payload + "Build the aggregate chatStatusChanged hook payload from a db snapshot." + [db chat-id] + (let [chat (get-in db [:chats chat-id]) + tool-calls (:tool-calls chat) + ask-user? (fn [tc] + (and (= "ask_user" (:name tc)) + (= "eca" (:server tc)))) + ids-by (fn [pred] + (->> tool-calls + (filter (fn [[_ tc]] (pred tc))) + (map key) + sort + vec)) + pending-approval (ids-by #(= :waiting-approval (:status %))) + pending-question (ids-by #(and (= :executing (:status %)) (ask-user? %))) + running (ids-by #(and (= :executing (:status %)) (not (ask-user? %)))) + awaiting? (boolean (or (seq pending-approval) (seq pending-question)))] + (cond-> {:chat-id chat-id + :status (or (:status chat) :idle) + :awaiting-user-input awaiting? + :pending-approval-tool-call-ids pending-approval + :pending-question-tool-call-ids pending-question + :running-tool-call-ids running} + awaiting? (assoc :waiting-reason (if (seq pending-approval) + "toolApproval" + "userQuestion"))))) + +(defn trigger-chat-status-hook! + "Trigger the advisory chatStatusChanged hook when the aggregate chat status + payload changed since the last trigger. + + The protocol `chat/statusChanged` notification is emitted separately and + carries only the chat id and status. This hook receives the aggregate status, + including awaiting-user-input and tool-call ids. + + Dedup state is stored at [:chats chat-id :last-status-payload]. Missing chats + are ignored." + [{:keys [db* chat-id config] :as chat-ctx}] + (when (some #(= :chatStatusChanged (keyword (:type %))) (vals (:hooks config))) + (let [path [:chats chat-id :last-status-payload] + ;; Compute inside the swap so the stored payload matches the db snapshot. + [old-db new-db] (swap-vals! db* (fn [db] + (if (get-in db [:chats chat-id]) + (assoc-in db path (chat-status-payload db chat-id)) + db))) + payload (get-in new-db path)] + (when (and (get-in new-db [:chats chat-id]) + (not= (get-in old-db path) payload)) + (f.hooks/trigger-if-matches! + :chatStatusChanged + (merge (f.hooks/chat-hook-data new-db chat-ctx) + (when (get-in new-db [:chats chat-id :subagent]) + {:parent-chat-id (db/parent-chat-id new-db chat-id)}) + (dissoc payload :chat-id)) + ;; No action callbacks: advisory hooks do not render in the chat UI. + {} + new-db + config))))) + (defn ^:private apply-status-transition! "Update chat status, send status/progress events, set created-at if needed." [{:keys [chat-id db* messenger] :as chat-ctx} status] (swap! db* assoc-in [:chats chat-id :status] status) (messenger/chat-status-changed messenger {:chat-id chat-id :status status}) + (trigger-chat-status-hook! chat-ctx) (send-content! chat-ctx :system {:type :progress :state :finished}) diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj index 53c2358c1..938cd3311 100644 --- a/src/eca/features/chat/tool_calls.clj +++ b/src/eca/features/chat/tool_calls.clj @@ -586,6 +586,8 @@ (doseq [action actions] (execute-action! action db* chat-ctx tool-call-id event-data)) + (lifecycle/trigger-chat-status-hook! (assoc chat-ctx :db* db*)) + {:status status :actions actions})) (def ^:private hook-approval-rank diff --git a/src/eca/features/commands.clj b/src/eca/features/commands.clj index 8b374abe4..a92f44e18 100644 --- a/src/eca/features/commands.clj +++ b/src/eca/features/commands.clj @@ -295,8 +295,8 @@ (def ^:private hook-type-order "Display order for hook types in /hooks output." - ["sessionStart" "sessionEnd" "chatStart" "chatEnd" "subagentStart" "preRequest" - "postRequest" "subagentPostRequest" "preToolCall" "postToolCall" + ["sessionStart" "sessionEnd" "chatStart" "chatEnd" "chatStatusChanged" "subagentStart" + "preRequest" "postRequest" "subagentPostRequest" "preToolCall" "postToolCall" "preCompact" "postCompact"]) (defn ^:private hooks-msg [config] diff --git a/src/eca/features/hooks.clj b/src/eca/features/hooks.clj index 16808c970..114bcd523 100644 --- a/src/eca/features/hooks.clj +++ b/src/eca/features/hooks.clj @@ -202,7 +202,7 @@ (defn run-hook-action! "Execute a single hook action. Supported hook types: - :sessionStart, :sessionEnd (session lifecycle) - - :chatStart, :chatEnd (chat lifecycle) + - :chatStart, :chatEnd, :chatStatusChanged (chat lifecycle) - :subagentStart, :subagentPostRequest (subagent lifecycle) - :preRequest, :postRequest (prompt lifecycle) - :preToolCall, :postToolCall (tool lifecycle) diff --git a/src/eca/features/tools/agent.clj b/src/eca/features/tools/agent.clj index 9f27988c0..06553fe73 100644 --- a/src/eca/features/tools/agent.clj +++ b/src/eca/features/tools/agent.clj @@ -92,10 +92,10 @@ (defn ^:private stop-subagent-chat! "Stop a running subagent chat silently (parent already shows 'Prompt stopped')." - [db* messenger metrics subagent-chat-id agent-name] + [db* messenger config metrics subagent-chat-id agent-name] (let [prompt-stop (requiring-resolve 'eca.features.chat/prompt-stop)] (try - (prompt-stop {:chat-id subagent-chat-id} db* messenger metrics {:silent? true}) + (prompt-stop {:chat-id subagent-chat-id} db* messenger config metrics {:silent? true}) (catch Exception e (logger/warn logger-tag (format "Error stopping subagent '%s': %s" agent-name (.getMessage e))))))) @@ -211,7 +211,7 @@ ;; Wait for subagent to complete by polling status (let [stopped-result (fn [] (logger/info logger-tag (format "Agent '%s' stopped by parent chat" agent-name)) - (stop-subagent-chat! db* messenger metrics subagent-chat-id agent-name) + (stop-subagent-chat! db* messenger config metrics subagent-chat-id agent-name) {:error true :contents [{:type :text :text (format "Agent '%s' was stopped because the parent chat was stopped." agent-name)}]})] diff --git a/src/eca/handlers.clj b/src/eca/handlers.clj index eb8136aa5..594441e88 100644 --- a/src/eca/handlers.clj +++ b/src/eca/handlers.clj @@ -223,17 +223,17 @@ (metrics/task metrics :eca/chat-query-commands (f.chat/query-commands params db* config))) -(defn chat-tool-call-approve [{:keys [messenger db* metrics]} params] +(defn chat-tool-call-approve [{:keys [messenger db* config metrics]} params] (metrics/task metrics :eca/chat-tool-call-approve - (f.chat/tool-call-approve params db* messenger metrics))) + (f.chat/tool-call-approve params db* messenger config metrics))) -(defn chat-tool-call-reject [{:keys [messenger db* metrics]} params] +(defn chat-tool-call-reject [{:keys [messenger db* config metrics]} params] (metrics/task metrics :eca/chat-tool-call-reject - (f.chat/tool-call-reject params db* messenger metrics))) + (f.chat/tool-call-reject params db* messenger config metrics))) -(defn chat-prompt-stop [{:keys [db* messenger metrics]} params] +(defn chat-prompt-stop [{:keys [db* messenger config metrics]} params] (metrics/task metrics :eca/chat-prompt-stop - (f.chat/prompt-stop params db* messenger metrics))) + (f.chat/prompt-stop params db* messenger config metrics {}))) (defn chat-prompt-steer [{:keys [db* metrics]} params] (metrics/task metrics :eca/chat-prompt-steer diff --git a/test/eca/features/chat/lifecycle_test.clj b/test/eca/features/chat/lifecycle_test.clj index c6bc6906d..6778e807e 100644 --- a/test/eca/features/chat/lifecycle_test.clj +++ b/test/eca/features/chat/lifecycle_test.clj @@ -1,7 +1,10 @@ (ns eca.features.chat.lifecycle-test (:require + [cheshire.core :as json] [clojure.test :refer [are deftest is testing]] [eca.features.chat.lifecycle :as lifecycle] + [eca.features.hooks :as f.hooks] + [eca.test-helper :as h] [matcher-combinators.test :refer [match?]])) (set! *warn-on-reflection* true) @@ -11,6 +14,111 @@ (def ^:private full-model "openai/test") (def ^:private config {:autoCompactPercentage 75}) +(deftest chat-status-payload-test + (testing "computes sorted aggregate status fields and classifies ask_user separately" + (let [db {:chats {chat-id {:status :running + :tool-calls {"tool-z" {:status :waiting-approval + :name "write_file" + :server "eca"} + "tool-a" {:status :waiting-approval + :name "shell" + :server "eca"} + "question" {:status :executing + :name "ask_user" + :server "eca"} + "other-question" {:status :executing + :name "ask_user" + :server "other"} + "running" {:status :executing + :name "shell" + :server "eca"} + "done" {:status :completed + :name "shell" + :server "eca"}}}}}] + (is (match? {:chat-id chat-id + :status :running + :awaiting-user-input true + :waiting-reason "toolApproval" + :pending-approval-tool-call-ids ["tool-a" "tool-z"] + :pending-question-tool-call-ids ["question"] + :running-tool-call-ids ["other-question" "running"]} + (#'lifecycle/chat-status-payload db chat-id))))) + + (testing "omits waiting reason when nothing is awaiting user input" + (let [payload (#'lifecycle/chat-status-payload + {:chats {chat-id {:status :idle + :tool-calls {"done" {:status :completed}}}}} + chat-id)] + (is (match? {:awaiting-user-input false + :pending-approval-tool-call-ids [] + :pending-question-tool-call-ids [] + :running-tool-call-ids []} + payload)) + (is (not (contains? payload :waiting-reason)))))) + +(deftest trigger-chat-status-hook!-dedups-and-runs-advisory-hook-test + (h/reset-components!) + (h/config! {:hooks {"a-stop" {:type "chatStatusChanged" + :actions [{:type "shell" :shell "status"}]} + "b-after" {:type "chatStatusChanged" + :actions [{:type "shell" :shell "after"}]}}}) + (let [db* (h/db*) + calls* (atom []) + hook-inputs* (atom []) + ctx {:chat-id chat-id + :db* db* + :messenger (h/messenger) + :config (h/config) + :agent "plan" + :full-model full-model + :variant "fast"}] + (swap! db* assoc-in [:chats chat-id] + {:id chat-id + :status :running + :tool-calls {"tool-2" {:status :waiting-approval + :name "shell" + :server "eca"}}}) + (with-redefs [f.hooks/run-shell-cmd + (fn [{:keys [input]}] + (let [payload (json/parse-string input true) + hook-name (:hook_name payload)] + (swap! calls* conj hook-name) + (swap! hook-inputs* conj payload) + (if (= "a-stop" hook-name) + {:exit 0 :out "{\"continue\":false,\"systemMessage\":\"ignored\"}" :err nil} + {:exit 0 :out "after" :err nil})))] + (lifecycle/trigger-chat-status-hook! ctx) + (lifecycle/trigger-chat-status-hook! ctx) + (is (nil? (:chat-status-changed (h/messages))) + "hook-only helper does not emit the protocol notification") + (is (= ["a-stop"] @calls*) + "unchanged aggregate is deduped; continue:false only stops later chatStatusChanged hooks for this event") + (is (nil? (:chat-content-received (h/messages))) + "advisory status hooks render no chat UI") + (is (match? {:hook_name "a-stop" + :hook_type "chatStatusChanged" + :chat_id chat-id + :agent "plan" + :full_model full-model + :variant "fast" + :status "running" + :awaiting_user_input true + :waiting_reason "toolApproval" + :pending_approval_tool_call_ids ["tool-2"] + :pending_question_tool_call_ids [] + :running_tool_call_ids []} + (first @hook-inputs*))) + + (swap! db* assoc-in [:chats chat-id] + {:id chat-id :status :idle :tool-calls {}}) + (lifecycle/trigger-chat-status-hook! ctx) + (is (= ["a-stop" "a-stop"] @calls*) + "an actual aggregate change fires the hooks again") + (is (match? {:status "idle" + :awaiting_user_input false + :pending_approval_tool_call_ids []} + (last @hook-inputs*)))))) + (defn- db-with [usage model-caps] {:chats {chat-id {:usage usage}} :models {full-model model-caps}}) @@ -220,3 +328,53 @@ (is (not (contains? (:ctx @captured*) :on-after-finish!))) ;; on-follow-up is preserved (but skip-post-request-hooks? means it won't fire) (is (contains? (:ctx @captured*) :on-follow-up))))) + +(deftest trigger-chat-status-hook!-exit-2-is-non-blocking-test + (testing "chatStatusChanged hook exit 2 is logged but does not stop later hooks" + (h/reset-components!) + ;; Names are chosen so aaa-exit2 runs before bbb-after. + (h/config! {:hooks {"aaa-exit2" {:type "chatStatusChanged" + :actions [{:type "shell" :shell "exit2"}]} + "bbb-after" {:type "chatStatusChanged" + :actions [{:type "shell" :shell "after"}]}}}) + (let [db* (h/db*) + calls* (atom []) + ctx {:chat-id chat-id + :db* db* + :messenger (h/messenger) + :config (h/config) + :agent "default" + :full-model full-model}] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :status :running :tool-calls {}}) + (with-redefs [f.hooks/run-shell-cmd + (fn [{:keys [input]}] + (let [hook-name (:hook_name (json/parse-string input true))] + (swap! calls* conj hook-name) + (if (= "aaa-exit2" hook-name) + {:exit 2 :out nil :err "something went wrong"} + {:exit 0 :out nil :err nil})))] + (lifecycle/trigger-chat-status-hook! ctx) + (is (= ["aaa-exit2" "bbb-after"] @calls*) + "exit 2 does not stop later hooks in the chain") + (is (nil? (:chat-content-received (h/messages))) + "exit 2 hook produces no chat content"))))) + +(deftest finish-chat-prompt!-always-emits-protocol-status-test + (testing "every turn finish emits the protocol chat/statusChanged even when the aggregate is unchanged (command-only turns stay idle throughout)" + (h/reset-components!) + (let [db* (h/db*) + ctx {:chat-id chat-id + :db* db* + :messenger (h/messenger) + :config (h/config) + :metrics (h/metrics) + :agent "default"}] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :status :idle}) + ;; First command-style finish: idle -> idle. + (lifecycle/finish-chat-prompt! :idle ctx) + ;; Simulate the next command turn: prompt entry clears :prompt-finished?. + (swap! db* update-in [:chats chat-id] dissoc :prompt-finished?) + (lifecycle/finish-chat-prompt! :idle ctx) + (is (= 2 (count (:chat-status-changed (h/messages)))) + "both finishes must notify clients despite the unchanged idle payload") + (is (every? #(= :idle (:status %)) (:chat-status-changed (h/messages))))))) diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index cc06f69d8..75a7376b0 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -1048,7 +1048,7 @@ (when (= :timeout (deref wait-for-tool3 10000 :timeout)) (println "tool-calls-with-prompt-stop-test: deref in prompt stop future timed out")) (Thread/sleep 50) - (f.chat/prompt-stop {:chat-id chat-id} (h/db*) (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} (h/db*) (h/messenger) (h/config) (h/metrics) {}) (deliver wait-for-stop true)) (on-tools-called [{:id "call-1" :full-name "eca__ro_tool_1" :arguments {}} {:id "call-2" :full-name "eca__ro_tool_2" :arguments {}} @@ -1718,7 +1718,7 @@ (testing "Approving a tool-call against an unknown chat does not throw or mutate state" (h/reset-components!) (f.chat/tool-call-approve {:chat-id "unknown-chat" :tool-call-id "t1"} - (h/db*) (h/messenger) (h/metrics)) + (h/db*) (h/messenger) (h/config) (h/metrics)) (is (= {} (:chats (h/db)))) (is (= {} (h/messages))))) @@ -1726,7 +1726,7 @@ (testing "Rejecting a tool-call against an unknown chat does not throw or mutate state" (h/reset-components!) (f.chat/tool-call-reject {:chat-id "unknown-chat" :tool-call-id "t1"} - (h/db*) (h/messenger) (h/metrics)) + (h/db*) (h/messenger) (h/config) (h/metrics)) (is (= {} (:chats (h/db)))) (is (= {} (h/messages))))) diff --git a/test/eca/features/chat_tool_call_state_test.clj b/test/eca/features/chat_tool_call_state_test.clj index 04d955ed3..5e21da890 100644 --- a/test/eca/features/chat_tool_call_state_test.clj +++ b/test/eca/features/chat_tool_call_state_test.clj @@ -1,10 +1,12 @@ (ns eca.features.chat-tool-call-state-test "A namespace for testing the tool call state transitions." (:require + [cheshire.core :as json] [clojure.string :as s] [clojure.test :refer [deftest is testing]] [eca.features.chat :as f.chat] [eca.features.chat.tool-calls :as tc] + [eca.features.hooks :as f.hooks] [eca.test-helper :as h] [matcher-combinators.test :refer [match?]])) @@ -641,6 +643,121 @@ (is (= :cleanup (:status (#'tc/get-tool-call-state @db* chat-id "tool-3"))) "Expected tool call state to be in :cleanup status"))))))) +(deftest transition-tool-call-triggers-chat-status-hook-test + (testing "tool call transitions trigger the chatStatusChanged hook with aggregate snapshots at the choke point" + (h/reset-components!) + (h/config! {:hooks {"status" {:type "chatStatusChanged" + :actions [{:type "shell" :shell "status"}]}}}) + (let [db* (h/db*) + chat-id "status-chat" + hook-inputs* (atom []) + chat-ctx {:chat-id chat-id + :request-id "req-status" + :messenger (h/messenger) + :config (h/config)} + last-hook #(last @hook-inputs*) + prepare! (fn [tool-call-id name server] + (#'tc/transition-tool-call! db* chat-ctx tool-call-id :tool-prepare + {:name name + :server server + :origin :native + :arguments-text "{}"})) + run! (fn [tool-call-id name server] + (#'tc/transition-tool-call! db* chat-ctx tool-call-id :tool-run + {:approved?* (promise) + :future-cleanup-complete?* (promise) + :name name + :server server + :origin :native + :arguments {} + :manual-approval true}))] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :status :running}) + (with-redefs [f.hooks/run-shell-cmd + (fn [{:keys [input]}] + (swap! hook-inputs* conj (json/parse-string input true)) + {:exit 0 :out nil :err nil})] + ;; Hook stdin keys are snake_case and :status is serialized as a string. + (prepare! "tool-1" "shell" "eca") + (run! "tool-1" "shell" "eca") + (#'tc/transition-tool-call! db* chat-ctx "tool-1" :approval-ask + {:progress-text "Waiting for tool call approval"}) + (is (match? {:status "running" + :awaiting_user_input true + :waiting_reason "toolApproval" + :pending_approval_tool_call_ids ["tool-1"] + :pending_question_tool_call_ids [] + :running_tool_call_ids []} + (last-hook))) + + (prepare! "tool-2" "shell" "eca") + (run! "tool-2" "shell" "eca") + (#'tc/transition-tool-call! db* chat-ctx "tool-2" :approval-ask + {:progress-text "Waiting for tool call approval"}) + (is (match? {:pending_approval_tool_call_ids ["tool-1" "tool-2"] + :awaiting_user_input true} + (last-hook))) + + (#'tc/transition-tool-call! db* chat-ctx "tool-1" :user-approve + {:reason {:code :user-choice-allow + :text "Tool call allowed by user choice"}}) + (is (match? {:pending_approval_tool_call_ids ["tool-2"] + :awaiting_user_input true} + (last-hook))) + + (#'tc/transition-tool-call! db* chat-ctx "tool-2" :user-approve + {:reason {:code :user-choice-allow + :text "Tool call allowed by user choice"}}) + (is (match? {:awaiting_user_input false + :pending_approval_tool_call_ids []} + (last-hook))) + + (prepare! "question" "ask_user" "eca") + (run! "question" "ask_user" "eca") + (#'tc/transition-tool-call! db* chat-ctx "question" :approval-allow + {:reason {:code :user-config-allow + :text "allowed"}}) + (#'tc/transition-tool-call! db* chat-ctx "question" :execution-start + {:delayed-future (delay nil) + :origin :native + :name "ask_user" + :server "eca" + :arguments {} + :start-time (System/currentTimeMillis) + :progress-text "Waiting for user"}) + (is (match? {:awaiting_user_input true + :waiting_reason "userQuestion" + :pending_question_tool_call_ids ["question"] + :running_tool_call_ids []} + (last-hook))) + + (#'tc/transition-tool-call! db* chat-ctx "question" :execution-end + {:origin :native + :name "ask_user" + :server "eca" + :arguments {} + :error false + :outputs [] + :total-time-ms 1 + :progress-text "Generating"}) + (is (match? {:awaiting_user_input false + :pending_question_tool_call_ids [] + :running_tool_call_ids []} + (last-hook))) + + (is (nil? (:chat-status-changed (h/messages))) + "tool-call transitions do not emit the protocol notification"))))) + +(deftest prompt-stop-emits-stopping-status-test + (testing "prompt-stop transitions to :stopping and emits the protocol status via finish" + (h/reset-components!) + (let [db* (h/db*) + chat-id "stopping-chat"] + (swap! db* assoc-in [:chats chat-id] {:id chat-id :status :running}) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) + ;; The protocol notification carries only the final stopping status. + (is (match? [{:chat-id chat-id :status :stopping}] + (:chat-status-changed (h/messages))))))) + (deftest test-stop-prompt-messages ;; Test what messages are sent when stop-prompt is called (testing "should send toolCallRejected for active tool calls" @@ -658,7 +775,7 @@ (#'tc/transition-tool-call! db* chat-ctx "tool-2" :tool-prepare {:name "read_file" :origin "filesystem" :arguments-text "{}"}) - (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) (let [messages (h/messages) chat-messages (:chat-content-received messages) @@ -694,7 +811,7 @@ (#'tc/transition-tool-call! db* chat-ctx "tool-1" :tool-prepare {:name "list_files" :origin "filesystem" :arguments-text "{}"}) - (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) (let [message-count (count (:chat-content-received (h/messages)))] (is (< 1 message-count) @@ -721,7 +838,7 @@ :name "list_files" :origin "filesystem" :arguments {"id" 123 "value" 42}}) - (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) (when-not @approved?* (#'tc/transition-tool-call! db* chat-ctx tool-call-id :send-reject {:approved?* approved?* @@ -748,7 +865,7 @@ (swap! db* assoc-in [:chats chat-id] {:status :idle}) ; Not running - (is (nil? (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics))) + (is (nil? (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {})) "Expected nil return value for non-running chat") (is (= 0 (count (:chat-content-received (h/messages)))) @@ -773,7 +890,7 @@ (#'tc/transition-tool-call! db* chat-ctx "tool-2" :tool-prepare {:name "read_file" :origin "filesystem" :arguments-text "{}"}) - (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) (let [chat-messages (:chat-content-received (h/messages))] @@ -796,7 +913,7 @@ (swap! db* assoc-in [:chats chat-id] {:status :running}) - (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/metrics)) + (f.chat/prompt-stop {:chat-id chat-id} db* (h/messenger) (h/config) (h/metrics) {}) (let [final-status (get-in @db* [:chats chat-id :status])] (is (= :stopping final-status) "Expected status to change to :stopping after stop processing"))))) diff --git a/test/eca/features/tools/agent_test.clj b/test/eca/features/tools/agent_test.clj index 456799bd4..2a91e9bd4 100644 --- a/test/eca/features/tools/agent_test.clj +++ b/test/eca/features/tools/agent_test.clj @@ -264,7 +264,7 @@ ;; Signal parent stop so the poll loop picks it up (reset! call-state* {:status :stopping})) eca.features.chat/prompt-stop - (fn [_params _db* _messenger _metrics] + (fn [_params _db* _messenger _config _metrics _opts] (swap! db* assoc-in [:chats subagent-chat-id :status] :idle)) (clojure.lang.RT/var (namespace sym) (name sym))))] (let [result ((spawn-handler)