From 0269a05b4862f23afde9c5d64c2d15f4b2a704bb Mon Sep 17 00:00:00 2001 From: Jakub Zika Date: Thu, 4 Jun 2026 13:57:14 +0200 Subject: [PATCH] expand-hook-system Add `preCompact`, `postCompact` and `subagentStart` hooks; subagents now fire `subagentStart` instead of `chatStart`, and both `postRequest` and `subagentPostRequest` run for subagents. - Add `/hooks` command and an optional `description` field in hook config; prefix plugin hooks with `plugin-name::`. - Expand hook contracts: `response` replaces `prompt`, plain-text `tool_response`, `continue:false` everywhere, `followUp`, `replacedOutput`, standalone `systemMessage`, and exact-string matchers. - Add `tool_call_id` to `preToolCall`/`postToolCall` input, plus `cwd`, `session_id`, `eca_executable`, `full_model`, `variant` and `follow_up_active` to hook input data. - Fix `postToolCall continue:false` leaking across turns (now prompt-id scoped), `chatStart` `additionalContext` being dropped, and `preRequest` exit-2 not naming the blocking hook. --- CHANGELOG.md | 6 +- docs/config.json | 17 +- docs/config/examples.md | 8 +- docs/config/hooks.md | 415 ++++- docs/features.md | 1 + docs/protocol.md | 6 + .../integration/chat/commands_test.clj | 1 + .../integration/chat/hooks_test.clj | 26 +- src/eca/db.clj | 3 + src/eca/features/chat.clj | 588 ++++--- src/eca/features/chat/lifecycle.clj | 435 ++++- src/eca/features/chat/tool_calls.clj | 663 ++++--- src/eca/features/commands.clj | 80 +- src/eca/features/hooks.clj | 160 +- src/eca/features/plugins.clj | 18 +- src/eca/features/prompt.clj | 11 +- src/eca/features/tools/shell.clj | 97 +- src/eca/features/tools/util.clj | 29 +- src/eca/shared.clj | 21 + test/eca/features/chat/lifecycle_test.clj | 159 +- test/eca/features/chat/tool_calls_test.clj | 275 ++- test/eca/features/chat_test.clj | 62 + test/eca/features/commands_test.clj | 50 + test/eca/features/hooks_test.clj | 1568 ++++++++++++++++- test/eca/features/plugins_test.clj | 5 +- test/eca/features/prompt_test.clj | 11 + test/eca/llm_providers/anthropic_test.clj | 4 +- test/eca/shared_test.clj | 15 +- 28 files changed, 3944 insertions(+), 790 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c981d3e6..713e5f5cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,12 @@ ## Unreleased - Add OpenAI Responses API for GitHub Copilot models that require it (gpt-5.5, gpt-5.4-mini). - - MCP OAuth: persist and reuse the dynamically-registered client on token refresh, so servers with non-idempotent DCR (e.g. RunLayer) refresh instead of forcing a browser re-login, and recover from expired-token tool errors automatically. +- Add `preCompact`, `postCompact` and `subagentStart` hooks; subagents no longer trigger `chatStart`. +- Add `/hooks` command with optional `description` field in hook config. +- Tool hooks (`preToolCall`/`postToolCall`) now include `tool_call_id` in input data. +- Expand hook contracts: `response` not `prompt`, plain-text `tool_response`, `continue:false` everywhere, `followUp`, `replacedOutput`, standalone `systemMessage`, exact-string matchers. +- Fix `postToolCall continue:false` leaking across turns, `chatStart` `additionalContext` dropped, and `preRequest` exit-2 naming the blocking hook. ## 0.138.1 diff --git a/docs/config.json b/docs/config.json index 6c6035d8d..b988f670b 100644 --- a/docs/config.json +++ b/docs/config.json @@ -855,20 +855,29 @@ "sessionEnd", "chatStart", "chatEnd", + "subagentStart", + "subagentPostRequest", "preRequest", "postRequest", + "preCompact", + "postCompact", "preToolCall", "postToolCall" ] }, + "description": { + "type": "string", + "description": "Brief explanation of the hook's purpose, shown by /hooks.", + "markdownDescription": "Brief explanation of the hook's purpose, shown by `/hooks`." + }, "matcher": { - "description": "Matches *ToolCall hooks; unmatched hooks are skipped. String: legacy regex against server__tool-name. Object: tool selector map with optional argsMatchers.", - "markdownDescription": "Matches `*ToolCall` hooks; unmatched hooks are skipped. **String**: legacy regex against `server__tool-name`. **Object**: tool selector map with optional `argsMatchers`.", + "description": "Matches *ToolCall hooks or compact hooks; unmatched hooks are skipped. For tool hooks, strings are legacy regex against server__tool-name and objects are tool selector maps with optional argsMatchers. For compact hooks, strings are exact matches against the triggered value (manual or auto); omitted matcher runs for both.", + "markdownDescription": "Matches `*ToolCall` hooks or compact hooks; unmatched hooks are skipped. For tool hooks, **String** is a legacy regex against `server__tool-name` and **Object** is a tool selector map with optional `argsMatchers`. For compact hooks, strings are matched as exact strings against the `triggered` value (`manual` or `auto`); omitted matcher runs for both.", "oneOf": [ { "type": "string", - "description": "Regex pattern for matching server__tool-name.", - "markdownDescription": "Regex pattern for matching `server__tool-name`." + "description": "Tool hooks: regex pattern for matching server__tool-name. Compact hooks: exact triggered value (manual or auto).", + "markdownDescription": "Tool hooks: regex pattern for matching `server__tool-name`. Compact hooks: exact `triggered` value (`manual` or `auto`)." }, { "type": "object", diff --git a/docs/config/examples.md b/docs/config/examples.md index 32663473c..457eaa0fd 100644 --- a/docs/config/examples.md +++ b/docs/config/examples.md @@ -298,12 +298,8 @@ If you think your config is relevant to be shared for other people, [open a pull data = json.loads(sys.argv[1]) hook_type = data.get('hook_type', '') - workspaces = data.get('workspaces', []) - cwd = workspaces[0] if workspaces else '' - # ECA does not provide a session_id; derive one from db_cache_path to get a - # stable per-session identifier, falling back to a constant. - db_path = data.get('db_cache_path', '') - session_id = db_path.split('/')[-2] if db_path else 'eca-default' + cwd = data.get('cwd', '') + session_id = data.get('session_id', 'eca-default') type_map = { 'sessionStart': 'SessionStart', diff --git a/docs/config/hooks.md b/docs/config/hooks.md index 061b9a09e..073eb8df4 100644 --- a/docs/config/hooks.md +++ b/docs/config/hooks.md @@ -6,62 +6,351 @@ description: "Configure ECA hooks: before/after event callbacks for tool calls a ![](../images/features/hooks.png) -Hooks are shell actions that run before or after specific events, useful for notifications, injecting context, modifying inputs, or blocking tool calls. +Hooks are shell actions that run before or after specific events. Use them to send notifications, inject context, modify inputs, or block tool calls. -## Hook Types +This page first explains the mechanics shared by **all** hooks, then provides a per-hook reference so you can quickly find which hook to use and what it can do. -| Type | When | Can Modify | -|------|------|------------| -| `sessionStart` | Server initialized | - | -| `sessionEnd` | Server shutting down | - | -| `chatStart` | New chat or resumed chat | Can inject `additionalContext` | -| `chatEnd` | Chat deleted | - | -| `preRequest` | Before prompt sent to LLM | Can rewrite prompt, inject context, stop request | -| `postRequest` | After primary agent prompt finished | - | -| `subagentPostRequest` | After a subagent prompt finished | - | -| `preToolCall` | Before tool execution | Can modify args, override approval, reject | -| `postToolCall` | After tool execution | Can inject context for next LLM turn | +## Hook index -## Hook Options +| Hook | Fires when | Main capability | +|------|-----------|-----------------| +| [`sessionStart`](#sessionstart) | Server initialized | Side effects | +| [`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) | +| [`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 | +| [`subagentPostRequest`](#subagentpostrequest) | After a subagent prompt finished | Trigger a follow-up turn | +| [`preToolCall`](#pretoolcall) | Before tool execution | Modify args, override approval, reject | +| [`postToolCall`](#posttoolcall) | After tool execution | Inject context, replace output | +| [`preCompact`](#precompact) | Before compaction | Block compaction | +| [`postCompact`](#postcompact) | After compaction | Inject context after the compact marker | -- **`matcher`**: For `*ToolCall` hooks. String = legacy regex for `server__tool-name`. Object = tool selector map with optional `argsMatchers`. Selectors follow tool approval: full tool name (`eca__write_file`), native ECA tool name (`write_file`), or server name. In `argsMatchers`, keys are tool argument names and values are arrays of regex alternatives for that argument. All listed arguments must match; multiple regexes for one argument are alternatives. -- **`visible`**: Show hook execution in chat (default: `true`). -- **`runOnError`**: For `postToolCall`, run even if tool errored (default: `false`). +## Global mechanics -## Execution Details +### Communication -- **Order**: Alphabetical by key. Prompt rewrites chain; argument updates merge (last wins). -- **Conflict**: Any rejection (`deny` or exit `2`) blocks the call immediately. -- **Timeout**: Actions time out after 30s unless `"timeout": ms` is set. +A hook is a shell command. ECA talks to it over standard streams: -## Input / Output +- **stdin** — event data as JSON (top-level keys are `snake_case`; nested data preserves case). +- **stdout** — on exit `0`, parsed as JSON; output fields are applied. +- **stderr** — logs and, on exit `2`, the intervention payload. -Hooks receive JSON via stdin with event data (top-level keys `snake_case`, nested data preserves case). Common fields: +### Exit codes -- All hooks: `hook_name`, `hook_type`, `workspaces`, `db_cache_path` -- Chat hooks add: `chat_id`, `agent`, `behavior` (deprecated alias) -- `preRequest` adds: `prompt` (current user text), `all_messages` (full conversation history including the current turn) -- Tool hooks add: `tool_name`, `server`, `tool_input`, `approval` (pre) or `tool_response`, `error` (post) -- `chatStart` adds: `resumed` (boolean) -- `subagentPostRequest` adds: `parent_chat_id` +The exit code selects which protocol ECA applies: -Hooks can output JSON to control execution: +| Exit | Meaning | +|------|---------| +| `0` | **Success.** stdout is parsed as JSON and output fields are applied. Preferred for all logic. | +| `2` | **Block / intervene.** A hard, hook-specific intervention. stdout JSON is ignored; **stderr** is the payload (meaning depends on the hook). | +| other | **Non-blocking error.** JSON is ignored, stderr is logged, and execution continues. | -```javascript +JSON output fields are processed **only** on exit `0`; on any non-zero exit they are ignored. + +**Plain-text stdout** (non-JSON) on exit `0` is display/debug output only. It is shown in the chat when the hook is visible but is **never** sent to the LLM. (This differs from Claude Code, which treats plain stdout as `additionalContext`.) To pass context to the model, emit JSON: + +```json +{"additionalContext": "context for the model"} +``` + +## Configuration + +Hooks are defined under the `hooks` key. Each hook has a `type` (the event), one or more `actions`, and optional fields: + +```javascript title="~/.config/eca/config.json" { - "additionalContext": "Extra context for LLM", // injected as XML block - "replacedPrompt": "New prompt text", // preRequest only - "updatedInput": {"key": "value"}, // preToolCall: merge into tool args - "approval": "allow" | "ask" | "deny", // preToolCall: override approval - "continue": false, // stop processing (with optional stopReason) - "stopReason": "Why stopped", - "suppressOutput": true // hide hook output from chat + "hooks": { + "my-hook": { + "type": "preToolCall", + "matcher": "eca__shell_command", + "visible": false, + "description": "Block dangerous shell commands", + "actions": [{"type": "shell", "file": "hooks/check.sh"}] + } + } } ``` -Plain text output (non-JSON) is treated as `additionalContext`. +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `type` | string | yes | The event that triggers the hook (see [Hook index](#hook-index)). | +| `actions` | array | yes | Actions to run. Each is `{"type": "shell", "shell": ""}` **or** `{"type": "shell", "file": ""}` (exactly one of `shell`/`file`), plus optional `timeout`. | +| `matcher` | string \| object | no | Filters when `*ToolCall`/`*Compact` hooks run (see below). | +| `visible` | boolean | no (default `true`) | Show the hook execution block in chat. | +| `description` | string | no | Shown by `/hooks`. | +| `runOnError` | boolean | no (default `false`) | `postToolCall` only — run even if the tool errored. | +| `timeout` | number | no (default `30000`) | Per-action timeout in milliseconds. | + +### `matcher` + +`matcher` filters when a hook runs. Hooks other than `*ToolCall`/`*Compact` ignore it. + +**Tool hooks (`preToolCall` / `postToolCall`)** — a string or an object: + +- **String** — legacy regex matched against `server__tool-name`. +- **Object** — a tool selector map, optionally with `argsMatchers`. + +Selector keys follow tool-approval naming: full tool name (`eca__write_file`), native ECA tool name (`write_file`), or a server name (matches every tool from that server). + +`argsMatchers` narrows by argument value: keys are argument names, values are arrays of regex alternatives (any one may match), and **all** listed arguments must match. + +```javascript +"matcher": { + "eca__write_file": { + "argsMatchers": { + "path": [".*\\.allium$", ".*\\.clj$"] + } + } +} +``` + +**Compact hooks (`preCompact` / `postCompact`)**: + +- **String** — exact match against `triggered` (`manual` or `auto`). +- **Omitted** — runs for both triggers. + +### Execution order + +Hooks for an event run **alphabetically by key**, and actions within a hook run in declaration order. `preRequest` prompt rewrites chain; `preToolCall` argument updates merge (later values for the same key win). A successful `continue: false` halts the rest of the chain (see [below](#stopping-continue-false-vs-exit-2)); exit `2` does not. + +## Common rules + +The following apply to many hooks. Each hook's reference entry lists exactly which it honors. + +### Common output fields + +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). | +| `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.** | + +Hook-specific fields (`additionalContext`, `replacedPrompt`, `followUp`, `updatedInput`, `approval`, `replacedOutput`) are documented per hook. + +### Stopping: `continue: false` vs `exit 2` + +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. + +When a hook stops the turn it ends immediately: remaining `postRequest`/`subagentPostRequest` hooks do not run and no `followUp`/continuation fires. + +### How a hook appears in the chat + +A hook reaches the user through two **independent** channels: + +- **The execution block** — the panel shown in the chat for the hook run. It carries the run's mechanics: status, stdout/stderr, and structural effect lines (`UpdatedInput`, `FollowUp`, `ReplacedOutput`, `AdditionalContext`, `ReplacedPrompt`). +- **`systemMessage`** — a standalone message, never part of the block. + +Two controls gate the block (`systemMessage` is unaffected by both): + +| Control | Effect | +|---------|--------| +| `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`. + +### Base input + +Every hook receives these fields on stdin: + +- `hook_name`, `hook_type`, `workspaces`, `cwd`, `db_cache_path`, `session_id`, `eca_executable` + - `cwd` — the first workspace folder, matching the working directory ECA uses for hook commands. + - `session_id` — the cache session key derived from the `db_cache_path` parent directory. + - `eca_executable` — the launch command of the running ECA process (executable path for native binaries; the full `java ... -jar` invocation for JVM launches). + +**Chat-scoped hooks** (everything except `sessionStart`/`sessionEnd`) additionally receive `chat_id`, `agent`, `behavior` (deprecated alias of `agent`), `full_model`, and `variant`. + +Each hook below lists any further fields it adds. + +## Session hooks + +### `sessionStart` + +Fires when the server is initialized. Use for global setup or telemetry. + +- **Input** — base fields only (no chat fields, no UI). +- **Honored output** — none. `continue: false` only stops remaining `sessionStart` hooks. +- **Exit 2** — non-blocking error. + +### `sessionEnd` + +Fires when the server is shutting down. Use for cleanup or final telemetry. + +- **Input** — base fields only (no chat fields, no UI). +- **Honored output** — none. `continue: false` only stops remaining `sessionEnd` hooks. +- **Exit 2** — non-blocking error. + +## Chat hooks + +### `chatStart` + +Fires when a chat is created or resumed. Use to inject session context (date, branch, conventions). + +- **Input adds** — `resumed` (boolean). +- **Honored output** — `additionalContext` (injected into the system prompt); `systemMessage`; `suppressOutput`; `continue: false` + `stopReason` stops the turn. +- **Exit 2** — non-blocking error. + +### `chatEnd` + +Fires when a chat is deleted. **Advisory and best-effort** — side effects only (cleanup, logging, external notifications). + +- **Input adds** — chat fields 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. + +### `subagentStart` + +Fires before a subagent's first prompt. Use to inject context scoped to the subagent. + +- **Input adds** — `parent_chat_id`. +- **Honored output** — `additionalContext`; `systemMessage`; `suppressOutput`; `continue: false` + `stopReason` stops the turn. +- **Exit 2** — non-blocking error. + +## Request hooks + +### `preRequest` + +Fires before a prompt is sent to the LLM. Use for prompt validation, rewriting, or injecting dynamic context. + +- **Input adds** — `prompt` (the current chainable prompt; earlier hooks may have rewritten it). +- **Honored output**: + - `replacedPrompt` — replace the prompt text (cannot modify commands or MCP prompts). + - `additionalContext` — appended to the prompt for this turn. + - `systemMessage`, `suppressOutput`. + - `continue: false` + `stopReason` stops the turn. +- **Exit 2** — blocks the prompt (not added to history) and stops the LLM. stderr is **not** sent to the LLM nor shown as a normal chat message; it appears only as the hook's error output when the hook is visible. An invisible hook instead surfaces `Request blocked by hook '': `. + +### `postRequest` + +Fires after a primary-agent prompt finishes. Also runs for subagents. Primary use: validate the response or trigger a follow-up turn. + +!!! note + `postRequest` fires only after LLM responses. Display-only commands (`/hooks`, `/model`, `/costs`) and compaction prompts (`/compact` and auto-compact) do **not** trigger it — use [`postCompact`](#postcompact) for compaction. -To reject a tool call, either output `{"approval": "deny"}` or exit with code `2`. +- **Input adds** — `response` (last assistant text) and `follow_up_active` (`true` when this turn was triggered by a previous `followUp`). +- **Honored output**: + - `followUp` — start a new LLM turn after this one (see [follow-up turns](#follow-up-turns)). + - `systemMessage`, `suppressOutput`. + - `continue: false` + `stopReason` cancels all `followUp`, skips auto-continuation, and stops the turn. +- **Exit 2** — stderr becomes the `followUp` text (the request already completed, so it cannot be blocked): + + ```bash + # Equivalent to: echo '{"followUp": "Run the tests"}' + echo "Run the tests" >&2; exit 2 + ``` + +### `subagentPostRequest` + +Fires after a subagent prompt finishes, **in addition** to `postRequest` (which also runs for subagents). Use for subagent-specific follow-ups or notifications. + +- **Input adds** — `response`, `follow_up_active`, and `parent_chat_id`. +- **Honored output** — same as [`postRequest`](#postrequest): `followUp`, `systemMessage`, `suppressOutput`, `continue: false` + `stopReason`. +- **Exit 2** — stderr becomes the `followUp` text. + +#### Follow-up turns + +`followUp` becomes a plain user message in the continuation turn (no XML wrapping); multiple hooks' values are joined with double newlines. + +- `continue: false` takes priority: if any post-request hook returns it, all `followUp` values are ignored and the turn stops. +- `followUp` also takes priority over automatic continuations (truncated-response auto-continue, post-compact auto-resume). +- To prevent loops, a turn triggered by `followUp` sets `follow_up_active: true` in the input. Hooks should check it and skip returning `followUp` when it is already active. + +## Tool hooks + +Tool hooks honor [`matcher`](#matcher) to target specific tools. + +### `preToolCall` + +Fires before a tool is invoked. Use for argument validation, security checks, or parameter rewriting. + +- **Input adds** — `tool_name`, `server`, `tool_input`, `tool_call_id`, `approval`. +- **Honored output**: + - `updatedInput` — merged into the tool arguments (across hooks, later keys win). + - `approval` — `"allow"` / `"ask"` / `"deny"` override. Approvals merge by precedence `deny > ask > allow`; a hook `allow` never overrides a config `deny`/`ask`. + - `additionalContext` — with `approval: "deny"`, gives the LLM the rejection context so it can adapt. + - `systemMessage`, `suppressOutput`. + - `continue: false` + `stopReason` stops the turn (the LLM sees a neutral placeholder in the tool result; `stopReason` reaches only the user). +- **Exit 2** — rejects the tool call; the turn continues. stderr becomes the rejection reason sent to the LLM and shown in the hook's output. + +**Choosing a denial method:** + +| Method | Effect | LLM gets info? | When to use | +|--------|--------|----------------|-------------| +| exit 0 + `approval: "deny"` + `additionalContext` | Structured denial; turn continues so the LLM can adapt | Yes — from `additionalContext` | Policy, validation | +| exit 2 + stderr | Hard rejection; turn continues | Yes — from stderr | Simple shell policy blocks | +| exit 0 + `continue: false` + `stopReason` | Stop everything; the user is told why | No — `stopReason` is user-only | Halt the whole turn, not just one tool | + +`preToolCall` also fires for the native `ask_user` tool, which always blocks waiting for a user answer (regardless of trust mode), so it receives `approval: "ask"`. + +### `postToolCall` + +Fires after a tool executes. Use for result auditing, context injection, or hiding sensitive output. By default it runs only on success; set `runOnError: true` to also run when the tool errored. + +- **Input adds** — `tool_name`, `server`, `tool_input`, `tool_call_id`, `tool_response`, `error`. +- **Honored output**: + - `replacedOutput` — replace the tool result (`""` replaces it with empty content). + - `additionalContext` — appended (wrapped as XML) to the tool result for the LLM. + - `systemMessage`, `suppressOutput`. + - `continue: false` + `stopReason` stops the turn before the LLM continues. +- **Exit 2** — replaces the result with stderr (empty stderr falls back to `[Tool output hidden by hook]`); the turn continues. + +## Compaction hooks + +Compaction hooks honor [`matcher`](#matcher) to target `manual` or `auto` triggers. + +### `preCompact` + +Fires before manual or automatic compaction. Use for logging or to veto compaction. + +- **Input adds** — `triggered` (`manual`/`auto`) and `custom_instructions` (manual `/compact` arguments, empty for auto). +- **Honored output** — `systemMessage`, `suppressOutput`; `continue: false` + `stopReason` blocks compaction **and** stops the turn. +- **Exit 2** — blocks compaction only; the LLM continues with full context. stderr appears only as the hook's error output — not the LLM, not a normal chat message; the user sees `Compaction blocked by hook ''.` + +### `postCompact` + +Fires after manual or automatic compaction. It cannot roll back or change the compaction result. + +- **Input adds** — `triggered` (`manual`/`auto`) and `compact_summary` (the LLM-produced summary). +- **Honored output**: + - `additionalContext` — appended to the post-compact summary so it stays visible after the compact marker. + - `systemMessage`, `suppressOutput`. + - `continue: false` + `stopReason` leaves the marker and summary in history, prevents the automatic resume after compaction, and stops the turn. +- **Exit 2** — non-blocking error. + +## Accessing chat history from hooks + +Chat-scoped hooks receive `db_cache_path`, `chat_id`, and `eca_executable`, so a shell hook can call the ECA CLI's `read-chat` command to inspect the current chat's history while the session is running: + +```bash title="~/.config/eca/hooks/inspect-history.sh" +#!/usr/bin/env bash +input=$(cat) +chat_id=$(echo "$input" | jq -r '.chat_id') +db_cache_path=$(echo "$input" | jq -r '.db_cache_path') +eca_executable=$(echo "$input" | jq -r '.eca_executable') + +# Stream the chat's messages as JSONL +$eca_executable read-chat --db-cache-path "$db_cache_path" --chat-id "$chat_id" +``` + +Keep the query focused and fast — the hook runs synchronously with a 30s timeout. `read-chat` needs no running server and reads directly from `db.transit.json`. See [`read-chat`](../read-chat.md) for full options and scripting examples. + +## `/hooks` command + +Lists all active hooks grouped by type, showing name, description, and matcher. Plugin hooks are prefixed with the plugin name (e.g. `my-plugin::hook-name`). + +## Plugin hooks + +Hooks defined in a plugin's `hooks/hooks.json` are automatically prefixed with the plugin name using `::` (e.g. a hook `check-commands` in plugin `security` becomes `security::check-commands`). This avoids collisions with user-defined hooks and makes the source identifiable in `/hooks` output. ## Examples @@ -174,6 +463,23 @@ To reject a tool call, either output `{"approval": "deny"}` or exit with code `2 } ``` +=== "Sanitize tool output" + + ```javascript title="~/.config/eca/config.json" + { + "hooks": { + "sanitize-output": { + "type": "postToolCall", + "matcher": "eca__shell_command", + "actions": [{ + "type": "shell", + "shell": "jq -c '{replacedOutput: .tool_response | gsub(\"secret\\\\d+\"; \"[REDACTED]\")}'" + }] + } + } + } + ``` + === "Match tool arguments" ```javascript title="~/.config/eca/config.json" @@ -211,6 +517,34 @@ To reject a tool call, either output `{"approval": "deny"}` or exit with code `2 } ``` +=== "Trigger follow-up after prompt" + + ```javascript title="~/.config/eca/config.json" + { + "hooks": { + "run-tests": { + "type": "postRequest", + "actions": [{ + "type": "shell", + "shell": "echo '{\"followUp\": \"Run the test suite and report any failures\"}'" + }] + } + } + } + ``` + + The hook runs after each prompt finishes. If `follow_up_active` is `true`, the hook should skip to avoid infinite loops: + + ```bash title="~/.config/eca/hooks/run-tests.sh" + #!/usr/bin/env bash + input=$(cat) + follow_up_active=$(echo "$input" | jq -r '.follow_up_active // false') + if [ "$follow_up_active" = "true" ]; then + exit 0 # Skip to avoid infinite loop + fi + echo '{"followUp": "Run the test suite and report any failures"}' + ``` + === "Use external script file" ```javascript title="~/.config/eca/config.json" @@ -223,4 +557,3 @@ To reject a tool call, either output `{"approval": "deny"}` or exit with code `2 } } ``` - diff --git a/docs/features.md b/docs/features.md index 573c4593a..cfc5b93c8 100644 --- a/docs/features.md +++ b/docs/features.md @@ -128,6 +128,7 @@ The built-in commands are: - `/login`: Log into a provider. Ex: `github-copilot`, `anthropic`. - `/model`: Select model for current chat directly from chat. Ex: `anthropic/claude-sonnet-4-6`. - `/skills`: List known skills that ECA can load. +- `/hooks`: List active hooks grouped by type, showing name, description, and matcher. - `/compact`: Compact/summarize conversation helping reduce context window. - `/resume`: Resume a chat from previous session of this workspace folder. - `/costs`: Show costs about current session. diff --git a/docs/protocol.md b/docs/protocol.md index fd5775780..e4e50c828 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -788,6 +788,12 @@ interface ChatHookActionFinishedContent { * The error of this hook if any */ error?: string; + + /** + * For tool hooks (preToolCall/postToolCall), the id of the tool call this + * hook acted on, so clients can correlate the hook block with its tool call. + */ + toolCallId?: string; } /** diff --git a/integration-test/integration/chat/commands_test.clj b/integration-test/integration/chat/commands_test.clj index c10219f18..a833df5d7 100644 --- a/integration-test/integration/chat/commands_test.clj +++ b/integration-test/integration/chat/commands_test.clj @@ -46,6 +46,7 @@ :arguments [{:name "plugin" :description "Plugin name or plugin@marketplace" :required true}]} {:name "plugin-uninstall" :arguments [{:name "plugin" :description "Plugin name" :required true}]} + {:name "hooks" :arguments []} {:name "eca-info" :arguments nil}]} resp)))) diff --git a/integration-test/integration/chat/hooks_test.clj b/integration-test/integration/chat/hooks_test.clj index eaaf8fb5b..4a29d594c 100644 --- a/integration-test/integration/chat/hooks_test.clj +++ b/integration-test/integration/chat/hooks_test.clj @@ -58,7 +58,7 @@ (m/embeds [{:chatId chat-id :role "system" - :content {:type "text" :text "STOPPED BY HOOK"}} + :content {:type "text" :text "Turn stopped by hook 'stop': STOPPED BY HOOK"}} {:chatId chat-id :role "system" :content {:type "progress" :state "finished"}}]) @@ -223,9 +223,8 @@ (let [hook-data (json/parse-string (slurp log-path) true)] (is (match? {:tool_input {:path (m/pred string?)} - :tool_response [{:type "text" - :text (m/pred #(and (string? %) - (not (string/blank? %))))}] + :tool_response (m/pred #(and (string? %) + (string/includes? % "file1.md"))) :chat_id (m/pred string?) :server (m/equals "eca") :db_cache_path (m/pred string?) @@ -236,9 +235,7 @@ :error (m/equals false) :workspaces (m/seq-of (m/pred string?)) :tool_name (m/equals "directory_tree")} - hook-data)) - ;; Explicitly check that we got some file listing content - (is (string/includes? (get-in hook-data [:tool_response 0 :text]) "file1.md"))))))) + hook-data))))))) (deftest pretoolcall-approval-deny-test (testing "preToolCall hook can reject tool calls via approval:deny" @@ -292,7 +289,7 @@ "Tool call should have been rejected")))) (deftest pretoolcall-exit-code-rejection-with-stop-test - (testing "preToolCall hook exit code 2 rejects tool and continue:false stops chat" + (testing "preToolCall hook exit 0 with approval:deny + continue:false rejects tool and stops chat" (let [win? (string/starts-with? (System/getProperty "os.name") "Windows")] (eca/start-process!) @@ -304,10 +301,13 @@ (hooks-init-options {"reject-and-stop" {:type "preToolCall" :actions [{:type "shell" - ;; Exit code 2 means rejection, with continue:false and stopReason + ;; Exit 0: JSON effects apply. approval:deny rejects the + ;; tool, additionalContext is the LLM-visible reason, + ;; continue:false stops the turn and stopReason is shown + ;; to the user. :shell (if win? - "Write-Output '{\"continue\":false,\"stopReason\":\"Security policy violation\"}'; exit 2" - "echo '{\"continue\":false,\"stopReason\":\"Security policy violation\"}' && exit 2")}]}})}))) + "Write-Output '{\"approval\":\"deny\",\"additionalContext\":\"Tool blocked by policy\",\"continue\":false,\"stopReason\":\"Security policy violation\"}'" + "echo '{\"approval\":\"deny\",\"additionalContext\":\"Tool blocked by policy\",\"continue\":false,\"stopReason\":\"Security policy violation\"}'")}]}})}))) (eca/notify! (fixture/initialized-notification)) @@ -338,10 +338,10 @@ notifications) "Tool call should have been rejected") - ;; Verify stopReason was displayed + ;; Verify stopReason was displayed (may be prefixed with the hook name) (is (some #(and (= "system" (:role %)) (= "text" (get-in % [:content :type])) - (= "Security policy violation" (get-in % [:content :text]))) + (string/includes? (str (get-in % [:content :text])) "Security policy violation")) notifications) "Stop reason should have been displayed")))) diff --git a/src/eca/db.clj b/src/eca/db.clj index d129fc348..8cd8be1ef 100644 --- a/src/eca/db.clj +++ b/src/eca/db.clj @@ -119,6 +119,9 @@ :refresh-token :string :expires-at :long}}}) +(defn parent-chat-id [db chat-id] + (get-in db [:chats chat-id :parent-chat-id])) + (defonce initial-db {:client-info {} :workspace-folders [] diff --git a/src/eca/features/chat.clj b/src/eca/features/chat.clj index 048deecfd..5f8cebab5 100644 --- a/src/eca/features/chat.clj +++ b/src/eca/features/chat.clj @@ -279,14 +279,33 @@ (defn default-model [db config] (llm-api/default-model db config)) +(defn ^:private resolve-full-model + "Resolve the full model id for a prompt response and LLM request." + [requested-model db chat-id agent-config config] + (or requested-model + (let [stored-model (get-in db [:chats chat-id :model]) + agent-default-model (:defaultModel agent-config)] + (cond + (and stored-model + (contains? (:models db) stored-model)) + stored-model + + (and agent-default-model + (contains? (:models db) agent-default-model)) + agent-default-model + + :else + (default-model db config))))) + (defn ^:private update-pre-request-state "Pure function to compute new state from hook result." - [{:keys [final-prompt additional-contexts stop?]} {:keys [parsed raw-output exit]} action-name] - (let [replaced-prompt (get parsed "replacedPrompt") - additional-context (if parsed - (get parsed "additionalContext") - raw-output) - success? (= 0 exit)] + [{:keys [final-prompt additional-contexts stop-turn? blocked? blocked-reasons stop-reason stop-hook-name]} + {:keys [parsed exit]} + action-name] + (let [replaced-prompt (shared/not-blank (get parsed "replacedPrompt")) + additional-context (shared/not-blank (get parsed "additionalContext")) + success? (= 0 exit) + stop-turn-result? (and success? (false? (get parsed "continue" true)))] {:final-prompt (if (and replaced-prompt success?) replaced-prompt final-prompt) @@ -294,19 +313,48 @@ (conj additional-contexts {:hook-name action-name :content additional-context}) additional-contexts) - :stop? (or stop? - (false? (get parsed "continue" true)))})) + :stop-turn? (or stop-turn? stop-turn-result?) + :blocked? blocked? + :blocked-reasons blocked-reasons + :stop-reason (if stop-turn-result? + (or stop-reason (shared/not-blank (get parsed "stopReason"))) + stop-reason) + :stop-hook-name (if stop-turn-result? + (or stop-hook-name action-name) + stop-hook-name)})) + +(defn ^:private pre-request-block-reason + "Stderr text of a preRequest exit-2 block, or nil when empty. The user-facing + wrapper (lifecycle/request-blocked-by-hook-message) supplies the fallback." + [{:keys [raw-error]}] + (some-> raw-error str string/trim not-empty)) + +(defn ^:private finish-blocked-or-stopped-pre-request! + [chat-ctx {:keys [blocked? stop-turn? stop-reason stop-hook-name blocked-reasons blocked-hook-name]}] + (when (or stop-turn? blocked?) + (cond + stop-turn? (lifecycle/send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason) + blocked? (if (seq blocked-reasons) + (doseq [reason blocked-reasons] + (lifecycle/send-content! chat-ctx :system {:type :text :text reason})) + ;; Visible hooks already showed their stderr in their block; just name the hook. + (lifecycle/send-content! chat-ctx :system + {:type :text + :text (lifecycle/request-blocked-by-hook-message blocked-hook-name nil)}))) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx))) (defn ^:private run-pre-request-action! "Run a single preRequest hook action, updating the accumulator state. State is a map: - :final-prompt - - :all-messages - :additional-contexts - - :stop? (true when any hook requests stop)" - [db chat-ctx chat-id hook hook-name idx action state] - (if (:stop? state) + - :stop-turn? (true when any hook requests current-turn stop via continue:false) + - :stop-reason + - :blocked? (true when any hook blocks the request via exit 2) + - :blocked-reasons" + [db chat-ctx hook hook-name idx action state] + (if (:stop-turn? state) state (let [id (str (random-uuid)) action-type (:type action) @@ -316,17 +364,17 @@ visible? (get hook :visible true)] (lifecycle/notify-before-hook-action! chat-ctx {:id id :visible? visible? - :name action-name}) + :name action-name + :type action-type}) ;; Run the hook action (if-let [result (f.hooks/run-hook-action! action action-name :preRequest - (merge (f.hooks/chat-hook-data db chat-id (:agent chat-ctx)) - {:prompt (:final-prompt state) - :all-messages (:all-messages state)}) + (merge (f.hooks/chat-hook-data db chat-ctx) + {:prompt (:final-prompt state)}) db)] - (let [{:keys [parsed raw-output raw-error exit]} result - should-continue? (get parsed "continue" true)] + (let [{:keys [raw-error exit]} result + exit-2? (= f.hooks/hook-rejection-exit-code exit)] ;; Notify after action (lifecycle/notify-after-hook-action! chat-ctx (merge result {:id id @@ -334,35 +382,45 @@ :type action-type :visible? visible? :status exit - :output raw-output + :hook-type :preRequest :error raw-error})) - ;; Check if hook wants to stop - (when (false? should-continue?) - (when-let [stop-reason (get parsed "stopReason")] - (lifecycle/send-content! chat-ctx :system {:type :text :text stop-reason})) - (lifecycle/finish-chat-prompt! :idle chat-ctx)) - ;; Update accumulator - (update-pre-request-state state - result - action-name)) + ;; Exit 2 blocks the prompt but does not stop remaining preRequest + ;; hooks; only successful continue:false short-circuits processing. + ;; The actual prompt finish happens once after reduction completes. + ;; Only collect stderr from INVISIBLE hooks; visible hooks already + ;; surface stderr through hookActionFinished :error, so duplicating + ;; it as a separate chat message would be noisy. + (if exit-2? + (cond-> (assoc state :blocked? true) + ;; Remember the first blocking hook so the user-facing message can + ;; name it even for visible hooks (consistent with turn-stopped). + (not (:blocked-hook-name state)) (assoc :blocked-hook-name action-name) + (not visible?) (update :blocked-reasons conj + (lifecycle/request-blocked-by-hook-message + action-name + (pre-request-block-reason result)))) + (update-pre-request-state state + result + action-name))) ;; No result from action (do (lifecycle/notify-after-hook-action! chat-ctx {:id id :name action-name :visible? visible? :type action-type + :hook-type :preRequest :exit 1 :status 1}) state))))) (defn ^:private run-pre-request-hook! "Run all actions for a single preRequest hook, threading state." - [db chat-ctx chat-id state [hook-name hook]] + [db chat-ctx state [hook-name hook]] (reduce (fn [s [idx action]] - (if (:stop? s) + (if (:stop-turn? s) (reduced s) - (run-pre-request-action! db chat-ctx chat-id hook (name hook-name) idx action s))) + (run-pre-request-action! db chat-ctx hook (name hook-name) idx action s))) state (map-indexed vector (:actions hook)))) @@ -372,19 +430,29 @@ Returns a map with: - :final-prompt - :additional-contexts (vector of {:hook-name name :content context}) - - :stop? (true when any hook requests stop)" - [{:keys [db* config chat-id message all-messages] :as chat-ctx}] + - :stop-turn? (true when any hook requests current-turn stop via continue:false) + - :stop-reason + - :blocked? (true when any hook blocks the request via exit 2) + - :blocked-reasons" + [{:keys [db* config message] :as chat-ctx}] (let [db @db*] (reduce (fn [state hook-entry] - (if (:stop? state) + (if (:stop-turn? state) (reduced state) - (run-pre-request-hook! db chat-ctx chat-id state hook-entry))) + (run-pre-request-hook! db chat-ctx state hook-entry))) {:final-prompt message - :all-messages all-messages :additional-contexts [] - :stop? false} + :stop-turn? false + :blocked? false + :blocked-reasons [] + :blocked-hook-name nil + :stop-reason nil + :stop-hook-name nil} (->> (:hooks config) + ;; This function owns preRequest chaining, so it must select legacy + ;; prePrompt hooks itself; hook-matches? only normalizes legacy types + ;; for the generic trigger-if-matches! path. (filter #({"preRequest" "prePrompt"} (:type (val %)))) (sort-by key))))) @@ -433,33 +501,63 @@ (when-not (string/blank? text) (odd? (count (re-seq #"(?m)^```" text))))) +(defn ^:private auto-compact-finished-side-effect! + "on-finished-side-effect for auto-compaction: clear the auto-compacting flag, + apply the compact side effects and run postCompact hooks. When a postCompact + hook stops the turn (continue:false), surface the reason and finish here + (postRequest hooks were already skipped while auto-compacting), returning + {:stop-after-finish? true} so the resume continuation does not fire." + [{:keys [db* chat-id] :as chat-ctx}] + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?) + (let [{:keys [stop-turn? stop-reason stop-hook-name]} (lifecycle/complete-compact! chat-ctx "auto")] + (when stop-turn? + (lifecycle/send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx)) + {:stop-after-finish? stop-turn?})) + +(defn ^:private resume-after-auto-compact! + "on-after-finish! for auto-compaction: resume the original user task." + [chat-ctx user-messages] + (prompt-messages! + (concat [{:role "user" + :content [{:type :text + :text "Continue with the task. The previous user request was:"}]}] + user-messages) + :auto-compact + (assoc chat-ctx :auto-compacted? true))) + (defn ^:private trigger-auto-compact! "Trigger auto-compact: send compact prompt, then resume the original task." [{:keys [db* config chat-id agent] :as chat-ctx} all-tools user-messages] - (let [db @db* - compact-prompt (f.prompt/compact-prompt nil all-tools agent config db)] - (logger/info logger-tag "Auto-compacting chat" {:chat-id chat-id}) - (swap! db* assoc-in [:chats chat-id :auto-compacting?] true) - (prompt-messages! - [{:role "user" :content "Compact the chat following the template:"} - {:role "user" :content compact-prompt}] - :auto-compact - (assoc chat-ctx - :on-finished-side-effect - (fn [] - (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?) - (shared/compact-side-effect! chat-ctx true) - ;; Resume the original task - (prompt-messages! - (concat [{:role "user" - :content [{:type :text - :text "Continue with the task. The previous user request was:"}]}] - user-messages) - :auto-compact - (assoc chat-ctx :auto-compacted? true))))) - nil)) + (let [{:keys [blocked? reason hook-name stop-turn?]} (lifecycle/run-pre-compact-hooks! chat-ctx "auto" "")] + (if blocked? + (do + (logger/info logger-tag "Auto-compaction blocked by hook" {:chat-id chat-id + :stop-turn? stop-turn?}) + ;; continue:false stops the turn (reason from stopReason, hook-name for + ;; provenance); exit 2 blocks compaction only, with no user-facing reason. + (if stop-turn? + (do (lifecycle/send-turn-stopped-by-hook! chat-ctx hook-name reason) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx)) + (do (lifecycle/send-content! chat-ctx :system {:type :text :text (lifecycle/compaction-blocked-by-hook-message hook-name)}) + (prompt-messages! user-messages + :auto-compact-blocked + (assoc chat-ctx :auto-compacted? true)))) + nil) + (let [db @db* + compact-prompt (f.prompt/compact-prompt nil all-tools agent config db)] + (logger/info logger-tag "Auto-compacting chat" {:chat-id chat-id}) + (swap! db* assoc-in [:chats chat-id :auto-compacting?] true) + (prompt-messages! + [{:role "user" :content "Compact the chat following the template:"} + {:role "user" :content compact-prompt}] + :auto-compact + (assoc chat-ctx + :on-finished-side-effect #(auto-compact-finished-side-effect! chat-ctx) + :on-after-finish! #(resume-after-auto-compact! chat-ctx user-messages))) + nil)))) (defn ^:private log-api-switch! "Log when the user swaps to a model whose api differs from the chat's @@ -605,13 +703,10 @@ modify-allowed? (= source-type :prompt-message) run-hooks? (#{:prompt-message :eca-command :mcp-prompt} source-type) user-messages (if run-hooks? - (let [past-messages (shared/messages-after-last-compact-marker - (get-in @db* [:chats chat-id :messages] [])) - {:keys [final-prompt additional-contexts stop?]} - (run-pre-request-hooks! (assoc chat-ctx :message original-text - :all-messages (into past-messages user-messages)))] + (let [{:keys [final-prompt additional-contexts stop-turn? blocked?] :as pre-request-state} + (run-pre-request-hooks! (assoc chat-ctx :message original-text))] (cond - stop? (do (lifecycle/finish-chat-prompt! :idle chat-ctx) nil) + (or stop-turn? blocked?) (do (finish-blocked-or-stopped-pre-request! chat-ctx pre-request-state) nil) :else (let [last-user-idx (llm-util/find-last-user-msg-idx user-messages) ;; preRequest additionalContext should ideally attach to the last user message, ;; but some prompt sources may not contain a user role (e.g. prompt templates). @@ -622,13 +717,13 @@ user-messages) with-contexts (cond (and (seq additional-contexts) context-idx) - (reduce (fn [msgs {:keys [hook-name content]}] + (reduce (fn [msgs {:keys [content]}] (update-in msgs [context-idx :content] #(conj (if (string? %) [{:type :text :text %}] (vec %)) {:type :text - :text (lifecycle/wrap-additional-context hook-name content)}))) + :text (lifecycle/wrap-additional-context content)}))) rewritten additional-contexts) @@ -841,7 +936,7 @@ :text (str "API limit reached. Tokens: " (json/generate-string (:tokens msg)))}) (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?) - (lifecycle/finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect))) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx)) :finish (let [response-text @received-msgs* stopping? (identical? :stopping (get-in @db* [:chats chat-id :status]))] (when-not (string/blank? response-text) @@ -852,7 +947,8 @@ (or (:premature? msg) (truncated-response? response-text)) (not (:auto-continued? chat-ctx)) - (not (:on-finished-side-effect chat-ctx))) + (not (or (:on-finished-side-effect chat-ctx) + (:on-after-finish! chat-ctx)))) (do (logger/info logger-tag "Truncated or premature response detected, auto-continuing" {:chat-id chat-id @@ -863,16 +959,21 @@ (swap! db* assoc-in [:chats chat-id :auto-compacting?] true) (lifecycle/finish-chat-prompt! :idle - (assoc chat-ctx :on-finished-side-effect + (assoc chat-ctx + :on-finished-side-effect + (fn [] + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?)) + :on-after-finish! (fn [] - (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?) (prompt-messages! [{:role "user" :content [{:type :text :text "Your previous response was interrupted mid-stream. Continue from where you left off, do not redo completed steps."}]}] :auto-continue (assoc chat-ctx :auto-continued? true)))))) - (lifecycle/finish-chat-prompt! :idle chat-ctx))))) + (lifecycle/finish-chat-prompt! + :idle + (assoc-some chat-ctx :response (some-> response-text string/trim not-empty))))))) :on-prepare-tool-call (fn [{:keys [id full-name arguments-text]}] (lifecycle/assert-chat-not-stopped! chat-ctx) (let [tool (f.tools/resolve-tool full-name all-tools) @@ -892,8 +993,12 @@ (assoc chat-ctx :continue-fn (fn [tc-all-tools tc-user-messages] (if (get-in @db* [:chats chat-id :compact-done?]) + ;; Manual /compact maintenance prompt: skip postRequest hooks + ;; (postCompact provides compact_summary; postRequest would + ;; just see the trailing "Compacted successfully!" tool result). (do (swap! db* update-in [:chats chat-id] dissoc :compact-done?) - (lifecycle/finish-chat-prompt! :idle chat-ctx) + (lifecycle/finish-chat-prompt! :idle + (assoc chat-ctx :skip-post-request-hooks? true)) nil) (if (and (lifecycle/auto-compact? chat-id agent full-model config @db*) (not (:auto-compacted? chat-ctx))) @@ -1071,7 +1176,8 @@ (or transient-error? (string/includes? (or message "") "idle timeout")) (not (:auto-continued? chat-ctx)) - (not (:on-finished-side-effect chat-ctx)) + (not (or (:on-finished-side-effect chat-ctx) + (:on-after-finish! chat-ctx))) (not compacting?))] (when compacting? (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?)) @@ -1086,9 +1192,12 @@ {:type :progress :state :running :text (str (or message "Connection interrupted") ", continuing...")}) (swap! db* assoc-in [:chats chat-id :auto-compacting?] true) (lifecycle/finish-chat-prompt! :idle - (assoc chat-ctx :on-finished-side-effect + (assoc chat-ctx + :on-finished-side-effect + (fn [] + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?)) + :on-after-finish! (fn [] - (swap! db* update-in [:chats chat-id] dissoc :auto-compacting?) (prompt-messages! [{:role "user" :content [{:type :text @@ -1103,10 +1212,11 @@ ;; which would leave a chat that hit an error without a save. ;; Persist explicitly so users can always /resume an errored chat. (db/update-workspaces-cache! @db* metrics) - (lifecycle/finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect))))))))}) + (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks chat-ctx))))))))}) (catch Exception e (when-not (:silent? (ex-data e)) (logger/error e) + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?) (when-not (string/blank? @received-msgs*) (add-to-history! {:role "assistant" :content [{:type :text :text @received-msgs*}]})) @@ -1114,7 +1224,7 @@ ;; Belt-and-suspenders: persist before finish-chat-prompt!, ;; which may short-circuit. See note above in :on-error. (db/update-workspaces-cache! @db* metrics) - (lifecycle/finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect)))) + (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks chat-ctx)))) (finally (when (and (= prompt-id (get-in @db* [:chats chat-id :prompt-id])) (contains? #{:stopping :running} (get-in @db* [:chats chat-id :status]))) @@ -1151,7 +1261,7 @@ (defn ^:private handle-command! [{:keys [command args]} chat-ctx] (try - (let [{:keys [type on-finished-side-effect] :as result} (f.commands/handle-command! command args chat-ctx)] + (let [{:keys [type on-finished-side-effect on-after-finish!] :as result} (f.commands/handle-command! command args chat-ctx)] (case type :chat-messages (do (when (:clear-before? result) @@ -1164,147 +1274,170 @@ (lifecycle/send-content! new-chat-ctx :system (assoc-some {:type :metadata} :title title))))) - (lifecycle/finish-chat-prompt! :idle chat-ctx)) - :new-chat-status (lifecycle/finish-chat-prompt! (:status result) chat-ctx) + (lifecycle/finish-chat-prompt! :idle (assoc chat-ctx :skip-post-request-hooks? true))) + :new-chat-status (lifecycle/finish-chat-prompt! (:status result) (assoc chat-ctx :skip-post-request-hooks? true)) :send-prompt (let [prompt-contents (:prompt result)] ;; Keep original slash command in :message for hooks (already in parent chat-ctx) (prompt-messages! [{:role "user" :content prompt-contents}] :eca-command - (assoc chat-ctx :on-finished-side-effect on-finished-side-effect))) + (assoc-some chat-ctx + :on-finished-side-effect on-finished-side-effect + :on-after-finish! on-after-finish!))) nil)) (catch Exception e (logger/error e) (lifecycle/send-content! chat-ctx :system {:type :text :text (str "Error: " (ex-message e) "\n\nCheck ECA stderr for more details.")}) - (lifecycle/finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect))))) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx)))) + +(defn ^:private run-start-hooks! + "Runs chatStart (or subagentStart for subagent chats) hooks for this chat's + first prompt. Collects additionalContext from successful hooks into + :startup-context so build-chat-instructions and /prompt-show pick it up, and + returns {:stop-turn? boolean :stop-reason string-or-nil} when a hook returns + {\"continue\":false} on exit 0. db-before-hooks is the stable snapshot used + for hook reads; db* is mutated only for hook side effects." + [{:keys [db* chat-id agent messenger variant] :as _chat-ctx} db-before-hooks resumed? full-model] + (config/await-plugins-resolved!) + (let [config (config/all db-before-hooks) + hook-results* (atom []) + subagent? (some? (get-in db-before-hooks [:chats chat-id :subagent])) + hook-ctx (cond-> {:messenger messenger :chat-id chat-id} + subagent? (assoc :parent-chat-id (db/parent-chat-id db-before-hooks chat-id))) + hook-type (if subagent? :subagentStart :chatStart) + hook-data (cond-> (f.hooks/chat-hook-data db-before-hooks {:chat-id chat-id + :agent agent + :full-model full-model + :variant variant}) + subagent? (assoc :parent-chat-id (db/parent-chat-id db-before-hooks chat-id)) + (not subagent?) (assoc :resumed resumed?))] + (f.hooks/trigger-if-matches! hook-type + hook-data + {:on-before-action (partial lifecycle/notify-before-hook-action! hook-ctx) + :on-after-action (fn [result] + (lifecycle/notify-after-hook-action! hook-ctx result) + (swap! hook-results* conj result))} + db-before-hooks + config) + (when-let [additional-contexts (seq (keep (fn [{:keys [parsed exit]}] + (when (zero? exit) + (shared/not-blank (get parsed "additionalContext")))) + @hook-results*))] + (swap! db* assoc-in [:chats chat-id :startup-context] + (string/join "\n\n" additional-contexts))) + ;; Note: :chat-start-fired is set by prompt* (via swap-vals!) before this + ;; runs, since prompt* is what decides whether start hooks fire at all. + (let [stop-result (some (fn [{:keys [parsed name] :as result}] + (when (f.hooks/successful-continue-false? result) + {:stop-turn? true + :stop-reason (shared/not-blank (get parsed "stopReason")) + :stop-hook-name name})) + @hook-results*)] + (or stop-result {:stop-turn? false})))) (defn ^:private prompt* [{:keys [model]} {:keys [chat-id contexts message agent agent-config db* messenger config metrics] :as base-chat-ctx}] (let [provided-chat-id chat-id ;; Snapshot DB to detect new/resumed chat BEFORE hooks mutate it - [db0 _] (swap-vals! db* assoc-in [:chat-start-fired chat-id] true) - existing-chat-before-prompt (get-in db0 [:chats chat-id]) - chat-start-fired? (get-in db0 [:chat-start-fired chat-id]) + [db-before-hooks _] (swap-vals! db* assoc-in [:chat-start-fired chat-id] true) + existing-chat-before-prompt (get-in db-before-hooks [:chats chat-id]) + chat-start-fired? (get-in db-before-hooks [:chat-start-fired chat-id]) has-messages? (seq (:messages existing-chat-before-prompt)) resumed? (boolean (and (not chat-start-fired?) provided-chat-id has-messages?)) - ;; Trigger chatStart hook as early as possible so its additionalContext - ;; is visible in build-chat-instructions and /prompt-show. - _ (when-not chat-start-fired? - ;; Wait for plugin resolution so plugin-defined hooks are available - (config/await-plugins-resolved!) - (let [config (config/all db0) - hook-results* (atom []) - hook-ctx {:messenger messenger :chat-id chat-id}] - (f.hooks/trigger-if-matches! :chatStart - (merge (f.hooks/base-hook-data db0) - {:chat-id chat-id - :resumed resumed?}) - {:on-before-action (partial lifecycle/notify-before-hook-action! hook-ctx) - :on-after-action (fn [result] - (lifecycle/notify-after-hook-action! hook-ctx result) - (swap! hook-results* conj result))} - db0 - config) - ;; Collect additionalContext from all chatStart hooks and store - ;; it as startup-context for this chat. - (when-let [additional-contexts (seq (keep #(get-in % [:parsed "additionalContext"]) @hook-results*))] - (swap! db* assoc-in [:chats chat-id :startup-context] - (string/join "\n\n" additional-contexts))) - ;; Mark chatStart as fired for this chat in this server run - (swap! db* assoc-in [:chat-start-fired chat-id] true))) - ;; Re-read DB after potential chatStart modifications - db @db* + ;; await-plugins-resolved! must run even though db-before-hooks may not + ;; have :model yet (model sync can lag behind plugin resolution). + _ (when-not chat-start-fired? (config/await-plugins-resolved!)) ;; Respect explicit model; otherwise prefer the chat's stored model ;; (so resumed chats keep the provider/model they started with, #417); ;; then fall back to the agent default if it resolves to an available ;; model; finally, deterministic default-model resolution. - full-model (or model - (let [stored-model (get-in db [:chats chat-id :model]) - agent-default-model (:defaultModel agent-config)] - (cond - (and stored-model - (contains? (:models db) stored-model)) - stored-model - - (and agent-default-model - (contains? (:models db) agent-default-model)) - agent-default-model - - :else - (default-model db config)))) - _ (when (seq contexts) - (lifecycle/send-content! {:messenger messenger :chat-id chat-id} :system {:type :progress - :state :running - :text "Parsing given context"})) - refined-contexts (concat - (f.context/agents-file-contexts db config) - (f.context/raw-contexts->refined contexts db)) - {static-rules :static path-scoped-rules :path-scoped} (f.rules/all-rules config (:workspace-folders db) agent full-model) - all-tools (f.tools/all-tools chat-id agent @db* config) - skills (->> (f.skills/all config (:workspace-folders db)) - (remove - (fn [skill] - (= :deny (f.tools/approval all-tools - {:server {:name "eca"} :name "skill"} - {"name" (:name skill)} - db - config - agent))))) - repo-map* (delay (f.index/repo-map db config {:as-string? true})) - prompt-cache (get-in db [:chats chat-id :prompt-cache]) - instructions (if (and prompt-cache - (= (:agent prompt-cache) agent) - (= (:model prompt-cache) full-model)) - {:static (:static prompt-cache) - :dynamic (f.prompt/build-dynamic-instructions refined-contexts db)} - (let [result (f.prompt/build-chat-instructions - refined-contexts static-rules path-scoped-rules skills repo-map* - agent config chat-id all-tools db)] - (swap! db* assoc-in [:chats chat-id :prompt-cache] - {:static (:static result) - :agent agent - :model full-model}) - result)) - image-contents (->> refined-contexts - (filter #(= :image (:type %)))) - expanded-prompt-contexts (when-let [contexts-str (some-> (f.context/contexts-str-from-prompt message db) - seq - (f.prompt/contexts-str repo-map* nil))] - [{:type :text :text contexts-str}]) - user-messages [{:role "user" :content (vec (concat [{:type :text :text message}] - expanded-prompt-contexts - image-contents))}] - [provider model] (when full-model (shared/full-model->provider+model full-model)) - chat-ctx (merge base-chat-ctx - {:instructions instructions - :user-messages user-messages - :full-model full-model - :provider provider - :model model - :messenger messenger}) - decision (message->decision message db config)] - ;; Show original prompt to user, but LLM receives the modified version - (lifecycle/send-content! chat-ctx :user {:type :text - :content-id (:user-content-id chat-ctx) - :text (str message "\n")}) - ;; Clear prompt-finished? so finish-chat-prompt! can properly terminate - ;; this prompt cycle. prompt-messages! already does this for regular - ;; prompts, but commands and mcp-prompts go through different paths. - (swap! db* update-in [:chats chat-id] dissoc :prompt-finished?) - (case (:type decision) - :mcp-prompt (send-mcp-prompt! decision chat-ctx) - :eca-command (handle-command! decision chat-ctx) - :prompt-message (prompt-messages! user-messages :prompt-message chat-ctx)) - (metrics/count-up! "prompt-received" - {:full-model full-model - :agent agent} - metrics) - {:chat-id chat-id - :model full-model - :status :prompting})) + full-model (resolve-full-model model @db* chat-id agent-config config) + start-hook-result (when-not chat-start-fired? + (run-start-hooks! base-chat-ctx db-before-hooks resumed? full-model))] + (if (:stop-turn? start-hook-result) + (do + (lifecycle/send-turn-stopped-by-hook! base-chat-ctx + (:stop-hook-name start-hook-result) + (:stop-reason start-hook-result)) + (lifecycle/finish-chat-prompt-stopped! :idle base-chat-ctx) + {:chat-id chat-id + :model full-model + :status :prompting}) + (let [;; Re-read DB after potential chatStart modifications + db @db* + _ (when (seq contexts) + (lifecycle/send-content! {:messenger messenger :chat-id chat-id} :system {:type :progress + :state :running + :text "Parsing given context"})) + refined-contexts (concat + (f.context/agents-file-contexts db config) + (f.context/raw-contexts->refined contexts db)) + {static-rules :static path-scoped-rules :path-scoped} (f.rules/all-rules config (:workspace-folders db) agent full-model) + all-tools (f.tools/all-tools chat-id agent @db* config) + skills (->> (f.skills/all config (:workspace-folders db)) + (remove + (fn [skill] + (= :deny (f.tools/approval all-tools + {:server {:name "eca"} :name "skill"} + {"name" (:name skill)} + db + config + agent))))) + repo-map* (delay (f.index/repo-map db config {:as-string? true})) + prompt-cache (get-in db [:chats chat-id :prompt-cache]) + instructions (if (and prompt-cache + (= (:agent prompt-cache) agent) + (= (:model prompt-cache) full-model)) + {:static (:static prompt-cache) + :dynamic (f.prompt/build-dynamic-instructions refined-contexts db)} + (let [result (f.prompt/build-chat-instructions + refined-contexts static-rules path-scoped-rules skills repo-map* + agent config chat-id all-tools db)] + (swap! db* assoc-in [:chats chat-id :prompt-cache] + {:static (:static result) + :agent agent + :model full-model}) + result)) + image-contents (->> refined-contexts + (filter #(= :image (:type %)))) + expanded-prompt-contexts (when-let [contexts-str (some-> (f.context/contexts-str-from-prompt message db) + seq + (f.prompt/contexts-str repo-map* nil))] + [{:type :text :text contexts-str}]) + user-messages [{:role "user" :content (vec (concat [{:type :text :text message}] + expanded-prompt-contexts + image-contents))}] + [provider model] (when full-model (shared/full-model->provider+model full-model)) + chat-ctx (merge base-chat-ctx + {:instructions instructions + :user-messages user-messages + :full-model full-model + :provider provider + :model model + :messenger messenger}) + decision (message->decision message db config)] + ;; Show original prompt to user, but LLM receives the modified version + (lifecycle/send-content! chat-ctx :user {:type :text + :content-id (:user-content-id chat-ctx) + :text (str message "\n")}) + ;; Clear prompt-finished? so finish-chat-prompt! can properly terminate + ;; this prompt cycle. prompt-messages! already does this for regular + ;; prompts, but commands and mcp-prompts go through different paths. + (swap! db* update-in [:chats chat-id] dissoc :prompt-finished? :follow-up-active?) + (case (:type decision) + :mcp-prompt (send-mcp-prompt! decision chat-ctx) + :eca-command (handle-command! decision chat-ctx) + :prompt-message (prompt-messages! user-messages :prompt-message chat-ctx)) + (metrics/count-up! "prompt-received" + {:full-model full-model + :agent agent} + metrics) + {:chat-id chat-id + :model full-model + :status :prompting})))) (def ^:private max-client-chat-id-length 256) @@ -1391,8 +1524,15 @@ :agent selected-agent :agent-config agent-config :trust effective-trust - :variant (or variant (:variant agent-config))} - :parent-chat-id (get-in @db* [:chats chat-id :parent-chat-id])) + :variant (or variant (:variant agent-config)) + :on-follow-up (fn [follow-up-text chat-ctx] + (prompt-messages! + [{:role "user" + :content [{:type :text + :text follow-up-text}]}] + :follow-up + chat-ctx))} + :parent-chat-id (db/parent-chat-id @db* chat-id)) _ (when (some? effective-trust) (swap! db* assoc-in [:chats chat-id :trust] effective-trust)) ;; When we seeded the default (the client didn't ask), align the @@ -1407,13 +1547,13 @@ (logger/error e) (lifecycle/send-content! base-chat-ctx :system {:type :text :text (str "Error: " (ex-message e) "\n\nCheck ECA stderr for more details.")}) - (lifecycle/finish-chat-prompt! :idle (dissoc base-chat-ctx :on-finished-side-effect)) + (lifecycle/finish-chat-prompt! :idle (lifecycle/strip-hook-callbacks base-chat-ctx)) {:chat-id chat-id :model "error" :status :error}))))))) (defn tool-call-approve [{:keys [chat-id tool-call-id save]} db* messenger metrics] - (logger/with-chat-context chat-id (get-in @db* [:chats chat-id :parent-chat-id]) + (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}) @@ -1429,7 +1569,7 @@ (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] - (logger/with-chat-context chat-id (get-in @db* [:chats chat-id :parent-chat-id]) + (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}) @@ -1472,7 +1612,7 @@ (defn prompt-steer [{:keys [chat-id message]} db*] - (logger/with-chat-context chat-id (get-in @db* [:chats chat-id :parent-chat-id]) + (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) (when (and (string? message) (not (string/blank? message)) (identical? :running (get-in @db* [:chats chat-id :status]))) @@ -1485,7 +1625,7 @@ No-op if no steer message is pending or the chat is not present. Idempotent: cancelling an already-consumed steer is silent." [{:keys [chat-id]} db*] - (logger/with-chat-context chat-id (get-in @db* [:chats chat-id :parent-chat-id]) + (logger/with-chat-context chat-id (db/parent-chat-id @db* chat-id) (let [removed?* (volatile! false)] (swap! db* (fn [db] (if (get-in db [:chats chat-id :steer-message]) @@ -1499,7 +1639,7 @@ ([params db* messenger metrics] (prompt-stop params db* messenger metrics {})) ([{:keys [chat-id]} db* messenger metrics {:keys [silent?]}] - (logger/with-chat-context chat-id (get-in @db* [:chats chat-id :parent-chat-id]) + (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? @@ -1508,7 +1648,7 @@ :db* db* :metrics metrics :messenger messenger - :parent-chat-id (get-in @db* [:chats chat-id :parent-chat-id])}] + :parent-chat-id (db/parent-chat-id @db* chat-id)}] (when-not silent? (lifecycle/send-content! chat-ctx :system {:type :text :text "\nPrompt stopped"})) @@ -1520,25 +1660,29 @@ :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 (dissoc chat-ctx :on-finished-side-effect))))))) + (lifecycle/finish-chat-prompt! :stopping (lifecycle/strip-hook-callbacks chat-ctx))))))) (defn delete-chat [{:keys [chat-id]} db* messenger config metrics] - (when-let [chat (get-in @db* [:chats chat-id])] - ;; Trigger chatEnd hook BEFORE deleting (chat still exists in cache) - (f.hooks/trigger-if-matches! :chatEnd - (merge (f.hooks/base-hook-data @db*) - {:chat-id chat-id - :title (:title chat) - :message-count (count (:messages chat))}) - {} - @db* - config)) - ;; Delete chat from memory - (swap! db* update :chats dissoc chat-id) - (messenger/chat-deleted messenger {:chat-id chat-id}) - ;; Save updated cache (without this chat) - (db/update-workspaces-cache! @db* metrics)) + (let [db @db*] + (logger/with-chat-context chat-id (db/parent-chat-id db chat-id) + (when-let [chat (get-in db [:chats chat-id])] + ;; Trigger chatEnd hook BEFORE deleting (chat still exists in cache) + (let [{:keys [agent model variant]} chat + hook-data (f.hooks/chat-hook-data db {:chat-id chat-id + :agent agent + :full-model model + :variant variant})] + (f.hooks/trigger-if-matches! :chatEnd + hook-data + {} + db + config))) + ;; Delete chat from memory + (swap! db* update :chats dissoc chat-id) + (messenger/chat-deleted messenger {:chat-id chat-id}) + ;; Save updated cache (without this chat) + (db/update-workspaces-cache! @db* metrics)))) (defn clear-chat "Clear specific aspects of a chat. Currently supports clearing :messages." diff --git a/src/eca/features/chat/lifecycle.clj b/src/eca/features/chat/lifecycle.clj index c0aeaa55c..7368c8f78 100644 --- a/src/eca/features/chat/lifecycle.clj +++ b/src/eca/features/chat/lifecycle.clj @@ -1,5 +1,6 @@ (ns eca.features.chat.lifecycle (:require + [clojure.string :as string] [eca.db :as db] [eca.features.hooks :as f.hooks] [eca.features.login :as f.login] @@ -36,14 +37,63 @@ :content content} :parent-chat-id parent-chat-id))) +(defn hook-system-text + "User-facing standalone message attributed to a hook by name. + Ends with a blank line so the message renders as its own paragraph and never + visually glues to a following assistant stream (e.g. a preRequest systemMessage + emitted right before the model's reply)." + [hook-name text] + (str "Hook '" hook-name "': " text "\n\n")) + +(defn turn-stopped-by-hook-message + "Canonical user-facing message when a hook stops the current turn. + stopReason is never sent to the LLM; this is the user-only explanation." + [hook-name stop-reason] + (if (shared/not-blank stop-reason) + (str "Turn stopped by hook '" hook-name "': " stop-reason) + (str "Turn stopped by hook '" hook-name "'."))) + +(defn send-turn-stopped-by-hook! + "Surface the canonical turn-stopped message to the user as a system text." + [chat-ctx hook-name reason] + (send-content! chat-ctx :system + {:type :text + :text (turn-stopped-by-hook-message hook-name reason)})) + +(defn request-blocked-by-hook-message + "Canonical user-facing message when a preRequest hook blocks the prompt." + [hook-name reason] + (if (shared/not-blank reason) + (str "Request blocked by hook '" hook-name "': " reason) + (str "Request blocked by hook '" hook-name "'."))) + +(defn compaction-blocked-by-hook-message + "Canonical user-facing message when a preCompact hook (exit 2) blocks + compaction without stopping the turn. Names the blocking hook when known." + [hook-name] + (if (shared/not-blank hook-name) + (str "Compaction blocked by hook '" hook-name "'.") + "Compaction blocked by hook.")) + (defn- format-hook-output - "Format hook output for display, showing parsed JSON fields or raw output." - [{:strs [systemMessage replacedPrompt additionalContext] :as parsed} raw-output] - (if parsed - (cond-> (or systemMessage "Hook executed") - replacedPrompt (str "\nReplacedPrompt: " (pr-str replacedPrompt)) - additionalContext (str "\nAdditionalContext: " additionalContext)) - raw-output)) + "Format the hook execution block's text: 'Hook executed' (or raw plain-text + stdout when the hook emitted no JSON) plus structural effect lines. + systemMessage is intentionally NOT included here - it is surfaced as a + standalone user-facing message by notify-after-hook-action!, never as the + block's text. Tool hooks correlate with their tool widget via the structured + :tool-call-id sent on hookActionFinished, not via the block text." + [hook-type {:strs [replacedPrompt additionalContext followUp replacedOutput updatedInput] :as parsed} raw-output] + (let [replaced-prompt (shared/not-blank replacedPrompt) + additional-context (shared/not-blank additionalContext) + follow-up (shared/not-blank followUp)] + (if parsed + (cond-> "Hook executed" + (and (= :preRequest hook-type) replaced-prompt) (str "\nReplacedPrompt: " (pr-str replaced-prompt)) + (and (= :preToolCall hook-type) (some? updatedInput)) (str "\nUpdatedInput: " (pr-str updatedInput)) + (and (#{:postRequest :subagentPostRequest} hook-type) follow-up) (str "\nFollowUp: " follow-up) + (and (= :postToolCall hook-type) (string? replacedOutput)) (str "\nReplacedOutput: " (pr-str replacedOutput)) + additional-context (str "\nAdditionalContext: " additional-context)) + (not-empty raw-output)))) (defn notify-before-hook-action! [chat-ctx {:keys [id name type visible?]}] (when visible? @@ -53,53 +103,338 @@ :name name :id id}))) -(defn notify-after-hook-action! [chat-ctx {:keys [id name parsed raw-output raw-error exit type visible?]}] - (when (and visible? (not (get parsed "suppressOutput"))) - (send-content! chat-ctx :system - {:type :hookActionFinished - :action-type type - :id id - :name name - :status exit - :output (format-hook-output parsed raw-output) - :error raw-error}))) +(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) + suppress? (boolean (get parsed-effects "suppressOutput"))] + ;; Two independent channels: + ;; 1. Execution block, gated by `visible?`. If a started event was sent we must + ;; send a matching finished (same id) so the client closes the spinner. + ;; `suppressOutput` drops only the body, not the block. Never carries systemMessage. + (when visible? + (send-content! chat-ctx :system + (cond-> (assoc-some {:type :hookActionFinished + :action-type type + :id id + :name name + :status exit} + :tool-call-id tool-call-id) + (not suppress?) (assoc :output (format-hook-output hook-type parsed-effects raw-output) + :error raw-error)))) + + ;; 2. systemMessage: standalone message, independent of visible/suppressOutput. + (when-let [system-message (shared/not-blank (get parsed-effects "systemMessage"))] + (send-content! chat-ctx :system {:type :text :text (hook-system-text name system-message)})))) + +(defn strip-hook-callbacks + "Remove finish side-effect callbacks so abort/stop paths do not resume + special flows. Intentionally preserves :on-follow-up; whether postRequest + followUp can fire is controlled by :skip-post-request-hooks?." + [chat-ctx] + (dissoc chat-ctx :on-finished-side-effect :on-after-finish!)) (defn wrap-additional-context - "Return XML-wrapped additional context attributed to `from`." - [from content] - (format "\n%s\n" - (name from) + "Return XML-wrapped additional context." + [content] + (format "\n%s\n" content)) -(defn finish-chat-prompt! [status {:keys [message chat-id db* messenger metrics config on-finished-side-effect prompt-id] :as chat-ctx}] +(defn ^:private compact-hook-data + [db chat-ctx trigger extra] + (merge (f.hooks/chat-hook-data db chat-ctx) + {:triggered trigger} + extra)) + +(defn ^:private compact-block-reason + "Extract the user-facing stopReason from a continue:false hook result, or nil. + Exit 2 never produces a user-facing reason here." + [{:keys [parsed exit]}] + (when (zero? exit) + (shared/not-blank (get parsed "stopReason")))) + +(defn ^:private compact-hook-options + [chat-ctx after-fn] + {:on-before-action (partial notify-before-hook-action! chat-ctx) + :on-after-action (fn [result] + (notify-after-hook-action! chat-ctx result) + (after-fn result))}) + +(defn ^:private process-pre-compact-hook-result + "Pure function: fold a single hook result into accumulated state. + + acc is {:blocked? false, :reason nil, :hook-name nil, :stop-turn? false}" + [acc result] + (let [parsed (:parsed result) + success? (zero? (:exit result)) + exit-code-2? (= f.hooks/hook-rejection-exit-code (:exit result)) + stop-turn-result? (and success? (false? (get parsed "continue" true)))] + (cond + ;; continue:false blocks compaction and stops the turn (with optional stopReason). + stop-turn-result? + (merge acc {:blocked? true + :reason (compact-block-reason result) + :hook-name (:name result) + :stop-turn? true}) + + ;; Exit 2 blocks only compaction (no user-facing reason from stderr), but we + ;; still remember the first blocking hook's name for provenance in the + ;; user-facing "Compaction blocked by hook ''." message. + exit-code-2? + (cond-> (assoc acc :blocked? true) + (nil? (:hook-name acc)) (assoc :hook-name (:name result))) + + :else acc))) + +(defn run-pre-compact-hooks! + "Run preCompact hooks. Returns {:blocked? boolean :reason string-or-nil :stop-turn? boolean}. + Hooks match on triggered: \"manual\" or \"auto\". Compaction is blocked when + a hook returns {\"continue\":false} or exits with code 2; only continue:false + also requests stopping the current turn. :reason is only set for continue:false + (from stopReason); exit 2 does not produce a user-facing reason." + [{:keys [db* config] :as chat-ctx} trigger custom-instructions] + (let [hook-state* (atom {:blocked? false + :reason nil + :hook-name nil + :stop-turn? false}) + db @db*] + (f.hooks/trigger-if-matches! + :preCompact + (compact-hook-data db chat-ctx trigger + {:custom-instructions (or custom-instructions "")}) + (compact-hook-options + chat-ctx + (fn [result] + (swap! hook-state* process-pre-compact-hook-result result))) + db + config) + (let [{:keys [blocked? reason hook-name stop-turn?]} @hook-state*] + {:blocked? blocked? + :reason reason + :hook-name hook-name + :stop-turn? stop-turn?}))) + +(defn ^:private post-compact-summary-index + "Find the compact summary message inserted by compact-side-effect!. + The current history layout is compact_marker followed by a user summary; + return nil if that structure is not present so callers can safely fall back." + [messages] + (let [msgs (vec messages) + n (count msgs) + marker-idx (loop [i (dec n)] + (cond + (neg? i) nil + (= "compact_marker" (:role (msgs i))) i + :else (recur (dec i))))] + (when marker-idx + (some (fn [i] + (when (= "user" (:role (msgs i))) i)) + (range (inc marker-idx) n))))) + +(defn ^:private append-post-compact-contexts! + "Append additionalContext entries to the compact summary message that follows + the latest compact_marker. If no marker/summary is present, emit a warning + and leave history untouched." + [db* chat-id additional-contexts] + (when (seq additional-contexts) + (let [summary-idx (post-compact-summary-index (get-in @db* [:chats chat-id :messages]))] + (if summary-idx + (let [entries (mapv (fn [{:keys [content]}] + {:type :text :text (wrap-additional-context content)}) + additional-contexts)] + (swap! db* update-in [:chats chat-id :messages] + (fn [messages] + (let [messages (vec messages)] + (if (and (< summary-idx (count messages)) + (= "user" (:role (messages summary-idx)))) + (update-in messages [summary-idx :content] + #(into (if (string? %) + [{:type :text :text %}] + (vec %)) + entries)) + messages))))) + (let [db @db*] + (logger/with-chat-context chat-id (db/parent-chat-id db chat-id) + (logger/warn logger-tag "Skipping postCompact additionalContext: compact summary message not found" + {:chat-id chat-id}))))))) + +(defn run-post-compact-hooks! + "Run postCompact hooks after ECA has added the compact marker and compact + summary message. Collects additionalContext entries from successful hooks + and applies them to the compact summary message in history. + + Returns {:stop-turn? boolean} when no additionalContext was produced; when + one or more entries were produced, also returns :additional-contexts so + callers/tests can inspect what was appended." + [{:keys [db* config chat-id] :as chat-ctx} trigger compact-summary] + (let [db @db* + hook-state* (atom {:stop-turn? false + :stop-reason nil + :stop-hook-name nil + :additional-contexts []})] + (f.hooks/trigger-if-matches! + :postCompact + (compact-hook-data db chat-ctx trigger + {:compact-summary (or compact-summary "")}) + (compact-hook-options + chat-ctx + (fn [{:keys [parsed exit name] :as _result}] + (when (zero? exit) + (when (false? (get parsed "continue" true)) + (swap! hook-state* assoc + :stop-turn? true + :stop-reason (shared/not-blank (get parsed "stopReason")) + :stop-hook-name name)) + (when-let [additional-context (shared/not-blank (get parsed "additionalContext"))] + (swap! hook-state* update :additional-contexts conj + {:hook-name name :content additional-context}))))) + db + config) + (let [{:keys [stop-turn? stop-reason stop-hook-name additional-contexts]} @hook-state*] + (append-post-compact-contexts! db* chat-id additional-contexts) + (cond-> {:stop-turn? stop-turn? + :stop-reason stop-reason + :stop-hook-name stop-hook-name} + (seq additional-contexts) (assoc :additional-contexts additional-contexts))))) + +(defn complete-compact! + "Apply ECA's compact side effects, then run postCompact hooks (which + internally append any returned additionalContext to the compact summary)." + [{:keys [db* chat-id] :as chat-ctx} trigger] + (let [summary (get-in @db* [:chats chat-id :last-summary])] + (shared/compact-side-effect! chat-ctx (= "auto" trigger)) + (run-post-compact-hooks! chat-ctx trigger summary))) + +(defn ^:private run-post-request-hooks! + "Run postRequest (and subagentPostRequest for subagents) hooks. + Returns {:follow-up-text string-or-nil :stop-turn? boolean + :stop-reason string-or-nil :stop-hook-name string-or-nil}. + + postRequest exit 2 with stderr is treated as followUp: stderr becomes the + followUp text, analogous to how preToolCall/postToolCall exit 2 makes stderr + LLM-visible payload. This is because postRequest runs after the prompt + finished, so exit 2 cannot 'block' the request; instead it contributes a + continuation instruction." + [{:keys [db* config chat-id response] :as chat-ctx}] + (let [db @db* + results* (atom []) + subagent? (some? (get-in db [:chats chat-id :subagent])) + follow-up-active? (get-in db [:chats chat-id :follow-up-active?]) + ;; follow-up-active is always sent as a boolean (documented contract), + ;; so hook authors can rely on the key being present as true or false. + base-hook-data (assoc-some (f.hooks/chat-hook-data db chat-ctx) + :response response + :follow-up-active (boolean follow-up-active?)) + cb {:on-before-action (partial notify-before-hook-action! chat-ctx) + :on-after-action (fn [result] + (notify-after-hook-action! chat-ctx result) + (swap! results* conj result))} + _ (f.hooks/trigger-if-matches! :postRequest base-hook-data cb db config) + ;; A successful continue:false on a postRequest hook stops the turn, so + ;; the remaining relevant hooks must not run. For subagents this means + ;; subagentPostRequest is skipped, otherwise it could emit side effects + ;; (systemMessage, followUp) after the turn was already stopped. + post-request-stopped? (boolean (some f.hooks/successful-continue-false? @results*)) + _ (when (and subagent? (not post-request-stopped?)) + (f.hooks/trigger-if-matches! :subagentPostRequest + (assoc base-hook-data :parent-chat-id (db/parent-chat-id db chat-id)) + cb + db + config)) + hook-results @results* + follow-ups (->> hook-results + (keep (fn [{:keys [parsed exit raw-error]}] + (cond + (zero? exit) + (shared/not-blank (get parsed "followUp")) + + (= f.hooks/hook-rejection-exit-code exit) + (shared/not-blank raw-error)))) + seq) + follow-up-text (some->> follow-ups (string/join "\n\n")) + stop-result (some (fn [{:keys [parsed name] :as result}] + (when (f.hooks/successful-continue-false? result) + {:stop-reason (shared/not-blank (get parsed "stopReason")) + :stop-hook-name name})) + hook-results)] + {:follow-up-text follow-up-text + :stop-turn? (boolean stop-result) + :stop-reason (:stop-reason stop-result) + :stop-hook-name (:stop-hook-name stop-result)})) + +(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}) + (send-content! chat-ctx :system + {:type :progress + :state :finished}) + (when-not (get-in @db* [:chats chat-id :created-at]) + (swap! db* assoc-in [:chats chat-id :created-at] (System/currentTimeMillis)))) + +(defn ^:private dispatch-finish-callbacks! + "Dispatch finish-flow callbacks. `on-finished-side-effect` always runs first + (and may itself request `stop-after-finish?`). After that, exactly one + continuation may fire, in priority order: + + - nothing, when the turn is stopping or a hook returned continue:false; + - `on-follow-up` with the followUp text, when present; + - `on-after-finish!`, unless a side-effect/caller requested stop-after-finish?." + [{:keys [db* chat-id] :as chat-ctx} + {:keys [follow-up-text stop-turn? stopping? stop-after-finish?] + :or {stop-turn? false stopping? false stop-after-finish? false}}] + (let [{:keys [on-follow-up on-finished-side-effect on-after-finish!]} chat-ctx + side-effect-result (when on-finished-side-effect (on-finished-side-effect)) + stop-after-finish? (or stop-after-finish? (:stop-after-finish? side-effect-result))] + (cond + (or stopping? stop-turn?) + nil + + (and follow-up-text on-follow-up) + (do + (swap! db* update-in [:chats chat-id] dissoc :auto-compacting? :compacting?) + (swap! db* assoc-in [:chats chat-id :follow-up-active?] true) + (on-follow-up follow-up-text chat-ctx)) + + (and (not (or follow-up-text stop-after-finish?)) on-after-finish!) + (on-after-finish!)))) + +(defn finish-chat-prompt! [status {:keys [chat-id db* metrics prompt-id + skip-post-request-hooks?] + :as chat-ctx}] (when-not (get-in @db* [:chats chat-id :prompt-finished?]) (when-not (and prompt-id (not= prompt-id (get-in @db* [:chats chat-id :prompt-id]))) - (when-not (get-in @db* [:chats chat-id :auto-compacting?]) - (swap! db* assoc-in [:chats chat-id :prompt-finished?] true) - (swap! db* assoc-in [:chats chat-id :status] status) - (swap! db* update-in [:chats chat-id] dissoc :steer-message) - (messenger/chat-status-changed messenger {:chat-id chat-id :status status}) - (let [db @db* - subagent? (some? (get-in db [:chats chat-id :subagent])) - hook-type (if subagent? :subagentPostRequest :postRequest) - hook-data (cond-> (merge (f.hooks/chat-hook-data db chat-id (:agent chat-ctx)) - {:prompt message}) - subagent? (assoc :parent-chat-id (get-in db [:chats chat-id :parent-chat-id])))] - (f.hooks/trigger-if-matches! hook-type - hook-data - {:on-before-action (partial notify-before-hook-action! chat-ctx) - :on-after-action (partial notify-after-hook-action! chat-ctx)} - db - config)) - (send-content! chat-ctx :system - {:type :progress - :state :finished}) - (when-not (get-in @db* [:chats chat-id :created-at]) - (swap! db* assoc-in [:chats chat-id :created-at] (System/currentTimeMillis)))) - (when (and on-finished-side-effect - (not (identical? :stopping (get-in @db* [:chats chat-id :status])))) - (on-finished-side-effect)) - (db/update-workspaces-cache! @db* metrics)))) + (let [auto-compacting? (get-in @db* [:chats chat-id :auto-compacting?]) + {:keys [follow-up-text stop-turn? stop-reason stop-hook-name]} + (when (and (not auto-compacting?) (not skip-post-request-hooks?)) + (run-post-request-hooks! chat-ctx)) + stopping? (identical? :stopping (get-in @db* [:chats chat-id :status]))] + (when-not auto-compacting? + (swap! db* assoc-in [:chats chat-id :prompt-finished?] true) + (swap! db* update-in [:chats chat-id] dissoc :steer-message) + (apply-status-transition! chat-ctx status)) + ;; A postRequest hook that returned continue:false stops the turn and + ;; cancels followUp; surface the reason to the user (prefixed with the + ;; hook name), consistent with the other turn-stopping hooks. + (when stop-turn? + (send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason)) + (dispatch-finish-callbacks! chat-ctx {:follow-up-text follow-up-text + :stop-turn? stop-turn? + :stopping? stopping?}) + (db/update-workspaces-cache! @db* metrics))))) + +(defn finish-chat-prompt-stopped! + "Finish a turn that was halted by a hook (continue:false) or otherwise aborted. + Skips postRequest hooks and strips finish side-effect callbacks so no + postRequest / followUp / continuation fires — the turn ends cleanly and the + user can start a new prompt. Use this for every hook turn-stop so they behave + consistently (preRequest, preToolCall, postToolCall, ...)." + [status chat-ctx] + (finish-chat-prompt! status + (-> chat-ctx + (assoc :skip-post-request-hooks? true) + strip-hook-callbacks))) (defn maybe-renew-auth-token [chat-ctx] (f.login/maybe-renew-auth-token! @@ -110,7 +445,7 @@ :text "Renewing auth token"})) :on-error (fn [error-msg] (send-content! chat-ctx :system {:type :text :text error-msg}) - (finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect)) + (finish-chat-prompt! :idle (strip-hook-callbacks chat-ctx)) (throw (ex-info "Auth token renew failed" {})))} chat-ctx)) @@ -121,7 +456,7 @@ (:prompt-finished? chat) superseded?)] (when stopped? - (finish-chat-prompt! :idle (dissoc chat-ctx :on-finished-side-effect)) + (finish-chat-prompt! :idle (strip-hook-callbacks chat-ctx)) (logger/info logger-tag "Chat prompt stopped:" chat-id (when superseded? "(superseded)")) (throw (ex-info "Chat prompt stopped" {:silent? true :chat-id chat-id}))))) diff --git a/src/eca/features/chat/tool_calls.clj b/src/eca/features/chat/tool_calls.clj index ef7ca0806..53c2358c1 100644 --- a/src/eca/features/chat/tool_calls.clj +++ b/src/eca/features/chat/tool_calls.clj @@ -28,22 +28,58 @@ (def ^:private logger-tag "[CHAT]") +(defn ^:private update-tool-output-contents! + "Apply `f` to the :contents of the tool_call_output message matching + `tool-call-id`, a no-op when no such message exists. Scans messages backwards + since the tool output is usually one of the last items." + [db* chat-id tool-call-id f] + (swap! db* update-in [:chats chat-id :messages] + (fn [messages] + (let [idx (llm-util/find-last-msg-idx + #(and (= "tool_call_output" (:role %)) + (= tool-call-id (get-in % [:content :id]))) + messages)] + (if idx + (update-in messages [idx :content :output :contents] f) + messages))))) + (defn ^:private append-post-tool-additional-context! "Append additionalContext (wrapped as XML) from a postToolCall hook to the matching tool_call_output message so LLM sees it in the next round." - [db* chat-id tool-call-id hook-name additional-context] - (when (not (string/blank? additional-context)) - (let [entry {:type :text :text (lifecycle/wrap-additional-context hook-name additional-context)}] - (swap! db* update-in [:chats chat-id :messages] - ;; Optimized: Scans messages backwards since the tool output is likely one of the last items. - #(let [idx (llm-util/find-last-msg-idx - (fn [msg] - (and (= "tool_call_output" (:role msg)) - (= tool-call-id (get-in msg [:content :id])))) - %)] - (if idx - (update-in % [idx :content :output :contents] conj entry) - %)))))) + [db* chat-id tool-call-id additional-context] + (when-not (string/blank? additional-context) + (let [entry {:type :text :text (lifecycle/wrap-additional-context additional-context)}] + (update-tool-output-contents! db* chat-id tool-call-id #(conj % entry))))) + +(defn ^:private replace-tool-output! + "Replace the tool result content of the matching tool_call_output message. + replacedOutput is a string that replaces the original tool result. + An empty string hides the result (empty content)." + [db* chat-id tool-call-id replaced-output] + (update-tool-output-contents! db* chat-id tool-call-id + (constantly [{:type :text :text replaced-output}]))) + +(defn ^:private post-tool-call-stop-info + "Single pass over tool-call states. Returns + {:stop-turn? boolean :stop-reason string-or-nil :stop-hook-name string-or-nil} + describing whether any postToolCall hook requested a current-turn stop + (continue:false) and the first stop reason / hook name found, if any." + [tool-calls prompt-id] + (let [current-prompt-stop? (fn [state] + (and (some? prompt-id) + (:post-tool-call-stop? state) + (= prompt-id (:post-tool-call-stop-prompt-id state))))] + (reduce (fn [acc [_ state]] + (if (current-prompt-stop? state) + (cond-> (assoc acc :stop-turn? true) + (and (nil? (:stop-reason acc)) (:post-tool-call-stop-reason state)) + (assoc :stop-reason (:post-tool-call-stop-reason state)) + + (and (nil? (:stop-hook-name acc)) (:post-tool-call-stop-hook-name state)) + (assoc :stop-hook-name (:post-tool-call-stop-hook-name state))) + acc)) + {:stop-turn? false :stop-reason nil :stop-hook-name nil} + tool-calls))) ;;; Helper functions for tool call state management @@ -63,35 +99,72 @@ (into {}))) (defn ^:private run-post-tool-call-hooks! - "Run postToolCall hooks and append any additionalContext to the tool output." + "Run postToolCall hooks and append any additionalContext to the tool output. + Returns {:stop-turn? boolean :stop-reason string-or-nil} when a hook returns + continue:false." [db* chat-ctx tool-call-id event-data] - (let [tool-call-state (get-tool-call-state @db* (:chat-id chat-ctx) tool-call-id) - chat-id (:chat-id chat-ctx) + (let [chat-id (:chat-id chat-ctx) + db @db* + tool-call-state (get-tool-call-state db chat-id tool-call-id) native-tools (filter #(= :native (:origin %)) - (f.tools/all-tools chat-id (:agent chat-ctx) @db* (:config chat-ctx)))] + (f.tools/all-tools chat-id (:agent chat-ctx) db (:config chat-ctx))) + hook-state* (atom {:stop-turn? false :stop-reason nil :stop-hook-name nil})] (f.hooks/trigger-if-matches! :postToolCall - (merge (f.hooks/chat-hook-data @db* chat-id (:agent chat-ctx)) + (merge (f.hooks/chat-hook-data db chat-ctx) {:tool-name (:name tool-call-state) :server (:server tool-call-state) :tool-input (:arguments tool-call-state) - :tool-response (:outputs event-data) - :error (:error event-data)}) + :tool-response (tools.util/contents->text (:outputs event-data)) + :error (:error event-data) + :tool-call-id tool-call-id}) {:on-before-action (partial lifecycle/notify-before-hook-action! chat-ctx) - :on-after-action (fn [{:keys [parsed name] :as result}] - ;; Always notify UI - (lifecycle/notify-after-hook-action! chat-ctx result) - ;; If hook provided additionalContext, append as XML to the tool output - (when-let [ac (get parsed "additionalContext")] - (append-post-tool-additional-context! - (:db* chat-ctx) - (:chat-id chat-ctx) - tool-call-id - name - ac))) + :on-after-action (fn [{:keys [parsed raw-error name exit] :as result}] + (let [stop-reason (shared/not-blank (get parsed "stopReason"))] + ;; Always notify UI, tagged with the tool-call-id for correlation. + (lifecycle/notify-after-hook-action! + chat-ctx (assoc result :tool-call-id tool-call-id)) + (cond + ;; postToolCall exit 2 hides the original result from the LLM. + ;; stderr becomes the replacement content; empty stderr falls back to a + ;; default placeholder so the LLM still sees a meaningful signal. + ;; raw-error is already normalized to nil-or-non-empty-string by + ;; run-and-parse-output!, so a plain `or` is enough here. + (= f.hooks/hook-rejection-exit-code exit) + (replace-tool-output! + (:db* chat-ctx) + (:chat-id chat-ctx) + tool-call-id + (or raw-error "[Tool output hidden by hook]")) + + (zero? exit) + (do + (when (false? (get parsed "continue" true)) + (swap! hook-state* + #(-> (assoc % :stop-turn? true :stop-hook-name name) + (assoc-some :stop-reason stop-reason)))) + (when-let [replaced-output (get parsed "replacedOutput")] + (if (string? replaced-output) + (replace-tool-output! + (:db* chat-ctx) + (:chat-id chat-ctx) + tool-call-id + replaced-output) + (logger/warn logger-tag "Ignoring non-string replacedOutput from postToolCall hook" + {:hook-name name + :tool-call-id tool-call-id + :value replaced-output}))) + ;; If hook provided additionalContext, append as XML to the tool output + (when-let [ac (shared/not-blank (get parsed "additionalContext"))] + (append-post-tool-additional-context! + (:db* chat-ctx) + (:chat-id chat-ctx) + tool-call-id + ac)))))) :native-tools native-tools} - @db* - (:config chat-ctx)))) + db + (:config chat-ctx)) + @hook-state*)) ;;; Event-driven state machine for tool calls @@ -172,10 +245,6 @@ {:status :execution-approved :actions [:set-decision-reason :deliver-approval-true]} - [:waiting-approval :hook-rejected] - {:status :rejected - :actions [:set-decision-reason :set-hook-continue :set-hook-stop-reason :deliver-approval-false]} - [:waiting-approval :user-reject] {:status :rejected :actions [:set-decision-reason :deliver-approval-false :log-rejection]} @@ -184,10 +253,6 @@ {:status :rejected :actions [:send-toolCallRejected]} - [:execution-approved :hook-rejected] - {:status :rejected - :actions [:set-decision-reason :set-hook-continue :set-hook-stop-reason]} - [:execution-approved :execution-start] {:status :executing :actions [:set-start-time :add-future :send-toolCallRunning :send-progress]} @@ -366,7 +431,15 @@ :summary (:summary event-data)))) :trigger-post-tool-call-hook - (run-post-tool-call-hooks! db* chat-ctx tool-call-id event-data) + (let [{:keys [stop-turn? stop-reason stop-hook-name]} (run-post-tool-call-hooks! db* chat-ctx tool-call-id event-data)] + (when (or stop-turn? stop-reason) + (swap! db* update-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id] + (fn [state] + (cond-> (or state {}) + stop-turn? (assoc :post-tool-call-stop? true + :post-tool-call-stop-prompt-id (:prompt-id chat-ctx)) + stop-reason (assoc :post-tool-call-stop-reason stop-reason) + stop-hook-name (assoc :post-tool-call-stop-hook-name stop-hook-name)))))) ;; Actions on parts of the state :deliver-approval-false @@ -400,9 +473,9 @@ ;; :arguments (map), :summary, :details and :manual-approval are ;; initialized by the :init-arguments action ;; :start-time (long) is initialized by the :set-start-time action - ;; :future (future) is initialized by the :add-future action - ;; :resources (map) is updated by the :add-resources and remove-resources actions - ;; NOTE: :future and :resources are forcibly removed from the state directly, NOT VIA ACTIONS. + ;; :future (future) is added by the :add-future action and removed by :remove-future + ;; :resources (map) is updated by the :add-resources and :remove-resources actions, + ;; and removed wholesale by :remove-all-resources during cleanup :name (:name event-data) :full-name (:full-name event-data) :server (:server event-data) @@ -431,14 +504,6 @@ (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :decision-reason] (:reason event-data)) - :set-hook-continue - (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :hook-continue] - (:hook-continue event-data)) - - :set-hook-stop-reason - (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :hook-stop-reason] - (:hook-stop-reason event-data)) - :set-start-time (swap! db* assoc-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :start-time] (:start-time event-data)) @@ -461,7 +526,7 @@ #(apply dissoc %1 %2) (:resources event-data)) :remove-all-resources - (swap! db* update-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id :resources] + (swap! db* update-in [:chats (:chat-id chat-ctx) :tool-calls tool-call-id] dissoc :resources) :remove-all-promises @@ -523,28 +588,52 @@ {:status status :actions actions})) +(def ^:private hook-approval-rank + {"allow" 1 + "ask" 2 + "deny" 3}) + +(defn ^:private stronger-hook-approval + [current candidate] + (if (> (get hook-approval-rank candidate 0) + (get hook-approval-rank current 0)) + candidate + current)) + (defn ^:private process-pre-tool-call-hook-result "Pure function: fold a single hook result into accumulated state. - acc is {:hook-results [], :approval-override nil, :hook-rejected? false, - :hook-rejection-reason nil, :hook-continue true, :hook-stop-reason nil}" + acc is {:hook-results [], :approval-override nil, + :tool-call-rejected-by-hook? false, :tool-call-rejection-reason nil, + :stop-turn? false, :stop-reason nil, :stop-hook-name nil}" [acc result] (let [parsed (:parsed result) - hook-approval (get parsed "approval") - exit-code-2? (= f.hooks/hook-rejection-exit-code (:exit result))] + success? (zero? (:exit result)) + exit-code-2? (= f.hooks/hook-rejection-exit-code (:exit result)) + stop-turn-result? (and success? (false? (get parsed "continue" true))) + ;; success? (exit 0) and exit-code-2? are mutually exclusive, so the + ;; approval field is only read on exit 0. + hook-approval (when success? + (get parsed "approval"))] (cond-> (update acc :hook-results conj result) - ;; Handle rejection (exit code 2 or explicit deny) + ;; Handle current-turn stop for successful hooks, even without rejection. + stop-turn-result? + (merge {:stop-turn? true + :stop-reason (shared/not-blank (get parsed "stopReason")) + :stop-hook-name (:name result)}) + + ;; Handle rejection (exit code 2 or successful explicit deny) (or exit-code-2? (= "deny" hook-approval)) - (merge {:hook-rejected? true - :hook-rejection-reason (or (get parsed "additionalContext") - (:raw-error result) - "Tool call rejected by hook") - :hook-continue (get parsed "continue" true) - :hook-stop-reason (get parsed "stopReason")}) + (merge {:tool-call-rejected-by-hook? true + :tool-call-rejection-reason (if exit-code-2? + (or (:raw-error result) + "Tool call rejected by hook") + (or (shared/not-blank (get parsed "additionalContext")) + "Tool call rejected by hook"))}) - ;; Handle approval override (allow/ask) when not exit-code-2 - (and hook-approval (not exit-code-2?)) - (assoc :approval-override hook-approval)))) + ;; Handle approval override with hook precedence: deny > ask > allow. + (contains? hook-approval-rank hook-approval) + (update :approval-override stronger-hook-approval hook-approval)))) (defn ^:private decide-tool-call-action "Decides what action to take for a tool call, running hooks and collecting their results. @@ -553,15 +642,17 @@ - :decision (:ask | :allow | :deny) - :arguments (potentially modified by hooks) - :approval-override (from hooks) - - :hook-rejected? (boolean) + - :tool-call-rejected-by-hook? (boolean, explicit hook rejection via exit 2 or approval:deny) + - :tool-call-blocked-by-hook? (boolean, hook rejection or current-turn stop prevents execution) - :reason (map with :code and :text, when decision is :allow or :deny) - - :hook-continue (boolean, for hook rejections) - - :hook-stop-reason (string, for hook rejections) + - :stop-turn? (boolean, when a successful hook returns continue:false) + - :stop-reason (string, for hook turn stops) + - :stop-hook-name (string, for hook turn stops) The on-before-hook-action and on-after-hook-action callbacks are optional (default to noops) and are used for UI notifications. In tests, these can be omitted." - [{:keys [full-name arguments]} all-tools db config agent-name chat-id - & [{:keys [on-before-hook-action on-after-hook-action trust] + [{:keys [full-name arguments] tool-call-id :id} all-tools db config agent-name chat-id + & [{:keys [on-before-hook-action on-after-hook-action trust full-model variant] :or {on-before-hook-action (fn [_] nil) on-after-hook-action (fn [_] nil)}}]] (let [{:keys [name server] :as tool} (f.tools/resolve-tool full-name all-tools) @@ -586,51 +677,80 @@ ;; 2. Run hooks to collect modifications and approval overrides hook-state* (atom {:hook-results [] :approval-override nil - :hook-rejected? false - :hook-rejection-reason nil - :hook-continue true - :hook-stop-reason nil}) + :tool-call-rejected-by-hook? false + :tool-call-rejection-reason nil + :stop-turn? false + :stop-reason nil + :stop-hook-name nil}) _ (f.hooks/trigger-if-matches! :preToolCall - (merge (f.hooks/chat-hook-data db chat-id agent-name) + (merge (f.hooks/chat-hook-data db {:chat-id chat-id + :agent agent-name + :full-model full-model + :variant variant}) {:tool-name name :server server-name :tool-input arguments - :approval hook-approval}) + :approval hook-approval + :tool-call-id tool-call-id}) {:on-before-action on-before-hook-action :on-after-action (fn [result] - (on-after-hook-action result) + (on-after-hook-action (assoc result :tool-call-id tool-call-id)) (swap! hook-state* process-pre-tool-call-hook-result result)) :native-tools native-tools} db config) ;; 3. Merge all updatedInput from hooks - {:keys [hook-results approval-override hook-rejected? - hook-rejection-reason hook-continue hook-stop-reason]} @hook-state* - updated-inputs (keep #(get-in % [:parsed "updatedInput"]) hook-results) + {:keys [hook-results approval-override tool-call-rejected-by-hook? + tool-call-rejection-reason stop-turn? stop-reason stop-hook-name]} @hook-state* + updated-inputs (keep (fn [{:keys [parsed exit]}] + (when (zero? exit) + (get parsed "updatedInput"))) + hook-results) final-arguments (if (not-empty updated-inputs) (reduce merge arguments updated-inputs) arguments) arguments-modified? (boolean (seq updated-inputs)) - ;; 4. Determine Final Approval (Hook Override > Config, but hook rejection takes precedence) + ;; A current-turn stop is not a tool-specific rejection, but it still + ;; blocks this prepared tool and downstream must stop the agent loop. + ;; tool-call-blocked-by-hook? = any hook intervention that prevents the + ;; tool from running. + tool-call-blocked-by-hook? (or tool-call-rejected-by-hook? stop-turn?) + turn-stopped-only? (and stop-turn? (not tool-call-rejected-by-hook?)) + + ;; 4. Determine final approval. Hook deny/stop wins, but hook allow + ;; cannot override config deny/ask; it only confirms an already-allowed call. final-decision (cond - hook-rejected? :deny - approval-override (keyword approval-override) + tool-call-blocked-by-hook? :deny + (= :deny effective-approval) :deny + (= :ask effective-approval) :ask + (= "ask" approval-override) :ask :else effective-approval) - ;; 5. Build the reason map + ;; 5. Build the reason map. A hook current-turn stop is represented + ;; separately from a tool-specific rejection so downstream code and the + ;; LLM-visible history do not conflate the two intents. reason (case final-decision :allow (if trusted? {:code :trust-allow :text "Tool call allowed by trust mode"} {:code :user-config-allow :text "Tool call allowed by user config"}) - :deny (if hook-rejected? + :deny (cond + ;; Turn stop: LLM-visible text must stay neutral; + ;; stopReason is surfaced separately to the user. + turn-stopped-only? + {:code :hook-stopped + :text "Tool call stopped by hook"} + + tool-call-rejected-by-hook? {:code :hook-rejected - :text hook-rejection-reason} + :text tool-call-rejection-reason} + + :else {:code :user-config-deny :text "Tool call rejected by user config"}) nil)] @@ -639,15 +759,19 @@ (cond-> {:decision final-decision :arguments final-arguments :approval-override approval-override - :hook-rejected? hook-rejected? + :tool-call-rejected-by-hook? tool-call-rejected-by-hook? + :tool-call-blocked-by-hook? tool-call-blocked-by-hook? + :stop-turn? stop-turn? :arguments-modified? arguments-modified?} reason (assoc :reason reason) - hook-rejected? (assoc :hook-continue hook-continue - :hook-stop-reason hook-stop-reason)))) + stop-turn? (assoc :stop-reason stop-reason + :stop-hook-name stop-hook-name)))) (defn on-tools-called! [{:keys [db* config chat-id agent messenger metrics] :as chat-ctx} received-msgs* add-to-history! user-messages] (fn [tool-calls] + ;; postToolCall hooks report continue:false through the tool call state, + ;; accumulated per-tool by the state machine action :trigger-post-tool-call-hook. (let [all-tools (f.tools/all-tools chat-id agent @db* config) max-steps-reached? (check-subagent-max-steps! db* chat-id)] (lifecycle/assert-chat-not-stopped! chat-ctx) @@ -664,163 +788,165 @@ (when-not (string/blank? @received-msgs*) (add-to-history! {:role "assistant" :content [{:type :text :text @received-msgs*}]}) (reset! received-msgs* "")) - (let [rejected-tool-call-info* (atom nil)] - (run! (fn do-tool-call [{:keys [id full-name] :as tool-call}] - (let [approved?* (promise) - {:keys [origin name server parameters] - :as resolved-tool} (f.tools/resolve-tool full-name all-tools) - full-name (or (:full-name resolved-tool) full-name) - server-name (:name server) - spawn-agent? (and (= :native origin) - (= "eca" server-name) - (= "spawn_agent" name)) - arguments (if parameters - (tools.util/omit-optional-empty-string-args parameters (:arguments tool-call)) - (:arguments tool-call)) - tool-call (assoc tool-call - :full-name full-name - :arguments (cond-> arguments - spawn-agent? f.tools.agent/normalize-arguments)) - decision-plan (decide-tool-call-action - tool-call all-tools @db* config agent chat-id - {:on-before-hook-action (partial lifecycle/notify-before-hook-action! chat-ctx) - :on-after-hook-action (partial lifecycle/notify-after-hook-action! chat-ctx) - :trust (get-in @db* [:chats chat-id :trust])}) - {:keys [decision hook-rejected? reason hook-continue - hook-stop-reason arguments-modified?]} decision-plan - arguments (cond-> (:arguments decision-plan) - spawn-agent? f.tools.agent/normalize-arguments) - _ (when arguments-modified? - (lifecycle/send-content! chat-ctx :system {:type :hookActionFinished - :action-type "shell" - :id (str (random-uuid)) - :name "input-modification" - :status 0 - :output "Hook modified tool arguments"})) - _ (swap! db* assoc-in [:chats chat-id :tool-calls id :arguments] arguments) - tool-call (assoc tool-call :arguments arguments) - ask? (= :ask decision) - details (f.tools/tool-call-details-before-invocation name arguments server @db* config chat-id ask? id) - summary (f.tools/tool-call-summary all-tools full-name arguments config @db*)] - (when-not (#{:stopping :cleanup} (:status (get-tool-call-state @db* chat-id id))) - (transition-tool-call! db* chat-ctx id :tool-run {:approved?* approved?* - :future-cleanup-complete?* (promise) + (let [blocked-tool-call-info* (atom nil)] + ;; preToolCall continue:false short-circuits the rest of this batch. + (reduce (fn do-tool-call [_ {:keys [id full-name] :as tool-call}] + (let [approved?* (promise) + {:keys [origin name server parameters] + :as resolved-tool} (f.tools/resolve-tool full-name all-tools) + full-name (or (:full-name resolved-tool) full-name) + server-name (:name server) + spawn-agent? (and (= :native origin) + (= "eca" server-name) + (= "spawn_agent" name)) + arguments (if parameters + (tools.util/omit-optional-empty-string-args parameters (:arguments tool-call)) + (:arguments tool-call)) + tool-call (assoc tool-call + :full-name full-name + :arguments (cond-> arguments + spawn-agent? f.tools.agent/normalize-arguments)) + decision-plan (decide-tool-call-action + tool-call all-tools @db* config agent chat-id + {:on-before-hook-action (partial lifecycle/notify-before-hook-action! chat-ctx) + :on-after-hook-action (partial lifecycle/notify-after-hook-action! chat-ctx) + :trust (get-in @db* [:chats chat-id :trust]) + :full-model (:full-model chat-ctx) + :variant (:variant chat-ctx)}) + {:keys [decision tool-call-blocked-by-hook? reason stop-turn? + stop-reason stop-hook-name]} decision-plan + ;; updatedInput is surfaced inside the real preToolCall + ;; hook finished block (see lifecycle/format-hook-output), + ;; so it respects visible:false / suppressOutput and the + ;; hook's real id/name. No synthetic event here. + arguments (cond-> (:arguments decision-plan) + spawn-agent? f.tools.agent/normalize-arguments) + _ (swap! db* assoc-in [:chats chat-id :tool-calls id :arguments] arguments) + tool-call (assoc tool-call :arguments arguments) + ask? (= :ask decision) + details (f.tools/tool-call-details-before-invocation name arguments server @db* config chat-id ask? id) + summary (f.tools/tool-call-summary all-tools full-name arguments config @db*)] + (when-not (#{:stopping :cleanup} (:status (get-tool-call-state @db* chat-id id))) + (transition-tool-call! db* chat-ctx id :tool-run {:approved?* approved?* + :future-cleanup-complete?* (promise) + :name name + :server server-name + :origin origin + :arguments arguments + :manual-approval ask? + :details details + :summary summary})) + (when-not (#{:stopping :cleanup :rejected} (:status (get-tool-call-state @db* chat-id id))) + (case decision + :ask (transition-tool-call! db* chat-ctx id :approval-ask {:progress-text "Waiting for tool call approval"}) + :allow (transition-tool-call! db* chat-ctx id :approval-allow {:reason reason}) + :deny (transition-tool-call! db* chat-ctx id :approval-deny {:reason reason}) + (logger/warn logger-tag "Unknown value of approval" {:approval decision :tool-call-id id}))) + (if (and @approved?* (not tool-call-blocked-by-hook?)) + (when-not (#{:stopping :cleanup} (:status (get-tool-call-state @db* chat-id id))) + (lifecycle/assert-chat-not-stopped! chat-ctx) + (let [delayed-future + (delay + (future + (let [result (f.tools/call-tool! full-name + arguments + chat-id + id + agent + db* + config + messenger + metrics + (partial get-tool-call-state @db* chat-id id) + (partial transition-tool-call! db* chat-ctx id) + {:trust (get-in @db* [:chats chat-id :trust])}) + details (f.tools/tool-call-details-after-invocation name arguments details result + {:db @db* + :config config + :chat-id chat-id + :tool-call-id id}) + {:keys [start-time]} (get-tool-call-state @db* chat-id id) + total-time-ms (- (System/currentTimeMillis) start-time)] + (add-to-history! {:role "tool_call" + :content (assoc tool-call :name name - :server server-name + :details details + :summary summary :origin origin - :arguments arguments - :manual-approval ask? + :server server-name)}) + (add-to-history! {:role "tool_call_output" + :content (assoc tool-call + :name name + :error (:error result) + :output result + :total-time-ms total-time-ms :details details - :summary summary})) - (when-not (#{:stopping :cleanup :rejected} (:status (get-tool-call-state @db* chat-id id))) - (case decision - :ask (transition-tool-call! db* chat-ctx id :approval-ask {:progress-text "Waiting for tool call approval"}) - :allow (transition-tool-call! db* chat-ctx id :approval-allow {:reason reason}) - :deny (transition-tool-call! db* chat-ctx id :approval-deny {:reason reason}) - (logger/warn logger-tag "Unknown value of approval" {:approval decision :tool-call-id id}))) - (if (and @approved?* (not hook-rejected?)) - (when-not (#{:stopping :cleanup} (:status (get-tool-call-state @db* chat-id id))) - (lifecycle/assert-chat-not-stopped! chat-ctx) - (let [delayed-future - (delay - (future - (let [result (f.tools/call-tool! full-name - arguments - chat-id - id - agent - db* - config - messenger - metrics - (partial get-tool-call-state @db* chat-id id) - (partial transition-tool-call! db* chat-ctx id) - {:trust (get-in @db* [:chats chat-id :trust])}) - details (f.tools/tool-call-details-after-invocation name arguments details result - {:db @db* - :config config - :chat-id chat-id - :tool-call-id id}) - {:keys [start-time]} (get-tool-call-state @db* chat-id id) - total-time-ms (- (System/currentTimeMillis) start-time)] - (add-to-history! {:role "tool_call" - :content (assoc tool-call - :name name - :details details - :summary summary - :origin origin - :server server-name)}) - (add-to-history! {:role "tool_call_output" - :content (assoc tool-call - :name name - :error (:error result) - :output result - :total-time-ms total-time-ms - :details details - :summary summary - :origin origin - :server server-name)}) - (let [state (get-tool-call-state @db* chat-id id) status (:status state)] - (case status - :executing (transition-tool-call! db* - chat-ctx - id - :execution-end {:origin origin - :name name - :server server-name - :arguments arguments - :error (:error result) - :outputs (:contents result) - :total-time-ms total-time-ms - :progress-text "Generating" - :details details - :summary summary}) - :stopping (transition-tool-call! db* - chat-ctx - id - :stop-attempted {:origin origin - :name name - :server server-name - :arguments arguments - :error (:error result) - :outputs (:contents result) - :total-time-ms total-time-ms - :reason :user-stop :details - details - :summary summary}) - (logger/warn logger-tag "Unexpected value of :status in tool call" {:status status}))))))] - (transition-tool-call! db* - chat-ctx - id - :execution-start {:delayed-future delayed-future - :origin origin - :name name - :server server-name - :arguments arguments - :start-time (System/currentTimeMillis) - :details details - :summary summary - :progress-text (if (= name "spawn_agent") - "Waiting subagent" - "Calling tool")}))) - (let [tool-call-state (get-tool-call-state @db* chat-id id) - {:keys [code text]} (:decision-reason tool-call-state) - effective-hook-continue (when hook-rejected? hook-continue) - effective-hook-stop-reason (when hook-rejected? hook-stop-reason)] - (add-to-history! {:role "tool_call" :content tool-call}) - (add-to-history! {:role "tool_call_output" - :content (assoc tool-call :output {:error true :contents [{:text text :type :text}]})}) - (reset! rejected-tool-call-info* {:code code - :hook-continue effective-hook-continue - :hook-stop-reason effective-hook-stop-reason}) - (transition-tool-call! db* chat-ctx id :send-reject {:origin origin - :name name - :server server-name - :arguments arguments - :reason code - :details details - :summary summary}))))) - tool-calls) + :summary summary + :origin origin + :server server-name)}) + (let [state (get-tool-call-state @db* chat-id id) status (:status state)] + (case status + :executing (transition-tool-call! db* + chat-ctx + id + :execution-end {:origin origin + :name name + :server server-name + :arguments arguments + :error (:error result) + :outputs (:contents result) + :total-time-ms total-time-ms + :progress-text "Generating" + :details details + :summary summary}) + :stopping (transition-tool-call! db* + chat-ctx + id + :stop-attempted {:origin origin + :name name + :server server-name + :arguments arguments + :error (:error result) + :outputs (:contents result) + :total-time-ms total-time-ms + :reason :user-stop :details + details + :summary summary}) + (logger/warn logger-tag "Unexpected value of :status in tool call" {:status status}))))))] + (transition-tool-call! db* + chat-ctx + id + :execution-start {:delayed-future delayed-future + :origin origin + :name name + :server server-name + :arguments arguments + :start-time (System/currentTimeMillis) + :details details + :summary summary + :progress-text (if (= name "spawn_agent") + "Waiting subagent" + "Calling tool")}))) + (let [tool-call-state (get-tool-call-state @db* chat-id id) + {:keys [code text]} (:decision-reason tool-call-state)] + (add-to-history! {:role "tool_call" :content tool-call}) + (add-to-history! {:role "tool_call_output" + :content (assoc tool-call :output {:error true :contents [{:text text :type :text}]})}) + (reset! blocked-tool-call-info* {:code code + :stop-turn? (when tool-call-blocked-by-hook? stop-turn?) + :stop-reason (when tool-call-blocked-by-hook? stop-reason) + :stop-hook-name (when tool-call-blocked-by-hook? stop-hook-name)}) + (transition-tool-call! db* chat-ctx id :send-reject {:origin origin + :name name + :server server-name + :arguments arguments + :reason code + :details details + :summary summary}))) + (when (and tool-call-blocked-by-hook? stop-turn?) + (reduced nil)))) + nil + tool-calls) (lifecycle/assert-chat-not-stopped! chat-ctx) (doseq [[tool-call-id state] (get-active-tool-calls @db* chat-id)] (when-let [f (:future state)] @@ -863,18 +989,21 @@ config) with-fresh-auth #(some-> % (assoc :fresh-api-key refreshed-api-key :provider-auth refreshed-provider-auth))] - (if-let [rejection-info @rejected-tool-call-info*] + (if-let [blocked-info @blocked-tool-call-info*] (let [reason-code - (if (map? rejection-info) (:code rejection-info) rejection-info) - hook-continue - (when (map? rejection-info) (:hook-continue rejection-info)) - hook-stop-reason - (when (map? rejection-info) (:hook-stop-reason rejection-info))] - (if (= :hook-rejected reason-code) - (if (false? hook-continue) - (do (lifecycle/send-content! chat-ctx :system {:type :text - :text (or hook-stop-reason "Tool rejected by hook")}) - (lifecycle/finish-chat-prompt! :idle chat-ctx) nil) + (if (map? blocked-info) (:code blocked-info) blocked-info) + stop-turn? + (when (map? blocked-info) (:stop-turn? blocked-info)) + stop-reason + (when (map? blocked-info) (:stop-reason blocked-info)) + stop-hook-name + (when (map? blocked-info) (:stop-hook-name blocked-info))] + (if (contains? #{:hook-rejected :hook-stopped} reason-code) + (if stop-turn? + ;; Turn stop: stopReason is user-only; tool_call_output stays neutral. + (do (lifecycle/send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx) + nil) (with-fresh-auth {:tools all-tools :new-messages (shared/messages-after-last-compact-marker @@ -896,9 +1025,17 @@ :text "I rejected one or more tool calls with the following reason"}]}) (lifecycle/finish-chat-prompt! :idle chat-ctx) nil)))) - (with-fresh-auth - (if-let [continue-fn (:continue-fn chat-ctx)] - (continue-fn all-tools user-messages) - {:tools all-tools - :new-messages (shared/messages-after-last-compact-marker - (get-in @db* [:chats chat-id :messages]))})))))))))) + (let [{:keys [stop-turn? stop-reason stop-hook-name]} (post-tool-call-stop-info + (get-in @db* [:chats chat-id :tool-calls] {}) + (:prompt-id chat-ctx))] + (if stop-turn? + (do + (lifecycle/send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason) + (lifecycle/finish-chat-prompt-stopped! :idle chat-ctx) + nil) + (with-fresh-auth + (if-let [continue-fn (:continue-fn chat-ctx)] + (continue-fn all-tools user-messages) + {:tools all-tools + :new-messages (shared/messages-after-last-compact-marker + (get-in @db* [:chats chat-id :messages]))})))))))))))) diff --git a/src/eca/features/commands.clj b/src/eca/features/commands.clj index 710ce8edd..e0f9e16bd 100644 --- a/src/eca/features/commands.clj +++ b/src/eca/features/commands.clj @@ -1,12 +1,14 @@ (ns eca.features.commands (:require [babashka.fs :as fs] + [cheshire.core :as json] [clojure.java.io :as io] [clojure.pprint :as pprint] [clojure.string :as string] [eca.config :as config] [eca.db :as db] [eca.features.chat.debug :as f.chat.debug] + [eca.features.chat.lifecycle :as lifecycle] [eca.features.index :as f.index] [eca.features.login :as f.login] [eca.features.plugins :as f.plugins] @@ -211,7 +213,11 @@ {:name "plugin-uninstall" :type :native :description "Uninstall a plugin (e.g. /plugin-uninstall my-plugin)" - :arguments [{:name "plugin" :description "Plugin name" :required true}]}] + :arguments [{:name "plugin" :description "Plugin name" :required true}]} + {:name "hooks" + :type :native + :description "List active hooks grouped by type." + :arguments []}] custom-cmds (map (fn [custom] {:name (:name custom) :type :custom-prompt @@ -287,6 +293,46 @@ "Subagents available:\n\n" subagents)))) +(def ^:private hook-type-order + "Display order for hook types in /hooks output." + ["sessionStart" "sessionEnd" "chatStart" "chatEnd" "subagentStart" "preRequest" + "postRequest" "subagentPostRequest" "preToolCall" "postToolCall" + "preCompact" "postCompact"]) + +(defn ^:private hooks-msg [config] + (let [hooks-map (:hooks config) + hooks-by-type (->> hooks-map + (keep (fn [[hook-name hook]] + (when (seq (:actions hook)) + {:name (name hook-name) + :type (:type hook) + :description (:description hook) + :visible (:visible hook true) + :matcher (:matcher hook)}))) + (group-by :type)) + known (set hook-type-order) + display-order (concat hook-type-order (sort (remove known (keys hooks-by-type))))] + (if (empty? hooks-by-type) + "No hooks configured. Add hooks to your config under the `hooks` key." + (reduce + (fn [s hook-type] + (if-let [hooks (seq (get hooks-by-type hook-type))] + (str s "**" hook-type "**\n" + (reduce + (fn [s2 {:keys [name description visible matcher]}] + (str s2 "- `" name "`" + (when description (str " — " description)) + (when-not visible " *(hidden)*") + (when (some? matcher) + (str " matcher: `" (if (string? matcher) matcher (json/generate-string matcher)) "`")) + "\n")) + "" + (sort-by :name hooks)) + "\n") + s)) + "Hooks active:\n\n" + display-order)))) + (defn ^:private format-expires-at [^long expires-at] (let [instant (Instant/ofEpochSecond expires-at) formatter (.withZone (DateTimeFormatter/ofPattern "yyyy-MM-dd HH:mm") (ZoneId/systemDefault))] @@ -399,12 +445,24 @@ :on-finished-side-effect (fn [] (swap! db* assoc-in [:chats chat-id :messages] [])) :prompt (f.prompt/init-prompt all-tools agent db config)} - "compact" (do - (swap! db* assoc-in [:chats chat-id :compacting?] true) - {:type :send-prompt - :on-finished-side-effect (fn [] - (shared/compact-side-effect! chat-ctx false)) - :prompt (f.prompt/compact-prompt (string/join " " args) all-tools agent config db)}) + "compact" (let [custom-instructions (string/join " " args) + {:keys [blocked? reason hook-name stop-turn?]} (lifecycle/run-pre-compact-hooks! chat-ctx "manual" custom-instructions)] + (if blocked? + (do + ;; continue:false stops the turn; exit 2 blocks compaction only. + (if stop-turn? + (lifecycle/send-turn-stopped-by-hook! chat-ctx hook-name reason) + (lifecycle/send-content! chat-ctx :system {:type :text :text (lifecycle/compaction-blocked-by-hook-message hook-name)})) + {:type :new-chat-status :status :idle}) + (do + (swap! db* assoc-in [:chats chat-id :compacting?] true) + {:type :send-prompt + :on-finished-side-effect #(let [{:keys [stop-turn? stop-reason stop-hook-name]} + (lifecycle/complete-compact! chat-ctx "manual")] + (when stop-turn? + (lifecycle/send-turn-stopped-by-hook! chat-ctx stop-hook-name stop-reason)) + {:stop-after-finish? stop-turn?}) + :prompt (f.prompt/compact-prompt custom-instructions all-tools agent config db)}))) "login" (do (messenger/chat-content-received messenger @@ -424,9 +482,6 @@ {:type :new-chat-status :status :login}) "model" (let [selected-model (first args) - current-model (or (get-in db [:chats chat-id :model]) - full-model - (llm-api/default-model db config)) available-models (sort (keys (:models db))) chat-message (fn [text] {:type :chat-messages @@ -437,7 +492,7 @@ (string/blank? selected-model) (if (seq available-models) (chat-message - (multi-str (str "Current model: `" current-model "`") + (multi-str (str "Current model: `" full-model "`") "" "Available models:" (string/join "\n" (map #(str "- `" % "`") available-models)) @@ -689,6 +744,9 @@ (f.plugins/uninstall-plugin! (:plugins config) plugin-input))] {:type :chat-messages :chats {chat-id {:messages [{:role "system" :content [{:type :text :text (:message result)}]}]}}}) + "hooks" (let [msg (hooks-msg config)] + {:type :chat-messages + :chats {chat-id {:messages [{:role "system" :content [{:type :text :text msg}]}]}}}) ;; else check if a custom command or skill (if-let [custom-command-prompt (get-custom-command command args custom-cmds)] diff --git a/src/eca/features/hooks.clj b/src/eca/features/hooks.clj index 705cee231..16808c970 100644 --- a/src/eca/features/hooks.clj +++ b/src/eca/features/hooks.clj @@ -8,28 +8,40 @@ [eca.logger :as logger] [eca.shared :as shared])) +(set! *warn-on-reflection* true) + (def ^:private logger-tag "[HOOK]") (def ^:const hook-rejection-exit-code 2) (def ^:const default-hook-timeout-ms 30000) +(defn ^:private session-id-from-db-cache-path + [db-cache-path] + (some-> db-cache-path fs/file fs/parent fs/file-name str not-empty)) + (defn base-hook-data - "Returns common fields for ALL hooks (session and chat hooks). - These fields are present in every hook type." + "Returns common fields for ALL hooks: :workspaces, :cwd, :db-cache-path, + :session-id, and :eca-executable." [db] - {:workspaces (shared/get-workspaces db) - :db-cache-path (shared/db-cache-path db)}) + (let [workspaces (shared/get-workspaces db) + db-cache-path (shared/db-cache-path db)] + {:workspaces workspaces + :cwd (first workspaces) + :db-cache-path db-cache-path + :session-id (session-id-from-db-cache-path db-cache-path) + :eca-executable @shared/eca-executable*})) (defn chat-hook-data "Returns common fields for CHAT-RELATED hooks. - Includes base fields plus chat-specific fields (chat-id, agent). - Use this for: preRequest, postRequest, subagentPostRequest, preToolCall, postToolCall, chatStart, chatEnd." - [db chat-id agent-name] + Merges base-hook-data, optional model fields (:full-model, :variant), + and chat-specific fields (:chat-id, :agent, :behavior)." + [db {:keys [chat-id agent full-model variant]}] (merge (base-hook-data db) - {:chat-id chat-id - :agent agent-name - :behavior agent-name})) + (shared/assoc-some {} :full-model full-model :variant variant) + {:chat-id chat-id + :agent agent + :behavior agent})) (defn ^:private parse-hook-json "Attempts to parse hook output as JSON. Returns parsed map if successful, nil otherwise." @@ -86,14 +98,21 @@ (->> matcher (keep (fn [[tool-selector config]] (if (map? config) - {:kind :selector - :selector (tools.util/selector->string tool-selector) - :args-matchers (:argsMatchers config)} + (let [has-args-matchers? (contains? config :argsMatchers) + am (:argsMatchers config)] + (if (and has-args-matchers? (not (map? am))) + (do + (logger/warn logger-tag "argsMatchers must be a map, ignoring matcher entry" {:selector tool-selector}) + nil) + {:kind :selector + :selector (tools.util/selector->string tool-selector) + :args-matchers am})) (logger/warn logger-tag "Ignoring non-map matcher entry for selector" {:selector tool-selector})))) vec) :else - [])) + (do (logger/warn logger-tag "Unsupported matcher type, ignoring" {:matcher matcher}) + []))) (defn ^:private arg-value [tool-input arg-name] (when (map? tool-input) @@ -122,10 +141,15 @@ :else (every? (fn [[arg-name matchers]] - (let [value (arg-value tool-input arg-name)] - (and (some? value) - (sequential? matchers) - (some #(regex-matches? % value) matchers)))) + (cond + (not (sequential? matchers)) + (do (logger/warn logger-tag "Arg matcher values must be sequential (list of regex alternatives), skipping" + {:arg-name arg-name :value matchers}) + false) + :else + (let [value (arg-value tool-input arg-name)] + (and (some? value) + (some #(regex-matches? % value) matchers))))) args-matchers))) (defn ^:private hook-matches? [hook-type data hook native-tools] @@ -150,6 +174,17 @@ false)) rules)) + (contains? #{:preCompact :postCompact} hook-type) + (let [matcher (:matcher hook)] + (cond + (nil? matcher) true + (string? matcher) (= matcher (:triggered data)) + :else (do + (logger/warn logger-tag "Ignoring unsupported compact hook matcher" + {:hook-type hook-type + :matcher matcher}) + false))) + :else true))) @@ -168,8 +203,10 @@ "Execute a single hook action. Supported hook types: - :sessionStart, :sessionEnd (session lifecycle) - :chatStart, :chatEnd (chat lifecycle) - - :preRequest, :postRequest, :subagentPostRequest (prompt lifecycle) + - :subagentStart, :subagentPostRequest (subagent lifecycle) + - :preRequest, :postRequest (prompt lifecycle) - :preToolCall, :postToolCall (tool lifecycle) + - :preCompact, :postCompact (compact lifecycle) Returns map with :exit, :raw-output, :raw-error, :parsed" [action name hook-type data db] @@ -204,8 +241,53 @@ (logger/warn logger-tag (format "Unknown hook action type '%s' for %s" (:type action) name)))) +(defn successful-continue-false? + "True when a hook action result is a successful (exit 0) continue:false. + This is the canonical 'turn stop' signal: JSON effects (including + continue:false) are only honored on exit 0, so a non-zero exit never counts + here even if its stdout contained continue:false." + [{:keys [exit parsed]}] + (and (zero? exit) + (false? (get parsed "continue" true)))) + +(defn ^:private stop-processing? + "Predicate over an action result: should iteration of remaining + actions/hooks halt? True when a successful action (exit 0) returns + continue:false." + [result] + (successful-continue-false? result)) + +(defn ^:private run-matched-action! + "Execute a single matched action: notify before, run, notify after. + Returns the merged action result so the caller can inspect it without + conflating 'what happened' with 'what to do next'." + [hook-type data {:keys [on-before-action on-after-action]} db hook-name hook i action] + (let [id (str (random-uuid)) + action-type (:type action) + action-name (if (> (count (:actions hook)) 1) + (str hook-name "-" (inc i)) + hook-name) + visible? (get hook :visible true) + base {:id id + :name action-name + :type action-type + :hook-type hook-type + :visible? visible?}] + (on-before-action (select-keys base [:id :visible? :name :type])) + (let [result (merge (or (run-hook-action! action action-name hook-type data db) + {:exit 1}) + base)] + (on-after-action result) + result))) + (defn trigger-if-matches! - "Run hook of specified type if matches any config for that type" + "Run matching hooks of `hook-type` against `data` for side effects. + Execution order is deterministic: hooks sorted by name, then actions + in declaration order. A successful action that returns continue:false + halts iteration over BOTH the remaining actions of the current hook + AND any subsequent matching hooks. Exit code 2 + carries hook-specific semantics via the per-action callbacks but does + NOT short-circuit iteration." [hook-type data {:keys [on-before-action on-after-action native-tools] @@ -213,29 +295,15 @@ on-after-action identity}} db config] - ;; Sort hooks by name to ensure deterministic execution order. - (doseq [[name hook] (sort-by key (:hooks config))] - (when (hook-matches? hook-type data hook native-tools) - (vec - (map-indexed (fn [i action] - (let [id (str (random-uuid)) - action-type (:type action) - name (if (> (count (:actions hook)) 1) - (str name "-" (inc i)) - name) - visible? (get hook :visible true)] - (on-before-action {:id id - :visible? visible? - :name name}) - (if-let [result (run-hook-action! action name hook-type data db)] - (on-after-action (merge result - {:id id - :name name - :type action-type - :visible? visible?})) - (on-after-action {:id id - :name name - :visible? visible? - :type action-type - :exit 1})))) - (:actions hook)))))) + (let [opts {:on-before-action on-before-action + :on-after-action on-after-action} + matched-actions (for [[hook-key hook] (sort-by key (:hooks config)) + :when (hook-matches? hook-type data hook native-tools) + [i action] (map-indexed vector (:actions hook))] + [(name hook-key) hook i action])] + (reduce (fn [_ [name hook i action]] + (let [result (run-matched-action! hook-type data opts db name hook i action)] + (when (stop-processing? result) + (reduced nil)))) + nil + matched-actions))) diff --git a/src/eca/features/plugins.clj b/src/eca/features/plugins.clj index fda525bc4..ccbc6c879 100644 --- a/src/eca/features/plugins.clj +++ b/src/eca/features/plugins.clj @@ -201,15 +201,23 @@ ;; -- Component readers -- +(defn ^:private plugin-hook-key + [plugin-name hook-key] + (str plugin-name "::" (name hook-key))) + (defn ^:private read-hooks "Reads hooks/hooks.json from a plugin directory. Expects ECA native hook format. - Applies dynamic string interpolation after JSON parsing." - [^java.io.File plugin-dir] + Applies dynamic string interpolation after JSON parsing. + Prefixes hook map keys with 'plugin-name::' so plugin hooks are identifiable." + [^java.io.File plugin-dir plugin-name] (let [hooks-file (io/file plugin-dir "hooks" "hooks.json")] (when (fs/exists? hooks-file) (try - (-> (json/parse-string (slurp hooks-file) true) - (interpolation/replace-dynamic-strings-in-data (str (fs/parent hooks-file)) nil)) + (let [parsed (-> (json/parse-string (slurp hooks-file) true) + (interpolation/replace-dynamic-strings-in-data (str (fs/parent hooks-file)) nil))] + (if plugin-name + (update-keys parsed #(plugin-hook-key plugin-name %)) + parsed)) (catch Exception e (logger/warn logger-tag "Failed to parse hooks.json:" (str hooks-file) (.getMessage e)) @@ -307,7 +315,7 @@ (discover-components plugin-dir nil)) ([^java.io.File plugin-dir plugin-name] (let [mcp-servers (read-mcp-servers plugin-dir) - hooks (read-hooks plugin-dir) + hooks (read-hooks plugin-dir plugin-name) eca-config (read-eca-config plugin-dir) skill-dirs (read-skill-dirs plugin-dir plugin-name) config-fragment (cond-> (or eca-config {}) diff --git a/src/eca/features/prompt.clj b/src/eca/features/prompt.clj index 30084103c..dbc5eef6e 100644 --- a/src/eca/features/prompt.clj +++ b/src/eca/features/prompt.clj @@ -90,7 +90,7 @@ "" refined-contexts) (when startup-ctx - (str "\n\n" startup-ctx "\n\n\n")) + (str "\n\n" startup-ctx "\n\n\n")) "")) (defn ->base-selmer-ctx @@ -241,10 +241,11 @@ (shared/safe-selmer-render (load-builtin-prompt "additional_system_info.md") selmer-ctx "additional-system-info") "" - (when (seq stable-contexts) - ["## Static Contexts" - "" - (contexts-str stable-contexts repo-map* (get-in db [:chats chat-id :startup-context]))])))) + (let [startup-ctx (get-in db [:chats chat-id :startup-context])] + (when (or (seq stable-contexts) (not (string/blank? startup-ctx))) + ["## Static Contexts" + "" + (contexts-str stable-contexts repo-map* startup-ctx)]))))) (defn build-dynamic-instructions "Builds the volatile portion of the system prompt: cursor/MCP resource contexts diff --git a/src/eca/features/tools/shell.clj b/src/eca/features/tools/shell.clj index 3a96c85ac..1e9e22658 100644 --- a/src/eca/features/tools/shell.clj +++ b/src/eca/features/tools/shell.clj @@ -59,7 +59,8 @@ :dir cwd :out out-mode :err out-mode - :continue true} + :continue true + :extra-env {"ECA_EXECUTABLE" @shared/eca-executable*}} input (assoc :in input))))) (def ^:private initial-output-wait-ms 2000) @@ -167,52 +168,52 @@ work-dir (resolve-work-dir db user-work-dir) _ (logger/debug logger-tag "Running command:" command-args) result (try - (if-let [proc (when-not (= :stopping (:status (call-state-fn))) - (start-shell-process! (cond-> {:cwd work-dir - :script command-args} - shell-path (assoc :shell-path shell-path) - shell-args (assoc :shell-args shell-args))))] - (do - (state-transition-fn :resources-created {:resources {:process proc}}) - (try (deref proc - timeout - ::timeout) - (catch InterruptedException e - (let [msg (or (.getMessage e) "Shell tool call was interrupted")] - (logger/debug logger-tag "Shell tool call was interrupted" {:tool-call-id tool-call-id :message msg}) - (tools.util/tool-call-destroy-resource! "eca__shell_command" :process proc) - (state-transition-fn :resources-destroyed {:resources [:process]}) - {:exit 1 :err msg})))) - {:exit 1 :err "Tool call is :stopping, so shell process not spawned"}) - (catch Exception e - (let [msg (or (.getMessage e) "Caught an Exception during execution of the shell tool")] - (logger/warn logger-tag "Got an Exception during execution" {:message msg}) - {:exit 1 :err msg})) - (finally - (let [state (call-state-fn)] - (when-let [resources (:resources state)] - (doseq [[res-kwd res] resources] - (tools.util/tool-call-destroy-resource! "eca__shell_command" res-kwd res)) - (when (#{:executing :stopping} (:status state)) - (state-transition-fn :resources-destroyed {:resources (keys resources)})))))) - err (some-> (:err result) string/trim) - out (some-> (:out result) string/trim)] - (if (= result ::timeout) - (do - (logger/debug logger-tag "Command timed out after " timeout " ms") - (tools.util/single-text-content (str "Command timed out after " timeout " ms") true)) - (do - (logger/debug logger-tag "Command executed:" result) - {:error (not (zero? (:exit result))) - :contents (remove nil? - (concat [{:type :text - :text (str "Exit code: " (:exit result))}] - (when-not (string/blank? err) - [{:type :text - :text (str "Stderr:\n" err)}]) - (when-not (string/blank? out) - [{:type :text - :text (str "Stdout:\n" out)}])))})))) + (if-let [proc (when-not (= :stopping (:status (call-state-fn))) + (start-shell-process! (cond-> {:cwd work-dir + :script command-args} + shell-path (assoc :shell-path shell-path) + shell-args (assoc :shell-args shell-args))))] + (do + (state-transition-fn :resources-created {:resources {:process proc}}) + (try (deref proc + timeout + ::timeout) + (catch InterruptedException e + (let [msg (or (.getMessage e) "Shell tool call was interrupted")] + (logger/debug logger-tag "Shell tool call was interrupted" {:tool-call-id tool-call-id :message msg}) + (tools.util/tool-call-destroy-resource! "eca__shell_command" :process proc) + (state-transition-fn :resources-destroyed {:resources [:process]}) + {:exit 1 :err msg})))) + {:exit 1 :err "Tool call is :stopping, so shell process not spawned"}) + (catch Exception e + (let [msg (or (.getMessage e) "Caught an Exception during execution of the shell tool")] + (logger/warn logger-tag "Got an Exception during execution" {:message msg}) + {:exit 1 :err msg})) + (finally + (let [state (call-state-fn)] + (when-let [resources (:resources state)] + (doseq [[res-kwd res] resources] + (tools.util/tool-call-destroy-resource! "eca__shell_command" res-kwd res)) + (when (#{:executing :stopping} (:status state)) + (state-transition-fn :resources-destroyed {:resources (keys resources)})))))) + err (some-> (:err result) string/trim) + out (some-> (:out result) string/trim)] + (if (= result ::timeout) + (do + (logger/debug logger-tag "Command timed out after " timeout " ms") + (tools.util/single-text-content (str "Command timed out after " timeout " ms") true)) + (do + (logger/debug logger-tag "Command executed:" result) + {:error (not (zero? (:exit result))) + :contents (remove nil? + (concat [{:type :text + :text (str "Exit code: " (:exit result))}] + (when-not (string/blank? err) + [{:type :text + :text (str "Stderr:\n" err)}]) + (when-not (string/blank? out) + [{:type :text + :text (str "Stdout:\n" out)}])))})))) (defn ^:private shell-command [arguments ctx] (or (tools.util/invalid-arguments arguments [["working_directory" #(or (nil? %) @@ -268,4 +269,4 @@ (case resource-kwd :process (p/destroy-tree resource) (logger/warn logger-tag "Unknown resource keyword" {:tool-name name - :resource-kwd resource-kwd}))) \ No newline at end of file + :resource-kwd resource-kwd}))) diff --git a/src/eca/features/tools/util.clj b/src/eca/features/tools/util.clj index 34067fba7..88d25607e 100644 --- a/src/eca/features/tools/util.clj +++ b/src/eca/features/tools/util.clj @@ -172,21 +172,22 @@ (not (contains? required (name k)))))) args))) -(defn ^:private contents->text +(defn contents->text "Concatenates all text contents from a tool result's :contents into a single string." [contents] - (reduce - (fn [^StringBuilder sb content] - (case (:type content) - :text (doto sb - (.append ^String (:text content)) - (.append "\n")) - :image (doto sb - (.append (format "[Image: %s]" (:media-type content))) - (.append "\n")) - sb)) - (StringBuilder.) - contents)) + (->> contents + (reduce + (fn [^StringBuilder sb content] + (case (:type content) + :text (doto sb + (.append ^String (:text content)) + (.append "\n")) + :image (doto sb + (.append (format "[Image: %s]" (:media-type content))) + (.append "\n")) + sb)) + (StringBuilder.)) + str)) (defn ^:private exceeds-truncation-limits? "Returns true if the text exceeds either the line limit or size limit." @@ -241,7 +242,7 @@ max-size-kb (get-in config [:toolCall :outputTruncation :sizeKb])] (if (or (nil? max-lines) (nil? max-size-kb) (:error result)) result - (let [full-text (str (contents->text (:contents result)))] + (let [full-text (contents->text (:contents result))] (if (exceeds-truncation-limits? full-text max-lines max-size-kb) (let [saved-path (cache/save-tool-call-output! tool-call-id full-text) truncated (truncate-text full-text max-lines max-size-kb) diff --git a/src/eca/shared.clj b/src/eca/shared.clj index bc68761f9..ede182b29 100644 --- a/src/eca/shared.clj +++ b/src/eca/shared.clj @@ -11,6 +11,7 @@ [eca.messenger :as messenger] [selmer.parser :as selmer]) (:import + [java.lang ProcessHandle] [java.net URI] [java.nio.file Paths] [java.time Instant ZoneId ZoneOffset] @@ -31,6 +32,22 @@ :else x)) +(def eca-executable* + "Absolute path and JVM flags used to launch this ECA process. + For a native binary this is just the executable path. + For a JVM launch it includes the java command, JVM flags, and -jar path. + Delayed so native-image builds do not capture build-time JVM process info." + (delay + (let [info (.info (ProcessHandle/current)) + command (.orElse (.command info) nil) + cmd-line (.orElse (.commandLine info) nil)] + (or (when cmd-line + (let [parts (string/split cmd-line #"\s+") + jar-idx (first (keep-indexed #(when (= "-jar" %2) %1) parts))] + (when jar-idx + (string/join " " (take (+ jar-idx 2) parts))))) + command)))) + (defn parse-md "Parses YAML frontmatter and body from a markdown string. Frontmatter must be delimited by --- at the start and end. @@ -513,3 +530,7 @@ (catch Exception e (logger/warn "[SELMER]" (format "Failed to render template '%s': %s" label (ex-message e))) fallback)))) + +(defn not-blank [s] + (when (and (string? s) (not (string/blank? s))) + s)) diff --git a/test/eca/features/chat/lifecycle_test.clj b/test/eca/features/chat/lifecycle_test.clj index b82bfd75e..c6bc6906d 100644 --- a/test/eca/features/chat/lifecycle_test.clj +++ b/test/eca/features/chat/lifecycle_test.clj @@ -1,7 +1,8 @@ (ns eca.features.chat.lifecycle-test (:require - [clojure.test :refer [deftest is testing]] - [eca.features.chat.lifecycle :as lifecycle])) + [clojure.test :refer [are deftest is testing]] + [eca.features.chat.lifecycle :as lifecycle] + [matcher-combinators.test :refer [match?]])) (set! *warn-on-reflection* true) @@ -65,3 +66,157 @@ {:limit {:context 1000 :output 1000}}) (assoc-in [:chats chat-id :auto-compacting?] true))] (is (nil? (lifecycle/auto-compact? chat-id agent-name full-model config db)))))) + +(deftest format-hook-output-test + ;; [hook-type parsed raw-output => expected] + (are [hook-type parsed raw-output expected] + (= expected (#'lifecycle/format-hook-output hook-type parsed raw-output)) + ;; postToolCall replacedOutput is surfaced for visible hooks + :postToolCall {"replacedOutput" "redacted"} nil "Hook executed\nReplacedOutput: \"redacted\"" + ;; replacedOutput is ignored for non-postToolCall hook types + :postRequest {"replacedOutput" "redacted"} nil "Hook executed" + ;; preToolCall updatedInput is surfaced as a structural effect line + :preToolCall {"updatedInput" {"recursive" true}} nil "Hook executed\nUpdatedInput: {\"recursive\" true}" + ;; updatedInput is ignored for non-preToolCall hook types + :postToolCall {"updatedInput" {"recursive" true}} nil "Hook executed" + ;; non-string replacedOutput is not rendered + :postToolCall {"replacedOutput" 42} nil "Hook executed" + ;; blank additive fields are not rendered as structural effect lines + :preRequest {"replacedPrompt" " " "additionalContext" ""} nil "Hook executed" + :postRequest {"followUp" ""} nil "Hook executed" + ;; empty replacedOutput remains a meaningful explicit replacement + :postToolCall {"replacedOutput" ""} nil "Hook executed\nReplacedOutput: \"\"" + ;; systemMessage is NOT part of the block text (surfaced standalone instead) + :postToolCall {"systemMessage" "all good" "additionalContext" "extra"} nil "Hook executed\nAdditionalContext: extra" + ;; falls back to raw-output when there is no parsed JSON + :postToolCall nil "raw text" "raw text" + ;; no parsed JSON and no raw-output yields nil (no block body) + :postToolCall nil nil nil)) + +(deftest hook-system-text-test + (testing "prefixes the hook name and ends with a blank line so it never glues to a following stream" + (is (= "Hook 'my-hook': all good\n\n" + (lifecycle/hook-system-text "my-hook" "all good"))))) + +(deftest turn-stopped-by-hook-message-test + (testing "blank stopReason uses the generic message" + (is (= "Turn stopped by hook 'guard'." + (lifecycle/turn-stopped-by-hook-message "guard" ""))) + (is (= "Turn stopped by hook 'guard'." + (lifecycle/turn-stopped-by-hook-message "guard" " "))))) + +(deftest compaction-blocked-by-hook-message-test + (testing "names the blocking hook when known" + (is (= "Compaction blocked by hook 'guard'." + (lifecycle/compaction-blocked-by-hook-message "guard")))) + + (testing "falls back to the generic message when the hook name is unknown" + (is (= "Compaction blocked by hook." + (lifecycle/compaction-blocked-by-hook-message nil))) + (is (= "Compaction blocked by hook." + (lifecycle/compaction-blocked-by-hook-message ""))))) + +(deftest process-pre-compact-hook-result-test + (testing "exit 2 blocks compaction and remembers the first blocking hook name (no stderr reason)" + (let [acc {:blocked? false :reason nil :hook-name nil :stop-turn? false} + result {:exit 2 :name "first" :raw-error "stderr should not be a reason"}] + (is (match? {:blocked? true + :hook-name "first" + :reason nil + :stop-turn? false} + (#'lifecycle/process-pre-compact-hook-result acc result))))) + + (testing "exit 2 keeps the first hook name when a second hook also blocks" + (let [acc {:blocked? true :reason nil :hook-name "first" :stop-turn? false} + result {:exit 2 :name "second"}] + (is (= "first" (:hook-name (#'lifecycle/process-pre-compact-hook-result acc result)))))) + + (testing "successful continue:false blocks, stops the turn and records its hook name" + (let [acc {:blocked? false :reason nil :hook-name nil :stop-turn? false} + result {:exit 0 :name "stopper" :parsed {"continue" false "stopReason" "no compacting now"}}] + (is (match? {:blocked? true + :hook-name "stopper" + :reason "no compacting now" + :stop-turn? true} + (#'lifecycle/process-pre-compact-hook-result acc result)))))) + +(defn- dispatch-ctx + "Build a chat-ctx for dispatch-finish-callbacks! whose callbacks record their + invocations into `calls*` (a vector atom). Extra keys override defaults." + [calls* extra] + (merge {:chat-id chat-id + :db* (atom {:chats {chat-id {:auto-compacting? true :compacting? true}}}) + :on-finished-side-effect (fn [] (swap! calls* conj :side-effect) nil) + :on-after-finish! (fn [] (swap! calls* conj :after-finish)) + :on-follow-up (fn [text _ctx] (swap! calls* conj [:follow-up text]))} + extra)) + +(deftest dispatch-finish-callbacks!-test + (testing "on-finished-side-effect always runs, then on-after-finish! in the default path" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {}) {}) + (is (= [:side-effect :after-finish] @calls*)))) + + (testing "stopping? halts continuations but the side-effect still runs" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {}) {:stopping? true}) + (is (= [:side-effect] @calls*)))) + + (testing "stop-turn? halts continuations but the side-effect still runs" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {}) {:stop-turn? true}) + (is (= [:side-effect] @calls*)))) + + (testing "follow-up text fires on-follow-up (not on-after-finish!) and flags the chat" + (let [calls* (atom []) + ctx (dispatch-ctx calls* {})] + (#'lifecycle/dispatch-finish-callbacks! ctx {:follow-up-text "do more"}) + (is (= [:side-effect [:follow-up "do more"]] @calls*)) + (is (true? (get-in @(:db* ctx) [:chats chat-id :follow-up-active?]))) + (is (nil? (get-in @(:db* ctx) [:chats chat-id :auto-compacting?]))) + (is (nil? (get-in @(:db* ctx) [:chats chat-id :compacting?]))))) + + (testing "stop-turn? takes precedence over follow-up text" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {}) + {:follow-up-text "do more" :stop-turn? true}) + (is (= [:side-effect] @calls*)))) + + (testing "follow-up text with no on-follow-up callback fires nothing else (no fallback to on-after-finish!)" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {:on-follow-up nil}) + {:follow-up-text "do more"}) + (is (= [:side-effect] @calls*)))) + + (testing "stop-after-finish? from opts suppresses on-after-finish!" + (let [calls* (atom [])] + (#'lifecycle/dispatch-finish-callbacks! (dispatch-ctx calls* {}) {:stop-after-finish? true}) + (is (= [:side-effect] @calls*)))) + + (testing "stop-after-finish? requested by the side-effect result suppresses on-after-finish!" + (let [calls* (atom []) + ctx (dispatch-ctx calls* {:on-finished-side-effect + (fn [] (swap! calls* conj :side-effect) {:stop-after-finish? true})})] + (#'lifecycle/dispatch-finish-callbacks! ctx {}) + (is (= [:side-effect] @calls*))))) + +(deftest finish-chat-prompt-stopped!-test + (testing "skips postRequest hooks and strips finish side-effect callbacks (keeps on-follow-up)" + (let [captured* (atom nil)] + (with-redefs [lifecycle/finish-chat-prompt! (fn [status ctx] + (reset! captured* {:status status :ctx ctx}))] + (lifecycle/finish-chat-prompt-stopped! + :idle + {:chat-id "c1" + :skip-post-request-hooks? false + :on-finished-side-effect (fn []) + :on-after-finish! (fn []) + :on-follow-up (fn [_ _])})) + (is (= :idle (:status @captured*))) + ;; postRequest hooks must not run for a hook-stopped turn + (is (true? (get-in @captured* [:ctx :skip-post-request-hooks?]))) + ;; finish side-effect callbacks are stripped so no continuation fires + (is (not (contains? (:ctx @captured*) :on-finished-side-effect))) + (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))))) diff --git a/test/eca/features/chat/tool_calls_test.clj b/test/eca/features/chat/tool_calls_test.clj index 61deb405f..f6a32aa68 100644 --- a/test/eca/features/chat/tool_calls_test.clj +++ b/test/eca/features/chat/tool_calls_test.clj @@ -1,6 +1,7 @@ (ns eca.features.chat.tool-calls-test (:require - [clojure.test :refer [deftest is testing]] + [clojure.string :as string] + [clojure.test :refer [are deftest is testing]] [eca.features.chat.lifecycle :as lifecycle] [eca.features.chat.tool-calls :as tc] [eca.features.hooks :as f.hooks] @@ -30,7 +31,7 @@ (is (match? {:decision :allow :arguments {:foo "bar"} :approval-override nil - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? false :reason {:code :user-config-allow :text string?}} @@ -55,7 +56,7 @@ (is (match? {:decision :ask :arguments {:foo "bar"} :approval-override nil - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? false} plan)))))) @@ -78,7 +79,7 @@ (is (match? {:decision :deny :arguments {:foo "bar"} :approval-override nil - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? false :reason {:code :user-config-deny :text string?}} @@ -94,7 +95,7 @@ :origin :native :server {:name "eca"}}] db (h/db) - config (h/config! {:hooks {"hook1" {:event "preToolCall" + config (h/config! {:hooks {"hook1" {:type "preToolCall" :actions [{:type "shell" :command "echo 'approval override'"}]}}}) agent-name :default chat-id "test-chat" @@ -111,10 +112,53 @@ (is (match? {:decision :ask :arguments {:foo "bar"} :approval-override "ask" - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? false} plan)))))) + (testing "hook approval precedence (deny > ask > allow; hook-allow never overrides config ask/deny)" + (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 :native + :server {:name "eca"}}] + config (h/config! {:hooks {"hook1" {:type "preToolCall" + :actions [{:type "shell" :command "echo"}]}}}) + run-plan (fn [config-approval hook-approvals] + (with-redefs [f.tools/approval (constantly config-approval) + f.hooks/trigger-if-matches! (fn [event _ callbacks _ _] + (when (= event :preToolCall) + (when-let [on-after (:on-after-action callbacks)] + (doseq [approval hook-approvals] + (on-after {:parsed {"approval" approval} + :exit 0 + :raw-error nil})))))] + (#'tc/decide-tool-call-action tool-call all-tools (h/db) config :default "test-chat")))] + ;; deny wins over ask and allow + (is (match? {:decision :deny + :approval-override "deny" + :tool-call-rejected-by-hook? true + :reason {:code :hook-rejected :text "Tool call rejected by hook"}} + (run-plan :allow ["allow" "deny" "ask"]))) + ;; ask wins over allow + (is (match? {:decision :ask + :approval-override "ask" + :tool-call-rejected-by-hook? false} + (run-plan :allow ["allow" "ask"]))) + ;; a hook "allow" never loosens a stricter config decision (ask/deny) + (is (match? {:decision :ask + :approval-override "allow" + :tool-call-rejected-by-hook? false} + (run-plan :ask ["allow"]))) + (is (match? {:decision :deny + :approval-override "allow" + :tool-call-rejected-by-hook? false + :reason {:code :user-config-deny}} + (run-plan :deny ["allow"]))))) + (testing "hook rejection via exit code 2" (h/reset-components!) (let [tool-call {:id "call-1" @@ -125,7 +169,7 @@ :origin :native :server {:name "eca"}}] db (h/db) - config (h/config! {:hooks {"hook1" {:event "preToolCall" + config (h/config! {:hooks {"hook1" {:type "preToolCall" :actions [{:type "shell" :command "exit 2"}]}}}) agent-name :default chat-id "test-chat"] @@ -140,13 +184,44 @@ (is (match? {:decision :deny :arguments {:foo "bar"} :approval-override nil - :hook-rejected? true + :tool-call-rejected-by-hook? true :arguments-modified? false :reason {:code :hook-rejected - :text "Hook rejected"} - :hook-continue true - :hook-stop-reason nil} - plan)))))) + :text "Command failed"} + :stop-turn? false} + plan)) + (is (not (contains? plan :stop-reason))))))) + + (testing "hook approval deny does not include stop-reason" + (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 :native + :server {:name "eca"}}] + db (h/db) + config (h/config! {:hooks {"hook1" {:type "preToolCall" + :actions [{:type "shell" :command "echo"}]}}}) + agent-name :default + chat-id "test-chat"] + (with-redefs [f.tools/approval (constantly :allow) + f.hooks/trigger-if-matches! (fn [event _ callbacks _ _] + (when (= event :preToolCall) + (when-let [on-after (:on-after-action callbacks)] + (on-after {:parsed {"approval" "deny" + "stopReason" "user-only"} + :exit 0 + :raw-error nil}))))] + (let [plan (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id)] + (is (match? {:decision :deny + :tool-call-rejected-by-hook? true + :stop-turn? false + :reason {:code :hook-rejected + :text "Tool call rejected by hook"}} + plan)) + (is (not (contains? plan :stop-reason))))))) (testing "trust mode converts ask to allow" (h/reset-components!) @@ -169,7 +244,7 @@ {:trust true})] (is (match? {:decision :allow :arguments {:foo "bar"} - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? false :reason {:code :trust-allow :text string?}} @@ -207,7 +282,7 @@ :origin :native :server {:name "eca"}}] db (h/db) - config (h/config! {:hooks {"hook1" {:event "preToolCall" + config (h/config! {:hooks {"hook1" {:type "preToolCall" :actions [{:type "shell" :command "echo ok"}]}}}) agent-name :default chat-id "test-chat" @@ -232,7 +307,7 @@ :origin :native :server {:name "eca"}}] db (h/db) - config (h/config! {:hooks {"hook1" {:event "preToolCall" + config (h/config! {:hooks {"hook1" {:type "preToolCall" :actions [{:type "shell" :command "exit 2"}]}}}) agent-name :default chat-id "test-chat"] @@ -246,9 +321,11 @@ (let [plan (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id {:trust true})] (is (match? {:decision :deny - :hook-rejected? true - :reason {:code :hook-rejected}} - plan)))))) + :tool-call-rejected-by-hook? true + :reason {:code :hook-rejected + :text "Command failed"}} + plan)) + (is (not (contains? plan :stop-reason))))))) (testing "hook modifies arguments" (h/reset-components!) @@ -260,7 +337,7 @@ :origin :native :server {:name "eca"}}] db (h/db) - config (h/config! {:hooks {"hook1" {:event "preToolCall" + config (h/config! {:hooks {"hook1" {:type "preToolCall" :actions [{:type "shell" :command "echo 'modify args'"}]}}}) agent-name :default chat-id "test-chat"] @@ -274,10 +351,37 @@ (is (match? {:decision :allow :arguments {"foo" "bar" "baz" "qux"} :approval-override nil - :hook-rejected? false + :tool-call-rejected-by-hook? false :arguments-modified? true} plan)))))) + (testing "preToolCall notifies UI with tool-call-id for correlation" + (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 :native + :server {:name "eca"}}] + db (h/db) + config (h/config! {:hooks {"hook1" {:type "preToolCall" + :actions [{:type "shell" :command "echo ok"}]}}}) + agent-name :default + chat-id "test-chat" + notified* (atom nil)] + (with-redefs [f.tools/approval (constantly :allow) + f.hooks/trigger-if-matches! (fn [event _ callbacks _ _] + (when (= event :preToolCall) + (when-let [on-after (:on-after-action callbacks)] + (on-after {:parsed {} :exit 0}))))] + (#'tc/decide-tool-call-action tool-call all-tools db config agent-name chat-id + {:on-after-hook-action (fn [result] (reset! notified* result))}) + ;; tool-call-id correlates the hook with its tool widget; clients can + ;; use it to associate the hook block with the tool call. + (is (match? {:tool-call-id "call-1"} + @notified*))))) + (testing "ask_user with allowed config sends :ask to preToolCall hook" (h/reset-components!) (let [tool-call {:id "call-1" @@ -300,7 +404,7 @@ (is (= :ask (:approval @hook-data*)) "preToolCall hook receives :ask so notification hooks fire while waiting on user") (is (match? {:decision :allow - :hook-rejected? false} + :tool-call-rejected-by-hook? false} plan) "tool actually executes (:allow) so the question still reaches the user"))))) @@ -456,6 +560,70 @@ (is (= (:api-key renewed-provider-auth) (:api-key (:provider-auth result))) "rejection path must also return refreshed provider-auth")))))) +(deftest on-tools-called!-pretoolcall-continue-false-halts-batch-test + (testing "a preToolCall continue:false on the first tool stops the batch: the second tool is never processed" + (h/reset-components!) + (let [chat-id "test-chat" + db* (h/db*) + _ (swap! db* #(-> % + (assoc-in [:chats chat-id :status] :running) + (assoc-in [:chats chat-id :messages] []) + (assoc-in [:chats chat-id :tool-calls "call-1" :status] :preparing) + (assoc-in [:chats chat-id :tool-calls "call-2" :status] :preparing))) + chat-ctx {:db* db* + :config (h/config) + :chat-id chat-id + :provider "openai" + :agent :default + :messenger (h/messenger) + :metrics (h/metrics)} + received-msgs* (atom "") + add-to-history! (fn [msg] + (swap! db* update-in [:chats chat-id :messages] (fnil conj []) msg)) + tool-calls [{:id "call-1" :full-name "eca__tool_a" :arguments {} :arguments-text "{}"} + {:id "call-2" :full-name "eca__tool_b" :arguments {} :arguments-text "{}"}] + all-tools [{:name "tool_a" :full-name "eca__tool_a" :origin :eca :server {:name "eca"}} + {:name "tool_b" :full-name "eca__tool_b" :origin :eca :server {:name "eca"}}] + ;; tool names that reached the preToolCall hook (i.e. were processed) + hooked-tools* (atom []) + ;; tools that were actually executed + called-tools* (atom []) + ;; system messages surfaced to the user + sent* (atom [])] + (with-redefs [f.tools/all-tools (constantly all-tools) + f.tools/approval (constantly :allow) + f.hooks/trigger-if-matches! + (fn [hook-type data {:keys [on-after-action]} _ _] + (when (= :preToolCall hook-type) + (swap! hooked-tools* conj (:tool-name data)) + ;; The first tool's hook returns a successful continue:false. + (when (= "tool_a" (:tool-name data)) + (on-after-action {:exit 0 + :name "stopper" + :parsed {"continue" false "stopReason" "halt"}})))) + f.tools/call-tool! (fn [full-name & _] + (swap! called-tools* conj full-name) + {:contents [{:text "ok" :type :text}]}) + f.tools/tool-call-details-before-invocation (constantly nil) + f.tools/tool-call-details-after-invocation (constantly nil) + f.tools/tool-call-summary (constantly "Tool") + lifecycle/maybe-renew-auth-token (fn [_] nil) + lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (let [result ((tc/on-tools-called! chat-ctx received-msgs* add-to-history! []) tool-calls)] + ;; continue:false stops the turn — the loop returns nil (no continuation). + (is (nil? result)) + ;; Only the first tool was ever processed by preToolCall hooks. + (is (= ["tool_a"] @hooked-tools*) + "the second tool must not be processed after a current-turn stop") + ;; The second tool was never executed. + (is (not-any? #{"eca__tool_b"} @called-tools*) + "the second tool must not run after a current-turn stop") + ;; A turn-stop message carrying the hook's reason is surfaced to the user. + ;; The exact wording is covered by lifecycle/turn-stopped-by-hook-message-test. + (is (some #(some-> (get-in % [:content :text]) (string/includes? "halt")) @sent*) + "a turn-stopped message must be shown")))))) + (deftest send-toolCalled-image-content-emission-test (testing "image outputs are split into separate ChatImageContent events" (h/reset-components!) @@ -497,3 +665,68 @@ (is (match? {:type :toolCalled :outputs [{:type :text :text "done"}]} (first events))))))) + +(deftest post-tool-call-stop-info-test + (let [prompt-id "prompt-2"] + ;; [tool-calls => expected stop info] + (are [tool-calls expected] (= expected (#'tc/post-tool-call-stop-info tool-calls prompt-id)) + ;; empty tool-calls map reports no stop + {} {:stop-turn? false :stop-reason nil :stop-hook-name nil} + ;; no hook requested a stop + {"t1" {:status :completed}} {:stop-turn? false :stop-reason nil :stop-hook-name nil} + ;; stale stop from an older prompt is ignored + {"t1" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id "prompt-1" + :post-tool-call-stop-reason "old halt" + :post-tool-call-stop-hook-name "old-redact"}} {:stop-turn? false :stop-reason nil :stop-hook-name nil} + ;; stop without prompt id is ignored; postToolCall stops are current-turn only + {"t1" {:post-tool-call-stop? true}} {:stop-turn? false :stop-reason nil :stop-hook-name nil} + ;; current prompt stop requested without a reason + {"t1" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id prompt-id}} {:stop-turn? true :stop-reason nil :stop-hook-name nil} + ;; current prompt stop requested with a reason + {"t1" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id prompt-id + :post-tool-call-stop-reason "halt" + :post-tool-call-stop-hook-name "redact"}} {:stop-turn? true :stop-reason "halt" :stop-hook-name "redact"}) + + (testing "reason is found even when it lives on a different current-prompt tool-call than the first stop flag" + (is (match? {:stop-turn? true :stop-reason "halt"} + (#'tc/post-tool-call-stop-info + (array-map "t1" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id prompt-id} + "t2" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id prompt-id + :post-tool-call-stop-reason "halt"}) + prompt-id)))) + + (testing "old prompt stop does not leak when a later tool call has no stop" + (is (= {:stop-turn? false :stop-reason nil :stop-hook-name nil} + (#'tc/post-tool-call-stop-info + (array-map "old" {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id "prompt-1" + :post-tool-call-stop-reason "old halt"} + "current" {:status :completed}) + prompt-id)))))) + +(deftest trigger-post-tool-call-hook-tags-stop-with-prompt-id-test + (testing "postToolCall continue:false stop state is scoped to the current prompt" + (h/reset-components!) + (let [db* (h/db*)] + (swap! db* assoc-in [:chats "chat-1" :tool-calls "tool-1"] {:status :completed}) + (with-redefs-fn {#'tc/run-post-tool-call-hooks! (constantly {:stop-turn? true + :stop-reason "halt" + :stop-hook-name "guard"})} + (fn [] + (#'tc/execute-action! :trigger-post-tool-call-hook + db* + {:chat-id "chat-1" + :prompt-id "prompt-2" + :messenger (h/messenger)} + "tool-1" + {:outputs [{:type :text :text "ok"}]}))) + (is (match? {:post-tool-call-stop? true + :post-tool-call-stop-prompt-id "prompt-2" + :post-tool-call-stop-reason "halt" + :post-tool-call-stop-hook-name "guard"} + (get-in @db* [:chats "chat-1" :tool-calls "tool-1"])))))) diff --git a/test/eca/features/chat_test.clj b/test/eca/features/chat_test.clj index 077f5177e..30baec691 100644 --- a/test/eca/features/chat_test.clj +++ b/test/eca/features/chat_test.clj @@ -595,6 +595,68 @@ (h/messages)) "Should still surface the API limit reached system message")))) +(deftest auto-compact-hook-block-test + (testing "preCompact continue:false stops the turn with the prefixed reason and does not resume" + (h/reset-components!) + (let [prompted* (atom nil)] + (with-redefs [lifecycle/run-pre-compact-hooks! (constantly {:blocked? true + :reason "auto blocked" + :hook-name "blocker" + :stop-turn? true}) + f.chat/prompt-messages! (fn [user-messages prompt-type chat-ctx] + (reset! prompted* {:user-messages user-messages + :prompt-type prompt-type + :chat-ctx chat-ctx}))] + (#'f.chat/trigger-auto-compact! + {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :parent-chat-id "parent-1"} + [] + [{:role "user" :content [{:type :text :text "keep going"}]}])) + (is (some #(= {:chat-id "chat-1" + :parent-chat-id "parent-1" + :role :system + :content {:type :text :text "Turn stopped by hook 'blocker': auto blocked"}} + %) + (:chat-content-received (h/messages)))) + (is (nil? @prompted*) + "stop-turn prevents resuming the original task"))) + + (testing "preCompact exit 2 blocks compaction without stop-turn and resumes the original task" + (h/reset-components!) + (let [prompted* (atom nil)] + (with-redefs [lifecycle/run-pre-compact-hooks! (constantly {:blocked? true + :reason nil + :hook-name "guard" + :stop-turn? false}) + f.chat/prompt-messages! (fn [user-messages prompt-type chat-ctx] + (reset! prompted* {:user-messages user-messages + :prompt-type prompt-type + :chat-ctx chat-ctx}))] + (#'f.chat/trigger-auto-compact! + {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :parent-chat-id "parent-1"} + [] + [{:role "user" :content [{:type :text :text "keep going"}]}])) + (is (match? {:chat-content-received + [{:chat-id "chat-1" + :parent-chat-id "parent-1" + :role :system + :content {:type :text :text "Compaction blocked by hook 'guard'."}}]} + (h/messages))) + (is (match? {:prompt-type :auto-compact-blocked + :chat-ctx {:auto-compacted? true}} + @prompted*))))) + + (defn ^:private make-tool-output-msg [id text] {:role "tool_call_output" :content {:id id diff --git a/test/eca/features/commands_test.clj b/test/eca/features/commands_test.clj index 24b79f257..a83efbe6c 100644 --- a/test/eca/features/commands_test.clj +++ b/test/eca/features/commands_test.clj @@ -466,3 +466,53 @@ (is (= 10 (count result))) (is (= {:name "arg1" :required true} (first result))) (is (= {:name "arg10" :required true} (last result)))))) + +(deftest hooks-msg-test + (testing "no hooks configured shows the empty message" + (is (= "No hooks configured. Add hooks to your config under the `hooks` key." + (#'f.commands/hooks-msg {:hooks {}})))) + + (testing "a hook with no actions is treated as no hooks" + (is (= "No hooks configured. Add hooks to your config under the `hooks` key." + (#'f.commands/hooks-msg {:hooks {"empty" {:type "preToolCall" :actions []}}})))) + + (testing "description is rendered after the hook name" + (let [out (#'f.commands/hooks-msg + {:hooks {"guard" {:type "preToolCall" + :description "blocks dangerous tools" + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (string/includes? out "`guard`")) + (is (string/includes? out "— blocks dangerous tools")))) + + (testing "invisible hooks render the hidden marker" + (let [out (#'f.commands/hooks-msg + {:hooks {"silent" {:type "preToolCall" + :visible false + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (string/includes? out "*(hidden)*")))) + + (testing "visible hooks (default) do not render the hidden marker" + (let [out (#'f.commands/hooks-msg + {:hooks {"loud" {:type "preToolCall" + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (not (string/includes? out "*(hidden)*"))))) + + (testing "a string matcher is rendered verbatim" + (let [out (#'f.commands/hooks-msg + {:hooks {"m" {:type "preToolCall" + :matcher "read_file" + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (string/includes? out "matcher: `read_file`")))) + + (testing "a map matcher is rendered as JSON" + (let [out (#'f.commands/hooks-msg + {:hooks {"m" {:type "preToolCall" + :matcher {:tool_name "read_file"} + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (string/includes? out "matcher: `{\"tool_name\":\"read_file\"}`")))) + + (testing "plugin-prefixed hook names are rendered" + (let [out (#'f.commands/hooks-msg + {:hooks {"my-plugin::guard" {:type "preToolCall" + :actions [{:type "shell" :shell "exit 0"}]}}})] + (is (string/includes? out "`my-plugin::guard`"))))) diff --git a/test/eca/features/hooks_test.clj b/test/eca/features/hooks_test.clj index dcd22bd46..c7668bcdf 100644 --- a/test/eca/features/hooks_test.clj +++ b/test/eca/features/hooks_test.clj @@ -1,15 +1,24 @@ (ns eca.features.hooks-test (:require + [babashka.fs :as fs] [cheshire.core :as json] + [clojure.string :as string] [clojure.test :refer [deftest is testing]] [eca.config :as config] + [eca.features.chat :as f.chat] [eca.features.chat.lifecycle :as lifecycle] + [eca.features.chat.tool-calls :as tc] [eca.features.hooks :as f.hooks] + [eca.features.tools :as f.tools] + [eca.logger :as logger] [eca.test-helper :as h] [matcher-combinators.test :refer [match?]])) (h/reset-components-before-test) +(defn cache-session-id [db-cache-path] + (some-> db-cache-path fs/file fs/parent fs/file-name str)) + (defn set-action-payload [a*] (fn [p] (reset! a* p))) @@ -55,7 +64,137 @@ {:server "my-mcp" :tool-name "my-tool"} {:on-after-action (set-action-payload result*)} (h/db) (h/config))) - (is (match? {:name "my-hook"} @result*))))) + (is (match? {:name "my-hook"} @result*)))) + + (testing "preCompact matcher filters by triggered" + (h/reset-components!) + (h/config! {:hooks {"manual-only" {:type "preCompact" + :matcher "manual" + :actions [{:type "shell" :shell "echo hey"}]}}}) + (let [result* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 :out "hey" :err nil})] + (f.hooks/trigger-if-matches! :preCompact + {:triggered "auto"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + (is (nil? @result*)) + + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 :out "hey" :err nil})] + (f.hooks/trigger-if-matches! :preCompact + {:triggered "manual"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + (is (match? {:name "manual-only"} @result*)))) + + (testing "preCompact without matcher runs for manual and auto triggers" + (h/reset-components!) + (h/config! {:hooks {"all-compact" {:type "preCompact" + :actions [{:type "shell" :shell "echo hey"}]}}}) + (let [triggers* (atom [])] + (with-redefs [f.hooks/run-shell-cmd (fn [{:keys [input]}] + (swap! triggers* conj (:triggered (json/parse-string input true))) + {:exit 0 :out "hey" :err nil})] + (doseq [triggered ["manual" "auto"]] + (f.hooks/trigger-if-matches! :preCompact + {:triggered triggered} + {} + (h/db) (h/config)))) + (is (= ["manual" "auto"] @triggers*)))) + + (testing "compact object matcher warns and skips" + (h/reset-components!) + (h/config! {:hooks {"bad-compact" {:type "preCompact" + :matcher {"manual" {}} + :actions [{:type "shell" :shell "echo hey"}]}}}) + (let [result* (atom nil) + warnings* (atom [])] + (with-redefs [logger/warn (fn [& args] (swap! warnings* conj args)) + f.hooks/run-shell-cmd (constantly {:exit 0 :out "hey" :err nil})] + (f.hooks/trigger-if-matches! :preCompact + {:triggered "manual"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + (is (nil? @result*)) + (is (some #(= "Ignoring unsupported compact hook matcher" (second %)) @warnings*)))) + + (testing "exit code 2 does not stop remaining pre hooks" + (doseq [[hook-type data] [[:preRequest {:foo "1"}] + [:preToolCall {:server "eca" :tool-name "write_file"}] + [:preCompact {:triggered "manual"}]]] + (h/reset-components!) + (h/config! {:hooks {"a-block" {:type (name hook-type) + :actions [{:type "shell" :shell "exit 2"}]} + "b-after" {:type (name hook-type) + :actions [{:type "shell" :shell "echo after"}]}}}) + (let [calls* (atom [])] + (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 (= "a-block" hook-name) + {:exit f.hooks/hook-rejection-exit-code + :out nil + :err "blocked"} + {:exit 0 + :out "after" + :err nil})))] + (f.hooks/trigger-if-matches! hook-type data {} (h/db) (h/config))) + (is (= ["a-block" "b-after"] @calls*))))) + + (testing "exit 0 continue false stops remaining non-chatEnd hooks" + (h/reset-components!) + (h/config! {:hooks {"a-stop" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]} + "b-after" {:type "postRequest" + :actions [{:type "shell" :shell "echo after"}]}}}) + (let [calls* (atom [])] + (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 (= "a-stop" hook-name) + {:exit 0 + :out "{\"continue\":false}" + :err nil} + {:exit 0 + :out "after" + :err nil})))] + (f.hooks/trigger-if-matches! :postRequest {:prompt "hi"} {} (h/db) (h/config))) + (is (= ["a-stop"] @calls*)))) + + (testing "non-zero continue false does not stop remaining hooks" + (h/reset-components!) + (h/config! {:hooks {"a-fail" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]} + "b-after" {:type "postRequest" + :actions [{:type "shell" :shell "echo after"}]}}}) + (let [calls* (atom [])] + (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 (= "a-fail" hook-name) + {:exit 1 + :out "{\"continue\":false}" + :err "failed"} + {:exit 0 + :out "after" + :err nil})))] + (f.hooks/trigger-if-matches! :postRequest {:prompt "hi"} {} (h/db) (h/config))) + (is (= ["a-fail" "b-after"] @calls*)))) + + (testing "chatEnd continue false stops remaining hooks" + (h/reset-components!) + (h/config! {:hooks {"a-cleanup" {:type "chatEnd" + :actions [{:type "shell" :shell "echo"}]} + "b-cleanup" {:type "chatEnd" + :actions [{:type "shell" :shell "echo after"}]}}}) + (let [calls* (atom [])] + (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) + {:exit 0 + :out "{\"continue\":false}" + :err nil}))] + (f.hooks/trigger-if-matches! :chatEnd {:chat-id "chat-1"} {} (h/db) (h/config))) + (is (= ["a-cleanup"] @calls*))))) ;;; JSON parsing tests @@ -99,13 +238,15 @@ (reset! result* (json/parse-string (:input opts) true)) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :preToolCall - (merge (f.hooks/chat-hook-data (h/db) "chat-1" "code") + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "chat-1" :agent "code"}) {:tool-name "read_file" :server "eca" :tool-input {:path "/foo"} - :approval :ask}) + :approval :ask + :tool-call-id "call-abc"}) {} (h/db) (h/config))) (is (= {:path "/foo"} (:tool_input @result*))) + (is (= "call-abc" (:tool_call_id @result*))) (is (not (contains? @result* :arguments))))) (testing "postToolCall receives tool_input and tool_response" @@ -118,17 +259,19 @@ (reset! result* (json/parse-string (:input opts) true)) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :postToolCall - (merge (f.hooks/chat-hook-data (h/db) "chat-1" "code") + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "chat-1" :agent "code"}) {:tool-name "read_file" :server "eca" :tool-input {:path "/foo"} - :tool-response {:content "data"}}) + :tool-response "file content\n" + :tool-call-id "call-def"}) {} (h/db) (h/config))) (is (= {:path "/foo"} (:tool_input @result*))) - (is (= {:content "data"} (:tool_response @result*)))))) + (is (= "call-def" (:tool_call_id @result*))) + (is (= "file content\n" (:tool_response @result*)))))) -(deftest stop-hook-active-test - (testing "postRequest receives stop_hook_active flag" +(deftest follow-up-active-test + (testing "postRequest receives follow_up_active flag" (h/reset-components!) (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) (h/config! {:hooks {"test" {:type "postRequest" @@ -138,11 +281,174 @@ (reset! result* (json/parse-string (:input opts) true)) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :postRequest - (merge (f.hooks/chat-hook-data (h/db) "chat-1" "code") + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "chat-1" :agent "code"}) {:prompt "test" - :stop-hook-active false}) + :follow-up-active false}) {} (h/db) (h/config))) - (is (false? (:stop_hook_active @result*)))))) + (is (false? (:follow_up_active @result*)))))) + +(deftest postrequest-follow-up-active-defaults-to-false-via-finish-test + (testing "finish-chat-prompt! always sends follow_up_active as a boolean (false when no follow-up is active)" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"pr" {:type "postRequest" + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [opts] + (reset! input* (json/parse-string (:input opts) true)) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (some? @input*)) + ;; The key is always present, even when false. + (is (contains? @input* :follow_up_active)) + (is (false? (:follow_up_active @input*)))))) + +(deftest post-request-response-test + (testing "postRequest receives current response, not prompt" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code" + :messages [{:role "assistant" + :content [{:type :text :text "old answer"}]}]}}) + (h/config! {:hooks {"test" {:type "postRequest" + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [opts] + (reset! input* (json/parse-string (:input opts) true)) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :message "original prompt" + :response "final answer" + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (= "final answer" (:response @input*))) + (is (not (contains? @input* :prompt))))) + + (testing "postRequest omits response instead of falling back to older history" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code" + :messages [{:role "assistant" + :content [{:type :text :text "old answer"}]}]}}) + (h/config! {:hooks {"test" {:type "postRequest" + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [opts] + (reset! input* (json/parse-string (:input opts) true)) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (not (contains? @input* :response)))))) + +(deftest finish-chat-prompt-finalization-test + (testing "finalizer runs while auto-compacting so maintenance prompts can unblock" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code" + :auto-compacting? true}}) + (let [finalized?* (atom false)] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :on-finished-side-effect (fn [] + (reset! finalized?* true) + (swap! (h/db*) update-in [:chats "chat-1"] dissoc :auto-compacting?))}) + (is (true? @finalized?*)) + (is (not (get-in (h/db) [:chats "chat-1" :auto-compacting?]))))) + + (testing "postRequest hooks are skipped while auto-compacting (maintenance prompt finish)" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code" + :auto-compacting? true}}) + (h/config! {:hooks {"pr" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [ran?* (atom false)] + (with-redefs [f.hooks/run-shell-cmd (fn [_] + (reset! ran?* true) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (false? @ran?*) "postRequest must not fire after the auto-compact maintenance prompt"))) + + (testing "postRequest hooks are skipped when caller sets :skip-post-request-hooks? (manual /compact)" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"pr" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [ran?* (atom false)] + (with-redefs [f.hooks/run-shell-cmd (fn [_] + (reset! ran?* true) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :skip-post-request-hooks? true})) + (is (false? @ran?*) "postRequest must not fire when :skip-post-request-hooks? is set (manual /compact)"))) + + (testing "postRequest followUp does not suppress mandatory finalization" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"follow" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [finalized?* (atom false) + after-finish?* (atom false) + follow-up* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"followUp\":\"please continue\"}" + :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :on-finished-side-effect #(reset! finalized?* true) + :on-after-finish! #(reset! after-finish?* true) + :on-follow-up (fn [follow-up-text _chat-ctx] + (reset! follow-up* follow-up-text))})) + (is (true? @finalized?*)) + (is (false? @after-finish?*)) + (is (= "please continue" @follow-up*)))) + + (testing "postRequest continue false does not suppress mandatory finalization" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"stop" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [finalized?* (atom false) + after-finish?* (atom false)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false}" + :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :on-finished-side-effect #(reset! finalized?* true) + :on-after-finish! #(reset! after-finish?* true)})) + (is (true? @finalized?*)) + (is (false? @after-finish?*))))) (deftest approval-field-test (testing "preToolCall can return approval in JSON output" @@ -173,6 +479,7 @@ {:id "action-1" :name "my-hook" :type "shell" + :hook-type :preRequest :visible? true :exit 0 :parsed {"systemMessage" "Hook done" @@ -186,11 +493,86 @@ :id "action-1" :name "my-hook" :status 0 - :output "Hook done\nReplacedPrompt: \"new prompt\"\nAdditionalContext: extra context" + :output "Hook executed\nReplacedPrompt: \"new prompt\"\nAdditionalContext: extra context" + :error nil}} + ;; systemMessage is surfaced standalone, prefixed with the hook name + {:role :system + :content {:type :text :text "Hook 'my-hook': Hook done\n\n"}}] + @sent*)))) + + (testing "tool hooks carry tool-call-id for correlation (no Tool line in the body)" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "action-1" + :name "my-hook" + :type "shell" + :hook-type :postToolCall + :visible? true + :exit 0 + :parsed {"replacedOutput" "redacted"} + :raw-output nil + :raw-error nil + :tool-call-id "call-42"})) + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "action-1" + :name "my-hook" + :status 0 + :tool-call-id "call-42" + :output "Hook executed\nReplacedOutput: \"redacted\"" :error nil}}] @sent*)))) - (testing "suppressOutput string key hides hook output" + (testing "tool-call-id survives suppressOutput (metadata, not body)" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "action-1" + :name "my-hook" + :type "shell" + :hook-type :postToolCall + :visible? true + :exit 0 + :parsed {"suppressOutput" true} + :raw-output nil + :raw-error nil + :tool-call-id "call-42"})) + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "action-1" + :name "my-hook" + :status 0 + :tool-call-id "call-42"}}] + @sent*)))) + + (testing "replacedPrompt display is limited to preRequest" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "action-1" + :name "my-hook" + :type "shell" + :hook-type :postRequest + :visible? true + :exit 0 + :parsed {"systemMessage" "Hook done" + "replacedPrompt" "ignored" + "additionalContext" "extra context"} + :raw-output nil + :raw-error nil})) + (is (= "Hook executed\nAdditionalContext: extra context" + (get-in (first @sent*) [:content :output]))))) + + (testing "suppressOutput keeps the finished event (closes the spinner) but drops output/error" (let [sent* (atom [])] (with-redefs [lifecycle/send-content! (fn [_ role content] (swap! sent* conj {:role role :content content}))] @@ -204,6 +586,153 @@ :parsed {"suppressOutput" true} :raw-output nil :raw-error nil})) + ;; visible? sent a started, so a matching finished must close the spinner; + ;; suppressOutput only strips the body (no :output/:error). + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "action-1" + :name "my-hook" + :status 0}}] + @sent*)))) + + (testing "non-zero exit displays raw stdout instead of parsed JSON fields" + (let [sent* (atom []) + raw-output "{\"additionalContext\":\"ignored\"}"] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "action-1" + :name "my-hook" + :type "shell" + :visible? true + :exit 1 + :parsed {"additionalContext" "ignored"} + :raw-output raw-output + :raw-error "failed"})) + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "action-1" + :name "my-hook" + :status 1 + :output raw-output + :error "failed"}}] + @sent*)))) + + (testing "chatEnd ignores parsed output effects" + (let [sent* (atom []) + raw-output "{\"suppressOutput\":true,\"additionalContext\":\"ignored\"}"] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "action-1" + :name "cleanup" + :type "shell" + :hook-type :chatEnd + :visible? true + :exit 0 + :parsed {"suppressOutput" true + "additionalContext" "ignored"} + :raw-output raw-output + :raw-error nil})) + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "action-1" + :name "cleanup" + :status 0 + :output raw-output + :error nil}}] + @sent*))))) + +(deftest invisible-hook-notification-test + (testing "invisible hook with no systemMessage stays silent" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "quiet" :type "shell" :hook-type :preToolCall + :visible? false :exit 0 + :parsed {"additionalContext" "ctx" "replacedOutput" "x"} + :raw-output "stdout" :raw-error "stderr"})) + (is (empty? @sent*) + "narration, stdout/stderr and LLM-payload fields are suppressed for invisible hooks"))) + + (testing "invisible hook surfaces systemMessage as a plain system text" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "quiet" :type "shell" :hook-type :preToolCall + :visible? false :exit 0 + :parsed {"systemMessage" "Heads up" "additionalContext" "ctx"} + :raw-output nil :raw-error nil})) + (is (= [{:role :system :content {:type :text :text "Hook 'quiet': Heads up\n\n"}}] + @sent*) + "only the deliberate systemMessage is surfaced (prefixed), not additionalContext/diagnostics"))) + + (testing "blank systemMessage is treated as absent" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "quiet" :type "shell" :hook-type :preToolCall + :visible? false :exit 0 + :parsed {"systemMessage" " "} + :raw-output nil :raw-error nil})) + (is (empty? @sent*)))) + + (testing "suppressOutput hides the block but never the systemMessage" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "quiet" :type "shell" :hook-type :preToolCall + :visible? false :exit 0 + :parsed {"systemMessage" "Heads up" "suppressOutput" true} + :raw-output nil :raw-error nil})) + (is (= [{:role :system :content {:type :text :text "Hook 'quiet': Heads up\n\n"}}] + @sent*)))) + + (testing "visible hook with suppressOutput still surfaces systemMessage (block hidden, message kept)" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "loud" :type "shell" :hook-type :preToolCall + :visible? true :exit 0 + :parsed {"systemMessage" "Heads up" "suppressOutput" true "additionalContext" "ctx"} + :raw-output nil :raw-error "noise"})) + ;; visible? -> a finished still closes the spinner, but suppressOutput strips + ;; its body (no stderr/additionalContext). systemMessage survives separately. + (is (= [{:role :system + :content {:type :hookActionFinished + :action-type "shell" + :id "a1" + :name "loud" + :status 0}} + {:role :system :content {:type :text :text "Hook 'loud': Heads up\n\n"}}] + @sent*) + "suppressOutput drops the block body but keeps the finished event; systemMessage survives"))) + + (testing "invisible hook does not surface systemMessage on non-zero exit (JSON effects only on exit 0)" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-after-hook-action! + {} + {:id "a1" :name "quiet" :type "shell" :hook-type :preToolCall + :visible? false :exit 2 + :parsed {"systemMessage" "Heads up"} + :raw-output nil :raw-error "boom"})) (is (empty? @sent*))))) (deftest session-hooks-test @@ -220,6 +749,8 @@ {} (h/db) (h/config))) (is (contains? @result* :workspaces)) (is (contains? @result* :db_cache_path)) + (is (= (cache-session-id (:db_cache_path @result*)) (:session_id @result*))) + (is (= (first (:workspaces @result*)) (:cwd @result*))) (is (not (contains? @result* :chat_id))) (is (not (contains? @result* :behavior))) (is (not (contains? @result* :agent))))) @@ -276,6 +807,243 @@ (is (= 10 (:message_count @result*))) (is (not (contains? @result* :session_end)))))) +(deftest compact-hooks-test + (testing "preCompact receives ECA-style compact fields" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :matcher "manual" + :visible false + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [opts] + (reset! input* (json/parse-string (:input opts) true)) + {:exit 0 :out "" :err nil})] + (is (= {:blocked? false :reason nil :hook-name nil :stop-turn? false} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :full-model "openai/gpt" + :variant "high"} + "manual" + "keep decisions")))) + (is (= "chat-1" (:chat_id @input*))) + (is (= "preCompact" (:hook_type @input*))) + (is (= "manual" (:triggered @input*))) + (is (= "keep decisions" (:custom_instructions @input*))) + (is (= "openai/gpt" (:full_model @input*))) + (is (= "high" (:variant @input*))) + (is (contains? @input* :workspaces)) + (is (= (cache-session-id (:db_cache_path @input*)) (:session_id @input*))) + (is (= (first (:workspaces @input*)) (:cwd @input*))) + (is (not (contains? @input* :hook_event_name))))) + + (testing "preCompact can block with continue false and stopReason" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"not now\"}" + :err nil})] + (is (= {:blocked? true :reason "not now" :hook-name "test" :stop-turn? true} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "auto" + ""))))) + + (testing "preCompact continue false without stopReason ignores stderr" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false}" + :err "do not show"})] + (is (= {:blocked? true :reason nil :hook-name "test" :stop-turn? true} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "auto" + ""))))) + + (testing "preCompact exit 2 blocks compaction without user-facing reason" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :visible true + :actions [{:type "shell" :shell "exit 2"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit f.hooks/hook-rejection-exit-code + :out nil + :err " blocked by policy "})] + ;; Exit 2 surfaces no stderr reason, but the blocking hook name is kept. + (is (= {:blocked? true :reason nil :hook-name "test" :stop-turn? false} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger)} + "manual" + ""))))) + + (testing "preCompact exit 2 when invisible also blocks without user-facing reason" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :visible false + :actions [{:type "shell" :shell "exit 2"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit f.hooks/hook-rejection-exit-code + :out nil + :err " hidden policy "})] + ;; Exit 2 surfaces no stderr reason, but the blocking hook name is kept. + (is (= {:blocked? true :reason nil :hook-name "test" :stop-turn? false} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + ""))))) + + (testing "preCompact non-zero continue false is ignored" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 1 + :out "{\"continue\":false,\"stopReason\":\"ignored\"}" + :err "failed"})] + (is (= {:blocked? false :reason nil :hook-name nil :stop-turn? false} + (lifecycle/run-pre-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + ""))))) + + (testing "postCompact additionalContext is appended after the compact marker" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "compact_marker" :content {:auto? false}} + {:role "user" :content [{:type :text :text "The conversation was compacted."}]} + {:role "assistant" :content [{:type :text :text "later message"}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [opts] + (reset! input* (json/parse-string (:input opts) true)) + {:exit 0 + :out "{\"additionalContext\":\"resume snapshot\"}" + :err nil})] + (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :full-model "openai/gpt" + :variant "low"} + "manual" + "summary")) + (is (= "postCompact" (:hook_type @input*))) + (is (= "manual" (:triggered @input*))) + (is (= "summary" (:compact_summary @input*))) + (is (= "openai/gpt" (:full_model @input*))) + (is (= "low" (:variant @input*))) + (is (= (cache-session-id (:db_cache_path @input*)) (:session_id @input*))) + (is (= (first (:workspaces @input*)) (:cwd @input*)))) + (is (= {:role "compact_marker" :content {:auto? false}} + (first (get-in (h/db) [:chats "chat-1" :messages])))) + (is (match? {:role "user" + :content [{:type :text :text "The conversation was compacted."} + {:type :text :text #"resume snapshot"}]} + (second (get-in (h/db) [:chats "chat-1" :messages]))))) + + (testing "postCompact continue false returns stop-turn without losing summary" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "compact_marker" :content {:auto? false}} + {:role "user" :content [{:type :text :text "The conversation was compacted."}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false}" + :err nil})] + (is (= {:stop-turn? true :stop-reason nil :stop-hook-name "test"} + (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "auto" + "summary")))) + (is (= [{:type :text :text "The conversation was compacted."}] + (:content (last (get-in (h/db) [:chats "chat-1" :messages])))))) + + (testing "postCompact plain stdout is NOT treated as additionalContext" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "compact_marker" :content {:auto? false}} + {:role "user" :content [{:type :text :text "The conversation was compacted."}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "plain resume snapshot" + :err nil})] + (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + "summary")) + ;; Plain stdout is NOT converted to additionalContext + (is (= [{:type :text :text "The conversation was compacted."}] + (:content (last (get-in (h/db) [:chats "chat-1" :messages])))))) + + (testing "postCompact blank additionalContext is treated as absent" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "compact_marker" :content {:auto? false}} + {:role "user" :content [{:type :text :text "The conversation was compacted."}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"additionalContext\":\"\"}" + :err nil})] + (is (= {:stop-turn? false :stop-reason nil :stop-hook-name nil} + (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + "summary")))) + (is (= [{:type :text :text "The conversation was compacted."}] + (:content (last (get-in (h/db) [:chats "chat-1" :messages])))))) + + (testing "postCompact warns when compact summary message is missing" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "assistant" :content [{:type :text :text "no marker"}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (let [warnings* (atom [])] + (with-redefs [logger/warn (fn [& args] (swap! warnings* conj args)) + f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"additionalContext\":\"resume snapshot\"}" + :err nil})] + (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + "summary")) + (is (= [{:role "assistant" :content [{:type :text :text "no marker"}]}] + (get-in (h/db) [:chats "chat-1" :messages]))) + (is (some #(= "Skipping postCompact additionalContext: compact summary message not found" (second %)) + @warnings*))))) + ;;; postToolCall runOnError tests (deftest posttoolcall-runonerror-test @@ -287,7 +1055,7 @@ (let [ran?* (atom false)] (with-redefs [f.hooks/run-shell-cmd (fn [_] (reset! ran?* true) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :postToolCall - (merge (f.hooks/chat-hook-data (h/db) "chat-1" "code") + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "chat-1" :agent "code"}) {:tool-name "tool" :server "eca" :error true}) {} (h/db) (h/config))) (is (false? @ran?*)))) @@ -301,13 +1069,13 @@ (let [ran?* (atom false)] (with-redefs [f.hooks/run-shell-cmd (fn [_] (reset! ran?* true) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :postToolCall - (merge (f.hooks/chat-hook-data (h/db) "chat-1" "code") + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "chat-1" :agent "code"}) {:tool-name "tool" :server "eca" :error true}) {} (h/db) (h/config))) (is (true? @ran?*))))) (deftest subagent-post-request-test - (testing "subagentPostRequest hook triggers with parent_chat_id" + (testing "subagentPostRequest hook triggers with parent_chat_id and response" (h/reset-components!) (swap! (h/db*) assoc :chats {"sub-1" {:agent "explorer"}}) (h/config! {:hooks {"test" {:type "subagentPostRequest" @@ -317,14 +1085,15 @@ (reset! result* (json/parse-string (:input opts) true)) {:exit 0 :out "" :err nil})] (f.hooks/trigger-if-matches! :subagentPostRequest - (merge (f.hooks/chat-hook-data (h/db) "sub-1" "explorer") - {:prompt "explore the codebase" + (merge (f.hooks/chat-hook-data (h/db) {:chat-id "sub-1" :agent "explorer"}) + {:response "found the answer" :parent-chat-id "parent-1"}) {} (h/db) (h/config))) (is (= "sub-1" (:chat_id @result*))) (is (= "explorer" (:agent @result*))) (is (= "parent-1" (:parent_chat_id @result*))) - (is (= "explore the codebase" (:prompt @result*))))) + (is (= "found the answer" (:response @result*))) + (is (not (contains? @result* :prompt))))) (testing "postRequest does not trigger for subagentPostRequest hook type" (h/reset-components!) @@ -367,7 +1136,9 @@ :tool-input {"path" "/foo/bar/spec.allium"}} {:on-after-action (set-action-payload result*)} (h/db) (h/config))) - (is (match? {:name :spec-check} @result*))))) + ;; Hook name is stringified by trigger-if-matches! (keywordized config key + ;; -> string), so downstream string-building never leaks a ':' prefix. + (is (match? {:name "spec-check"} @result*))))) (deftest matcher-map-test (testing "map matcher with argsMatchers filters by tool argument" @@ -407,14 +1178,14 @@ (testing "map matcher uses toolApproval-like selectors, not regex" (h/reset-components!) (h/config! {:hooks {"native-write" {:type "preToolCall" - :matcher {"write_file" {}} - :actions [{:type "shell" :shell "echo ok"}]} - "server-hook" {:type "preToolCall" - :matcher {"my-mcp" {}} - :actions [{:type "shell" :shell "echo ok"}]} - "regex-looking" {:type "preToolCall" - :matcher {".*write_file" {}} - :actions [{:type "shell" :shell "echo ok"}]}}}) + :matcher {"write_file" {}} + :actions [{:type "shell" :shell "echo ok"}]} + "server-hook" {:type "preToolCall" + :matcher {"my-mcp" {}} + :actions [{:type "shell" :shell "echo ok"}]} + "regex-looking" {:type "preToolCall" + :matcher {".*write_file" {}} + :actions [{:type "shell" :shell "echo ok"}]}}}) (let [native-tools [{:origin :native :name "write_file"}] result* (atom nil)] ;; Native short selector matches only the ECA native tool. @@ -469,9 +1240,9 @@ (h/config! {:hooks {"bad-legacy" {:type "preToolCall" :matcher "[" :actions [{:type "shell" :shell "echo ok"}]} - "bad-args" {:type "preToolCall" - :matcher {"eca__write_file" {:argsMatchers {"path" ["["]}}} - :actions [{:type "shell" :shell "echo ok"}]}}}) + "bad-args" {:type "preToolCall" + :matcher {"eca__write_file" {:argsMatchers {"path" ["["]}}} + :actions [{:type "shell" :shell "echo ok"}]}}}) (let [result* (atom nil)] (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 :out "ok" :err nil})] (f.hooks/trigger-if-matches! :preToolCall @@ -615,7 +1386,7 @@ (h/reset-components!) (h/config! {:hooks {"my-hook" {:type "preToolCall" :matcher {"eca__write_file" {:argsMatchers {"path" [".*\\.allium$"] - "original_content" [".*foo.*"]}}} + "original_content" [".*foo.*"]}}} :actions [{:type "shell" :shell "echo ok"}]}}}) (let [result* (atom nil)] ;; Both args match → match @@ -638,3 +1409,736 @@ {:on-after-action (set-action-payload result*)} (h/db) (h/config))) (is (nil? @result*))))) + +;;; New tests for hook consistency improvements + +(deftest json-only-additional-context-test + (testing "preRequest plain stdout is NOT treated as additionalContext" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preRequest" + :actions [{:type "shell" :shell "echo plain"}]}}}) + (let [result* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 :out "plain text" :err nil})] + (f.hooks/trigger-if-matches! :preRequest + {:foo "1"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + ;; Raw output is present but parsed is nil + (is (= "plain text" (:raw-output @result*))) + (is (nil? (:parsed @result*))))) + + (testing "preRequest JSON additionalContext works on exit 0" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [result* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"additionalContext\":\"ctx\"}" + :err nil})] + (f.hooks/trigger-if-matches! :preRequest + {:foo "1"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + (is (= "ctx" (get-in @result* [:parsed "additionalContext"])))))) + +(deftest prerequest-block-does-not-fire-postrequest-test + (testing "preRequest exit 2 blocks the prompt but not remaining preRequest hooks" + (h/reset-components!) + (h/config! {:hooks {"a-block" {:type "preRequest" + :actions [{:type "shell" :shell "exit 2"}]} + "b-after" {:type "preRequest" + :actions [{:type "shell" :shell "echo after"}]} + "c-post" {:type "postRequest" + :actions [{:type "shell" :shell "echo post"}]}}}) + (let [calls* (atom []) + inputs* (atom [])] + (with-redefs [f.hooks/run-shell-cmd (fn [{:keys [input]}] + (let [{:keys [hook_name] :as parsed-input} (json/parse-string input true) + hook-name hook_name] + (swap! inputs* conj parsed-input) + (swap! calls* conj hook-name) + (if (= "a-block" hook-name) + {:exit f.hooks/hook-rejection-exit-code + :out nil + :err "blocked"} + {:exit 0 + :out "after" + :err nil})))] + (#'f.chat/prompt-messages! + [{:role "user" :content [{:type :text :text "hello"}]}] + :prompt-message + {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :message "hello" + :messenger (h/messenger) + :metrics (h/metrics) + :full-model "openai/gpt" + :variant "high"})) + (is (= ["a-block" "b-after"] @calls*)) + (is (every? #(= "openai/gpt" (:full_model %)) @inputs*)) + (is (every? #(= "high" (:variant %)) @inputs*)) + (is (every? #(not (contains? % :all_messages)) @inputs*)))) + + (testing "preRequest exit 2 does not run command finish side effects and still shows reason when invisible" + (h/reset-components!) + (h/config! {:hooks {"a-block" {:type "preRequest" + :visible false + :actions [{:type "shell" :shell "exit 2"}]}}}) + (let [side-effect-ran?* (atom false)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit f.hooks/hook-rejection-exit-code + :out nil + :err "blocked invisibly"})] + (#'f.chat/prompt-messages! + [{:role "user" :content [{:type :text :text "hello"}]}] + :eca-command + {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :message "/custom" + :messenger (h/messenger) + :metrics (h/metrics) + :full-model "openai/gpt" + :on-finished-side-effect #(reset! side-effect-ran?* true)})) + (is (false? @side-effect-ran?*)) + (is (some #(= {:chat-id "chat-1" + :role :system + :content {:type :text :text "Request blocked by hook 'a-block': blocked invisibly"}} + %) + (:chat-content-received (h/messages))))))) + +(deftest start-hooks-test + (testing "chatStart receives documented chat fields" + (h/reset-components!) + (h/config! {:hooks {"start" {:type "chatStart" + :visible false + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [config/all (constantly (h/config)) + config/await-plugins-resolved! (fn [] nil) + f.hooks/run-shell-cmd (fn [{:keys [input]}] + (reset! input* (json/parse-string input true)) + {:exit 0 :out "" :err nil})] + (is (= {:stop-turn? false} + (#'f.chat/run-start-hooks! {:db* (h/db*) :chat-id "chat-1" :agent "code" + :messenger (h/messenger) :variant "high"} + (h/db) false "openai/gpt")))) + (is (match? {:chat_id "chat-1" + :agent "code" + :behavior "code" + :full_model "openai/gpt" + :variant "high" + :resumed false} + @input*)))) + + (testing "chatStart continue false returns current-turn stop state" + (h/reset-components!) + (h/config! {:hooks {"start" {:type "chatStart" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [config/all (constantly (h/config)) + config/await-plugins-resolved! (fn [] nil) + f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"not now\"}" + :err nil})] + (is (= {:stop-turn? true :stop-reason "not now" :stop-hook-name "start"} + (#'f.chat/run-start-hooks! {:db* (h/db*) :chat-id "chat-1" :agent "code" + :messenger (h/messenger)} + (h/db) false "openai/gpt"))))) + + (testing "prompt returns resolved model when chatStart stops without explicit model" + (h/reset-components!) + (h/config! {:defaultModel "openai/gpt" + :hooks {"start" {:type "chatStart" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (swap! (h/db*) assoc :models {"openai/gpt" {}}) + (with-redefs [config/all (constantly (h/config)) + config/await-plugins-resolved! (fn [] nil) + f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false}" + :err nil})] + (is (match? {:chat-id "chat-1" + :model "openai/gpt" + :status :prompting} + (#'f.chat/prompt* + {} + {:db* (h/db*) + :config (h/config) + :contexts [] + :chat-id "chat-1" + :agent "code" + :agent-config {} + :message "hello" + :messenger (h/messenger) + :metrics (h/metrics)})))))) + +(deftest pretoolcall-continue-false-test + (testing "preToolCall continue false stops the turn without executing the tool" + (h/reset-components!) + (h/config! {:hooks {"stop-tool" {:type "preToolCall" + :actions [{:type "shell" :shell "echo"}]}} + :toolCall {:approval {:byDefault "allow"}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"stop now\"}" + :err nil})] + (is (match? {:decision :deny + :tool-call-rejected-by-hook? false + :tool-call-blocked-by-hook? true + :stop-turn? true + :stop-reason "stop now" + :stop-hook-name "stop-tool" + ;; Q3: stopReason is user-only; the LLM-visible reason text is neutral. + :reason {:code :hook-stopped + :text "Tool call stopped by hook"}} + (#'tc/decide-tool-call-action + {:full-name "eca__shell_command" + :arguments {"command" "echo hi"}} + (f.tools/all-tools "chat-1" "code" (h/db) (h/config)) + (h/db) + (h/config) + "code" + "chat-1")))))) + +(deftest pretoolcall-structured-denial-test + (testing "exit 0 + approval deny + additionalContext: LLM-visible denial" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preToolCall" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [result* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"approval\":\"deny\",\"additionalContext\":\"You must never access /etc/priv\"}" + :err nil})] + (f.hooks/trigger-if-matches! :preToolCall + {:server "eca" :tool-name "read"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + (is (= "deny" (get-in @result* [:parsed "approval"]))) + (is (= "You must never access /etc/priv" (get-in @result* [:parsed "additionalContext"]))))) + + (testing "exit 2 rejection keeps agent turn continuable and uses stderr as LLM reason" + (let [state (#'tc/process-pre-tool-call-hook-result + {:hook-results [] + :approval-override nil + :tool-call-rejected-by-hook? false + :tool-call-rejection-reason nil + :stop-reason nil + :stop-hook-name nil + :stop-turn? false} + {:exit f.hooks/hook-rejection-exit-code + :raw-output "{\"additionalContext\":\"should not reach LLM\",\"continue\":false}" + :raw-error "Security block" + :parsed {"additionalContext" "should not reach LLM" + "continue" false}})] + (is (true? (:tool-call-rejected-by-hook? state))) + (is (= "Security block" (:tool-call-rejection-reason state))) + (is (false? (:stop-turn? state))) + (is (nil? (:stop-reason state))))) + + (testing "approval deny does not treat stopReason as hook stop reason" + (let [state (#'tc/process-pre-tool-call-hook-result + {:hook-results [] + :approval-override nil + :tool-call-rejected-by-hook? false + :tool-call-rejection-reason nil + :stop-reason nil + :stop-hook-name nil + :stop-turn? false} + {:exit 0 + :raw-output "{\"approval\":\"deny\",\"stopReason\":\"user-only\"}" + :raw-error nil + :parsed {"approval" "deny" + "stopReason" "user-only"}})] + (is (true? (:tool-call-rejected-by-hook? state))) + (is (nil? (:stop-reason state))))) + + (testing "non-zero JSON approval deny is ignored" + (let [state (#'tc/process-pre-tool-call-hook-result + {:hook-results [] + :approval-override nil + :tool-call-rejected-by-hook? false + :tool-call-rejection-reason nil + :stop-reason nil + :stop-hook-name nil + :stop-turn? false} + {:exit 1 + :raw-output "{\"approval\":\"deny\",\"additionalContext\":\"ignored\"}" + :raw-error "failed" + :parsed {"approval" "deny" + "additionalContext" "ignored"}})] + (is (false? (:tool-call-rejected-by-hook? state))) + (is (nil? (:approval-override state)))))) + +(deftest posttoolcall-replaced-output-test + (letfn [(setup! [] + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1"] + {:agent "code" + :messages [{:role "tool_call_output" + :content {:id "tool-1" + :output {:contents [{:type :text :text "original output"}]}}}] + :tool-calls {"tool-1" {:name "shell_command" + :server "eca" + :arguments {}}}}) + (h/config! {:hooks {"sanitize" {:type "postToolCall" + :visible false + :actions [{:type "shell" :shell "echo"}]}}})) + (run-hook! [result] + (with-redefs [f.hooks/run-shell-cmd (constantly result)] + (#'tc/run-post-tool-call-hooks! + (h/db*) + {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "tool-1" + {:outputs [{:type :text :text "original output"}]}))) + (tool-output [] + (get-in (h/db) [:chats "chat-1" :messages 0 :content :output :contents]))] + (testing "replacedOutput replaces tool result on exit 0" + (setup!) + (run-hook! {:exit 0 + :out "{\"replacedOutput\":\"sanitized content\"}" + :err nil}) + (is (= [{:type :text :text "sanitized content"}] + (tool-output)))) + + (testing "empty replacedOutput hides tool result with empty content" + (setup!) + (run-hook! {:exit 0 + :out "{\"replacedOutput\":\"\"}" + :err nil}) + (is (= [{:type :text :text ""}] + (tool-output)))) + + (testing "replacedOutput and continue false are ignored on non-zero exit other than 2" + (setup!) + (let [result (run-hook! {:exit 1 + :out "{\"replacedOutput\":\"sanitized content\",\"continue\":false}" + :err "failed"})] + (is (= [{:type :text :text "original output"}] + (tool-output))) + (is (not (:stop-turn? result))))) + + (testing "exit 2 replaces tool result with stderr and ignores stdout JSON" + (setup!) + (run-hook! {:exit f.hooks/hook-rejection-exit-code + :out "{\"replacedOutput\":\"ignored stdout\"}" + :err "redacted by policy"}) + (is (= [{:type :text :text "redacted by policy"}] + (tool-output)))) + + (testing "exit 2 with empty stderr replaces tool result with placeholder" + (setup!) + (run-hook! {:exit f.hooks/hook-rejection-exit-code + :out "{\"replacedOutput\":\"ignored stdout\"}" + :err nil}) + (is (= [{:type :text :text "[Tool output hidden by hook]"}] + (tool-output)))) + + (testing "continue false is current-turn control state only and preserves stopReason" + (setup!) + (let [result (run-hook! {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"pause here\"}" + :err nil})] + (is (:stop-turn? result)) + (is (= "pause here" (:stop-reason result))) + (is (nil? (get-in (h/db) [:chats "chat-1" :tool-calls "tool-1" :post-tool-call-stop?]))))) + + (testing "blank additionalContext and stopReason are treated as absent" + (setup!) + (let [result (run-hook! {:exit 0 + :out "{\"additionalContext\":\" \",\"continue\":false,\"stopReason\":\"\"}" + :err nil})] + (is (:stop-turn? result)) + (is (nil? (:stop-reason result))) + (is (= [{:type :text :text "original output"}] + (tool-output))))) + + (testing "non-string replacedOutput is ignored" + (setup!) + (run-hook! {:exit 0 + :out "{\"replacedOutput\":{\"text\":\"bad\"}}" + :err nil}) + (is (= [{:type :text :text "original output"}] + (tool-output)))))) + +(deftest compact-matcher-exact-string-test + (testing "compact matcher uses exact string match, not regex" + (h/reset-components!) + (h/config! {:hooks {"manual-regex" {:type "preCompact" + :matcher "man.*" + :actions [{:type "shell" :shell "echo hey"}]}}}) + (let [result* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 :out "hey" :err nil})] + (f.hooks/trigger-if-matches! :preCompact + {:triggered "manual"} + {:on-after-action (set-action-payload result*)} + (h/db) (h/config))) + ;; "man.*" should NOT match "manual" since matchers are exact strings now + (is (nil? @result*))))) + +(deftest hook-notification-action-type-test + (testing "hookActionStarted includes action-type" + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (lifecycle/notify-before-hook-action! + {} + {:id "action-1" + :name "my-hook" + :type "shell" + :visible? true})) + (is (= [{:role :system + :content {:type :hookActionStarted + :action-type "shell" + :name "my-hook" + :id "action-1"}}] + @sent*))))) + +(deftest matcher-warning-policy-test + (testing "unsupported matcher type warns" + (h/reset-components!) + (h/config! {:hooks {"bad" {:type "preToolCall" + :matcher 123 + :actions [{:type "shell" :shell "echo"}]}}}) + (let [warnings* (atom [])] + (with-redefs [logger/warn (fn [& args] (swap! warnings* conj args)) + f.hooks/run-shell-cmd (constantly {:exit 0 :out "" :err nil})] + (f.hooks/trigger-if-matches! :preToolCall + {:server "eca" :tool-name "write_file" :tool-input {}} + {} + (h/db) (h/config))) + (is (some #(= "Unsupported matcher type, ignoring" (second %)) @warnings*)))) + + (testing "argsMatchers not a map warns and matches nothing" + (h/reset-components!) + (h/config! {:hooks {"bad" {:type "preToolCall" + :matcher {"eca__write_file" {:argsMatchers "not-a-map"}} + :actions [{:type "shell" :shell "echo"}]}}}) + (let [warnings* (atom []) + calls* (atom [])] + (with-redefs [logger/warn (fn [& args] (swap! warnings* conj args)) + f.hooks/run-shell-cmd (fn [opts] + (swap! calls* conj opts) + {:exit 0 :out "" :err nil})] + (f.hooks/trigger-if-matches! :preToolCall + {:server "eca" :tool-name "write_file" :tool-input {}} + {} + (h/db) (h/config))) + (is (some #(= "argsMatchers must be a map, ignoring matcher entry" (second %)) @warnings*)) + (is (empty? @calls*))))) + +;;; Regression guards — negative contracts for intentional behavioral changes + +(deftest subagent-triggers-subagentStart-not-chatStart-test + (testing "subagent chat fires subagentStart, not chatStart" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"sub-1" {:agent "explorer" + :subagent {:max-steps 10}}}) + (h/config! {:hooks {"cs" {:type "chatStart" + :actions [{:type "shell" :shell "echo cs"}]} + "ss" {:type "subagentStart" + :actions [{:type "shell" :shell "echo ss"}]}}}) + (let [fired-types* (atom [])] + (with-redefs [config/all (constantly (h/config)) + config/await-plugins-resolved! (fn [] nil) + f.hooks/run-shell-cmd (fn [{:keys [input]}] + (let [data (json/parse-string input true)] + (swap! fired-types* conj (:hook_type data))) + {:exit 0 :out "" :err nil})] + (#'f.chat/run-start-hooks! {:db* (h/db*) :chat-id "sub-1" :agent "explorer" + :messenger (h/messenger)} + (h/db) false "openai/gpt")) + (is (= ["subagentStart"] @fired-types*))))) + +(deftest subagent-finish-fires-both-post-request-hooks-test + (testing "subagent finish fires postRequest then subagentPostRequest" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"sub-1" {:agent "explorer" + :subagent {:max-steps 10} + :parent-chat-id "parent-1"}}) + (h/config! {:hooks {"pr" {:type "postRequest" + :actions [{:type "shell" :shell "echo pr"}]} + "spr" {:type "subagentPostRequest" + :actions [{:type "shell" :shell "echo spr"}]}}}) + (let [fired-types* (atom [])] + (with-redefs [f.hooks/run-shell-cmd (fn [{:keys [input]}] + (let [data (json/parse-string input true)] + (swap! fired-types* conj (:hook_type data))) + {:exit 0 :out "" :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "sub-1" + :agent "explorer" + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (= ["postRequest" "subagentPostRequest"] @fired-types*))))) + +(deftest subagent-postrequest-skipped-when-postrequest-stops-test + (testing "a successful postRequest continue:false stops the turn and skips subagentPostRequest" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"sub-1" {:agent "explorer" + :subagent {:max-steps 10} + :parent-chat-id "parent-1"}}) + (h/config! {:hooks {"pr" {:type "postRequest" + :actions [{:type "shell" :shell "echo stop"}]} + "spr" {:type "subagentPostRequest" + :actions [{:type "shell" :shell "echo spr"}]}}}) + (let [fired-types* (atom [])] + (with-redefs [f.hooks/run-shell-cmd (fn [{:keys [input]}] + (let [data (json/parse-string input true)] + (swap! fired-types* conj (:hook_type data))) + ;; postRequest returns a successful continue:false + {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"halt\"}" + :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "sub-1" + :parent-chat-id "parent-1" + :agent "explorer" + :messenger (h/messenger) + :metrics (h/metrics)})) + ;; Only postRequest ran; subagentPostRequest was skipped. + (is (= ["postRequest"] @fired-types*)) + ;; The turn-stop message is still surfaced on the subagent chat. The binding + ;; (chat-id/parent-chat-id) is the contract here; the exact wording is covered + ;; by lifecycle/turn-stopped-by-hook-message-test. + (is (some (fn [m] + (and (= "sub-1" (:chat-id m)) + (= "parent-1" (:parent-chat-id m)) + (= :system (:role m)) + (string/includes? (str (get-in m [:content :text])) "halt"))) + (:chat-content-received (h/messages))))))) + +(deftest prerequest-plain-stdout-not-additional-context-test + (testing "update-pre-request-state ignores raw-output as additionalContext" + (let [state {:final-prompt "hello" + :additional-contexts [] + :stop-turn? false + :blocked? false + :stop-reason nil} + ;; Simulates a hook that outputs plain text (parsed=nil) on exit 0 + result {:parsed nil :exit 0} + new-state (#'f.chat/update-pre-request-state state result "my-hook")] + (is (empty? (:additional-contexts new-state))))) + + (testing "blank replacedPrompt/additionalContext/stopReason are treated as absent" + (let [state {:final-prompt "hello" + :additional-contexts [] + :stop-turn? false + :blocked? false + :blocked-reasons [] + :stop-reason nil + :stop-hook-name nil} + result {:parsed {"replacedPrompt" "" + "additionalContext" " " + "continue" false + "stopReason" ""} + :exit 0} + new-state (#'f.chat/update-pre-request-state state result "my-hook")] + (is (= "hello" (:final-prompt new-state))) + (is (empty? (:additional-contexts new-state))) + (is (true? (:stop-turn? new-state))) + (is (nil? (:stop-reason new-state)))))) + +(deftest prerequest-input-has-no-all-messages-test + (testing "preRequest hook input does not contain all_messages" + (h/reset-components!) + (h/config! {:hooks {"test" {:type "preRequest" + :actions [{:type "shell" :shell "cat"}]}}}) + (let [input* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (fn [{:keys [input]}] + (reset! input* (json/parse-string input true)) + {:exit 0 :out "" :err nil})] + (#'f.chat/run-pre-request-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :message "hello" + :messenger (h/messenger)})) + (is (some? @input*)) + (is (not (contains? @input* :all_messages)))))) + +(deftest prerequest-exit-2-captures-blocking-hook-name-test + (testing "a visible preRequest exit 2 records the blocking hook name for the user-facing message" + (h/reset-components!) + (h/config! {:hooks {"blocker" {:type "preRequest" + :visible true + :actions [{:type "shell" :shell "exit 2"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit f.hooks/hook-rejection-exit-code + :out nil + :err "nope"})] + (let [state (#'f.chat/run-pre-request-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :message "hello" + :messenger (h/messenger)})] + (is (match? {:blocked? true + :blocked-hook-name "blocker" + ;; visible hook: stderr stays in its block, not collected here + :blocked-reasons empty?} + state)))))) + +(deftest prerequest-continue-false-stop-reason-test + (testing "preRequest continue:false with stopReason sends canonical turn-stopped message" + (h/reset-components!) + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (#'f.chat/finish-blocked-or-stopped-pre-request! + {:db* (h/db*) :config (h/config) :chat-id "chat-1" :messenger (h/messenger)} + {:blocked? false :stop-turn? true :stop-reason "policy violation" :stop-hook-name "guard"})) + (is (some #(= "Turn stopped by hook 'guard': policy violation" (get-in % [:content :text])) @sent*))))) + +(deftest prerequest-finish-helper-noop-test + (testing "finish helper is defensive when neither stop nor block is present" + (h/reset-components!) + (let [sent* (atom []) + finished* (atom false)] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content})) + lifecycle/finish-chat-prompt-stopped! (fn [& _] + (reset! finished* true))] + (#'f.chat/finish-blocked-or-stopped-pre-request! + {:db* (h/db*) :config (h/config) :chat-id "chat-1" :messenger (h/messenger)} + {:blocked? false :stop-turn? false})) + (is (empty? @sent*)) + (is (false? @finished*))))) + +(deftest subagent-postrequest-stop-binds-to-subagent-chat-test + (testing "subagent postRequest continue:false surfaces the prefixed stop message bound to the subagent chat" + (h/reset-components!) + ;; Mark the chat as a subagent so finish runs the post-request hooks for it; + ;; the chat-ctx carries :parent-chat-id, so send-content! tags the message + ;; with it (nested under the parent in the UI). + (swap! (h/db*) assoc-in [:chats "sub-1" :subagent] {:max-steps nil}) + (h/config! {:hooks {"stopper" {:type "postRequest" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"continue\":false,\"stopReason\":\"halt subagent\"}" + :err nil})] + (lifecycle/finish-chat-prompt! :idle + {:chat-id "sub-1" + :parent-chat-id "parent-1" + :db* (h/db*) + :config (h/config) + :messenger (h/messenger) + :metrics (h/metrics)})) + (is (some (fn [m] + (and (= "sub-1" (:chat-id m)) + (= "parent-1" (:parent-chat-id m)) + (= :system (:role m)) + (string/includes? (str (get-in m [:content :text])) "halt subagent"))) + (:chat-content-received (h/messages))) + "the turn-stopped message must be bound to the subagent chat via parent-chat-id"))) + +(deftest prerequest-exit-2-no-stderr-chat-message-test + (testing "preRequest exit 2 names the blocking hook without duplicating its stderr" + (h/reset-components!) + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + (#'f.chat/finish-blocked-or-stopped-pre-request! + {:db* (h/db*) :config (h/config) :chat-id "chat-1" :messenger (h/messenger)} + {:blocked? true :stop-turn? false :stop-reason nil :blocked-hook-name "my-hook"})) + ;; Exit 2 blocks and names the hook (for visible hooks the stderr lives in + ;; the hookActionFinished block, so it is not duplicated here). + (is (some #(= "Request blocked by hook 'my-hook'." (get-in % [:content :text])) @sent*))))) + +(deftest prerequest-combined-exit-2-and-continue-false-test + (testing "combined preRequest exit 2 followed by continue:false prioritizes stop behavior" + (h/reset-components!) + (let [sent* (atom [])] + (with-redefs [lifecycle/send-content! (fn [_ role content] + (swap! sent* conj {:role role :content content}))] + ;; When both blocked? and stop-turn? are true, stop-turn? takes priority + (#'f.chat/finish-blocked-or-stopped-pre-request! + {:db* (h/db*) :config (h/config) :chat-id "chat-1" :messenger (h/messenger)} + {:blocked? true :stop-turn? true :stop-reason "explicit stop" :stop-hook-name "guard"})) + (is (some #(= "Turn stopped by hook 'guard': explicit stop" (get-in % [:content :text])) @sent*))))) + +(deftest simplified-additional-context-wrapper-test + (testing "wrap-additional-context no longer includes from attribute" + (is (= "\nextra info\n" + (lifecycle/wrap-additional-context "extra info"))))) + +(deftest followup-plain-text-test + (testing "followUp continuation is plain user text, not XML-wrapped" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"follow" {:type "postRequest" + :actions [{:type "shell" :shell "echo"}]}}}) + (let [follow-up* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"followUp\":\"Run the tests\"}" + :err nil})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :on-follow-up (fn [follow-up-text _chat-ctx] + (reset! follow-up* follow-up-text))})) + ;; followUp text should be plain, not wrapped in + (is (= "Run the tests" @follow-up*))))) + +(deftest strip-hook-callbacks-preserves-on-follow-up-test + (testing "strip-hook-callbacks preserves :on-follow-up but strips finish side-effect callbacks" + (let [chat-ctx {:on-finished-side-effect (fn []) + :on-after-finish! (fn []) + :on-follow-up (fn [_ _])} + result (lifecycle/strip-hook-callbacks chat-ctx)] + (is (not (contains? result :on-finished-side-effect))) + (is (not (contains? result :on-after-finish!))) + (is (contains? result :on-follow-up))))) + +(deftest postcompact-returns-data-test + (testing "postCompact hook runner returns data and does not mutate history directly" + (h/reset-components!) + (swap! (h/db*) assoc-in [:chats "chat-1" :messages] + [{:role "compact_marker" :content {:auto? false}} + {:role "user" :content [{:type :text :text "The conversation was compacted."}]}]) + (h/config! {:hooks {"test" {:type "postCompact" + :visible false + :actions [{:type "shell" :shell "echo"}]}}}) + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit 0 + :out "{\"additionalContext\":\"resume snapshot\"}" + :err nil})] + (let [result (lifecycle/run-post-compact-hooks! {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code"} + "manual" + "summary")] + (is (map? result)) + (is (contains? result :stop-turn?)) + (is (contains? result :additional-contexts)) + (is (= false (:stop-turn? result))) + (is (= 1 (count (:additional-contexts result)))))))) + +(deftest post-request-exit-2-as-followup-test + (testing "postRequest exit 2 with stderr treats stderr as followUp text" + (h/reset-components!) + (swap! (h/db*) assoc :chats {"chat-1" {:agent "code"}}) + (h/config! {:hooks {"test" {:type "postRequest" + :actions [{:type "shell" :shell "exit 2"}]}}}) + (let [follow-up* (atom nil)] + (with-redefs [f.hooks/run-shell-cmd (constantly {:exit f.hooks/hook-rejection-exit-code + :out nil + :err "Run the test suite"})] + (lifecycle/finish-chat-prompt! :idle {:db* (h/db*) + :config (h/config) + :chat-id "chat-1" + :agent "code" + :messenger (h/messenger) + :metrics (h/metrics) + :on-follow-up (fn [follow-up-text _chat-ctx] + (reset! follow-up* follow-up-text))})) + (is (= "Run the test suite" @follow-up*))))) diff --git a/test/eca/features/plugins_test.clj b/test/eca/features/plugins_test.clj index 8ea36b5c6..d073afdfc 100644 --- a/test/eca/features/plugins_test.clj +++ b/test/eca/features/plugins_test.clj @@ -2,6 +2,7 @@ (:require [babashka.fs :as fs] [cheshire.core :as json] + [clojure.string :as string] [clojure.test :refer [deftest is testing use-fixtures]] [eca.config :as config] [eca.features.commands :as commands] @@ -272,7 +273,9 @@ (is (= secret (get-in result [:config-fragment :quotedSecret]))) (is (= (str "node " plugin-root "/hooks/check.js") - (get-in result [:config-fragment :hooks :PostToolUse 0 :hooks 0 :command]))) + (get-in result [:config-fragment :hooks "demo::PostToolUse" 0 :hooks 0 :command]))) + (is (not-any? #(string/includes? (str %) ":::") + (keys (get-in result [:config-fragment :hooks])))) (let [loaded-commands (vec (#'commands/custom-commands {:pureConfig true :commands (:commands result)} diff --git a/test/eca/features/prompt_test.clj b/test/eca/features/prompt_test.clj index ec1e8bde0..c7a32c809 100644 --- a/test/eca/features/prompt_test.clj +++ b/test/eca/features/prompt_test.clj @@ -78,6 +78,17 @@ (is (string/includes? static "")) (is (nil? dynamic) "dynamic should be nil when no volatile contexts or MCP servers")))) +(deftest build-instructions-startup-context-test + (testing "chatStart startup-context renders even without any stable contexts" + (let [db (assoc-in (h/db) [:chats "chat-1" :startup-context] "injected by chatStart") + {:keys [static]} (build-instructions [] [] [] [] (delay "TREE") "code" {} "chat-1" [] db)] + (is (string/includes? static "")) + (is (string/includes? static "\ninjected by chatStart\n")))) + + (testing "no Contexts section when there are neither stable contexts nor startup-context" + (let [{:keys [static]} (build-instructions [] [] [] [] (delay "TREE") "code" {} "chat-1" [] (h/db))] + (is (not (string/includes? static "\ntoday is monday\n"}]}] + {:type :text :text "\ntoday is monday\n"}]}] (#'llm-providers.anthropic/normalize-messages [{:role "user" :content [{:type :text :text "compact the chat"} - {:type :text :text "\ntoday is monday\n"}]}] + {:type :text :text "\ntoday is monday\n"}]}] true))))) (deftest server-web-search-full-pipeline-test diff --git a/test/eca/shared_test.clj b/test/eca/shared_test.clj index e866ce01f..44fe79370 100644 --- a/test/eca/shared_test.clj +++ b/test/eca/shared_test.clj @@ -1,7 +1,7 @@ (ns eca.shared-test (:require [babashka.fs :as fs] - [clojure.test :refer [deftest is testing]] + [clojure.test :refer [are deftest is testing]] [eca.shared :as shared] [eca.test-helper :as h] [matcher-combinators.test :refer [match?]])) @@ -293,8 +293,8 @@ (deftest compact-side-effect-clears-validated-path-rules-test (let [db* (atom {:chats {"chat-1" {:last-summary "Short summary" - :messages [] - :validated-path-rules #{"/workspace/a/.eca/rules/format.md"}}}})] + :messages [] + :validated-path-rules #{"/workspace/a/.eca/rules/format.md"}}}})] (shared/compact-side-effect! {:chat-id "chat-1" :full-model "openai/gpt-5.2" :db* db* @@ -335,3 +335,12 @@ (testing "handles empty messages" (is (= [] (shared/messages-after-last-compact-marker []))))) + +(deftest not-blank-test + ;; blank/non-string values are absent; real strings pass through + (are [in expected] (= expected (shared/not-blank in)) + "" nil + " " nil + nil nil + 42 nil + "hello" "hello"))