Merge remote-tracking branch 'origin/main' into feat/PLAN-responses-api

# Conflicts:
#	src/client/common.rs
This commit is contained in:
Dark-Alex-17 committed 2026-09-16 14:33:29 -06:00
commit 717a48bef2
32 files changed
+6351 -62

No files matched your search

+4
View File
@@ -369,6 +369,10 @@ instructions: |
The direct `adversary`/probe spawns below are the FALLBACK when the gauntlet is not installed;
on that path an adversary/probe result that is ONLY a `PIPELINE-FAULT` (no verdict on the
code) is likewise an infrastructure fault to retry, never a divergence.
Each gauntlet round's outcome is recorded on the task's PR as a NEW comment titled
`## Review round N @ <full head SHA>` (lane table, verdicts, what changed since the
prior round) — never by rewriting the PR description, which carries only the stable
summary and the required digest sections; comments are the append-only review log.
- **Spawn `adversary`** with the diff base and the criteria pasted in:
```
+10
View File
@@ -697,6 +697,16 @@ instructions: |
and what changed (including any plan amendment). Zero autonomous decisions → omit the section.
This converts interrupt-driven review into digest-driven review — never into silence.
**The PR review log (MANDATORY when you author a PR).** The PR DESCRIPTION is a stable
document: what the PR does, how to verify it, and the required digest sections (autonomous
decisions, skipped reviews, observability). Every review round's evidence — the gauntlet
lane table and verdicts, the gates you ran, rulings recorded that round, and what changed
since the previous round — is posted as a NEW PR comment (`gh pr comment`) titled
`## Review round N @ <full head SHA>`. NEVER record round evidence by rewriting the
description: in-place edits destroy the review history and hide which commits each round
actually reviewed. Comments are the append-only review log; the SHA in the title names
exactly what was reviewed.
**Close the loop.** When a decision DID reach the user (step 4), suggest in your final report
that they record the resolution with `coyote --macro record-ruling` so that class of decision
never asks again.
+12
View File
@@ -149,6 +149,18 @@ mcp_tools: null # Per-server MCP tool allowlists (glob patterns
# - list_*
# slack: []
# ---- Hooks ----
# Fire-and-forget external commands that run when lifecycle events fire. Each event name
# maps to a list of named hooks; every listed hook's command runs when that event fires.
# Hooks never block coyote and never change its output or exit code.
# See the [Hooks documentation](https://github.com/Dark-Alex-17/coyote/wiki/Hooks) for more details
hooks: {} # Event -> list of named hook commands (default: none)
# Example:
# hooks:
# turn.completed:
# - name: notify
# command: ./hooks/notify.sh
# ---- Auto-Continue (Todo System) ----
# The auto-continue system provides built-in task tracking for improved reliability.
# When enabled, the model can create todo lists and the system will automatically
+40
View File
@@ -0,0 +1,40 @@
#!/usr/bin/env bash
# Event-logging hook: appends a timestamped snapshot of every COYOTE_* env var
# to a log file, so you can see exactly which events fire and what context
# each one carries.
#
# Env vars read:
# COYOTE_HOOK_LOG log file to append to (default: /tmp/coyote-hooks.log)
# COYOTE_* everything coyote sets for the event is dumped verbatim
#
# Secrets (COYOTE_SECRET_*) are excluded from the snapshot; widening the grep
# filter below opts into logging them.
#
# Wire it up under `hooks:` in your config.yaml (any event works):
# hooks:
# tool.completed:
# - name: log-events
# command: ./hooks/log-events.sh
#
# A relative command path like ./hooks/log-events.sh resolves against the
# coyote config directory for global hooks, so this works with the default
# hooks dir.
#
# See https://github.com/Dark-Alex-17/coyote/wiki/Hooks for the full list of
# events and env vars. Hooks are fire-and-forget: this script never fails
# loudly, and coyote ignores its exit code either way.
set -u
log_file="${COYOTE_HOOK_LOG:-/tmp/coyote-hooks.log}"
# The default lives in shared /tmp: refuse to append through a symlink someone
# else planted there. Point COYOTE_HOOK_LOG at a private path to avoid the
# shared directory entirely.
[ -L "$log_file" ] && exit 0
umask 077
{
echo "=== $(date '+%Y-%m-%dT%H:%M:%S%z') ${COYOTE_EVENT:-unknown}"
env | grep '^COYOTE_' | grep -v '^COYOTE_SECRET_' | sort
echo
} >> "$log_file" 2> /dev/null || true
+42
View File
@@ -0,0 +1,42 @@
#!/usr/bin/env bash
# Desktop-notification hook: posts a notification describing the event that
# fired, falling back to a message on the controlling terminal when no
# notifier is present.
#
# Env vars read (set by coyote when the hook fires):
# COYOTE_EVENT dotted event name, e.g. turn.completed
# COYOTE_AGENT_NAME agent name, when the event carries one
# COYOTE_TOOL_NAME tool name, when the event carries one
#
# Wire it up under `hooks:` in your config.yaml:
# hooks:
# turn.completed:
# - name: notify
# command: ./hooks/notify.sh
#
# A relative command path like ./hooks/notify.sh resolves against the coyote
# config directory for global hooks, so this works with the default hooks dir.
#
# See https://github.com/Dark-Alex-17/coyote/wiki/Hooks for the full list of
# events and env vars. Hooks are fire-and-forget: this script never fails
# loudly, and coyote ignores its exit code either way.
set -u
title="coyote"
body="${COYOTE_EVENT:-event}"
[ -n "${COYOTE_AGENT_NAME:-}" ] && body="$body agent=$COYOTE_AGENT_NAME"
[ -n "${COYOTE_TOOL_NAME:-}" ] && body="$body tool=$COYOTE_TOOL_NAME"
if command -v notify-send > /dev/null 2>&1; then
notify-send "$title" "$body" 2> /dev/null || true
elif command -v osascript > /dev/null 2>&1; then
# $body carries model-influenced values (e.g. tool names); strip quotes and
# backslashes so it cannot escape the AppleScript string literal.
safe_body=${body//[\"\\]/}
osascript -e "display notification \"$safe_body\" with title \"$title\"" 2> /dev/null || true
else
# coyote discards hook stdout, so aim the fallback at the controlling
# terminal when one exists; plain stdout keeps direct/manual runs working.
{ echo "[$title] $body" > /dev/tty; } 2> /dev/null || echo "[$title] $body"
fi
+12
View File
@@ -206,6 +206,18 @@ enabled_macros: null # Which macros are invocable by default (no ro
# Example (comma-separated form):
# enabled_macros: generate-commit-message,review
# ---- Hooks ----
# Fire-and-forget external commands that run when lifecycle events fire. Each event name
# maps to a list of named hooks; every listed hook's command runs when that event fires.
# Hooks never block coyote and never change its output or exit code.
# See the [Hooks documentation](https://github.com/Dark-Alex-17/coyote/wiki/Hooks) for more details
hooks: {} # Event -> list of named hook commands (default: none)
# Example:
# hooks:
# turn.completed:
# - name: notify
# command: ./hooks/notify.sh
# ---- Auto-Continue (Todo System) ----
# The auto-continue system provides built-in task tracking for improved reliability.
# When enabled, the model can create todo lists and the system will automatically
+232
View File
@@ -0,0 +1,232 @@
#!/usr/bin/env bash
# Black-box usage-pattern probe for the coyote hook system.
#
# Verifies, against a REAL built `coyote` binary in a clean/isolated
# COYOTE_CONFIG_DIR (dry_run:true, no network/LLM credentials required):
# A) exactly one turn.started + one turn.completed + one llm.request.completed
# fire per headless run, with correctly scoped env vars (no cross-event
# extras leakage).
# B) a failing/missing-script hook never changes coyote's stdout, stderr, or
# exit code.
# C) `coyote --install-builtins hooks` installs both bundled example scripts
# 0755 into <config_dir>/hooks via the real CLI, and a second invocation
# (force reinstall) overwrites local edits back to the packaged content.
# D) `coyote --agent <bundled-agent> --info` (an inspection flag) fires ZERO
# agent.* hook events.
#
# Not covered here (see src/function/mod.rs tests for tool.* + payload-file
# coverage instead): a live tool.started firing requires a real LLM round
# trip that actually decides to call a tool; dry_run's echo_messages() path
# never invokes tools, so it cannot be exercised via this recipe.
#
# Usage: scripts/usage-probe-hooks.sh
# Requires: a `coyote` binary on PATH or at ./target/debug/coyote or
# ./target/release/coyote (build with `cargo build --bin coyote` first).
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
BIN="${COYOTE_BIN:-}"
if [ -z "$BIN" ]; then
if [ -x "$REPO_ROOT/target/debug/coyote" ]; then
BIN="$REPO_ROOT/target/debug/coyote"
elif [ -x "$REPO_ROOT/target/release/coyote" ]; then
BIN="$REPO_ROOT/target/release/coyote"
else
echo "No coyote binary found; run 'cargo build --bin coyote' first." >&2
exit 2
fi
fi
WORKDIR="$(mktemp -d)"
trap 'rm -rf "$WORKDIR"' EXIT
fail() {
echo "FAIL: $1" >&2
exit 1
}
write_dryrun_config() {
# $1 = config dir, $2..$n = extra YAML fragment lines appended verbatim
local dir="$1"
shift
cat > "$dir/config.yaml" <<'BASE'
model: dryrun:dry-model
dry_run: true
clients:
- type: openai
name: dryrun
auth: none
api_key: 'unused'
models:
- name: dry-model
max_input_tokens: 100000
supports_function_calling: true
save: false
memory: false
stream: false
BASE
if [ "$#" -gt 0 ]; then
printf '%s\n' "$@" >> "$dir/config.yaml"
fi
}
echo "== Scenario A: turn/llm hook cadence and env scoping =="
CFG_A="$WORKDIR/a"
mkdir -p "$CFG_A"
TS_LOG="$WORKDIR/turn-started.log"
TC_LOG="$WORKDIR/turn-completed.log"
LC_LOG="$WORKDIR/llm-completed.log"
write_dryrun_config "$CFG_A" "hooks:" \
" turn.started:" \
" - name: probe" \
" command: \"{ echo \\\"EVENT=\$COYOTE_EVENT NAME=\$COYOTE_HOOK_NAME\\\"; } >> $TS_LOG\"" \
" turn.completed:" \
" - name: probe" \
" command: \"{ echo \\\"EVENT=\$COYOTE_EVENT NAME=\$COYOTE_HOOK_NAME\\\"; } >> $TC_LOG\"" \
" llm.request.completed:" \
" - name: probe-llm" \
" command: \"{ echo \\\"EVENT=\$COYOTE_EVENT PROVIDER=\$COYOTE_LLM_PROVIDER MODEL=\$COYOTE_LLM_MODEL\\\"; } >> $LC_LOG\""
COYOTE_CONFIG_DIR="$CFG_A" "$BIN" --no-stream "say exactly: hi" > /dev/null
sleep 1
[ "$(grep -c '^EVENT=turn.started' "$TS_LOG" 2>/dev/null || echo 0)" = "1" ] \
|| fail "expected exactly one turn.started, got: $(cat "$TS_LOG" 2>/dev/null)"
[ "$(grep -c '^EVENT=turn.completed' "$TC_LOG" 2>/dev/null || echo 0)" = "1" ] \
|| fail "expected exactly one turn.completed, got: $(cat "$TC_LOG" 2>/dev/null)"
[ "$(grep -c '^EVENT=llm.request.completed' "$LC_LOG" 2>/dev/null || echo 0)" = "1" ] \
|| fail "expected exactly one llm.request.completed, got: $(cat "$LC_LOG" 2>/dev/null)"
grep -q 'PROVIDER=dryrun MODEL=dryrun:dry-model' "$LC_LOG" \
|| fail "llm.request.completed missing expected COYOTE_LLM_PROVIDER/MODEL: $(cat "$LC_LOG")"
echo "PASS: exactly 1 turn.started + 1 turn.completed + 1 llm.request.completed, correctly scoped"
echo "== Scenario B: failing/missing-script hooks never affect output or exit code =="
CFG_B="$WORKDIR/b"
mkdir -p "$CFG_B"
write_dryrun_config "$CFG_B" "hooks:" \
" turn.completed:" \
" - name: failing" \
" command: /nonexistent/script/does/not/exist" \
" - name: exit1" \
" command: \"exit 1\""
CFG_BASE="$WORKDIR/base"
mkdir -p "$CFG_BASE"
write_dryrun_config "$CFG_BASE"
exit_b=0
COYOTE_CONFIG_DIR="$CFG_B" "$BIN" --no-stream "say exactly: hi" > "$WORKDIR/out_b.txt" 2> "$WORKDIR/err_b.txt" || exit_b=$?
exit_base=0
COYOTE_CONFIG_DIR="$CFG_BASE" "$BIN" --no-stream "say exactly: hi" > "$WORKDIR/out_base.txt" 2> "$WORKDIR/err_base.txt" || exit_base=$?
[ "$exit_b" = "$exit_base" ] || fail "exit code differs: with-hooks=$exit_b baseline=$exit_base"
diff -q "$WORKDIR/out_b.txt" "$WORKDIR/out_base.txt" > /dev/null || fail "stdout differs when a hook fails/is missing"
diff -q "$WORKDIR/err_b.txt" "$WORKDIR/err_base.txt" > /dev/null || fail "stderr differs when a hook fails/is missing"
echo "PASS: failing/missing-script hooks are invisible to stdout/stderr/exit code"
echo "== Scenario C: coyote --install-builtins hooks (real CLI, clean config dir) =="
CFG_C="$WORKDIR/c"
mkdir -p "$CFG_C"
out_install1="$WORKDIR/install1.out"
COYOTE_CONFIG_DIR="$CFG_C" "$BIN" --install-builtins hooks > "$out_install1" 2>&1
[ -f "$CFG_C/hooks/notify.sh" ] || fail "notify.sh was not installed: $(cat "$out_install1")"
[ -f "$CFG_C/hooks/log-events.sh" ] || fail "log-events.sh was not installed: $(cat "$out_install1")"
grep -q "Reinstalled bundled hooks" "$out_install1" \
|| fail "expected confirmation message, got: $(cat "$out_install1")"
if command -v stat > /dev/null 2>&1; then
mode_notify="$(stat -c '%a' "$CFG_C/hooks/notify.sh" 2>/dev/null || stat -f '%Lp' "$CFG_C/hooks/notify.sh")"
mode_log="$(stat -c '%a' "$CFG_C/hooks/log-events.sh" 2>/dev/null || stat -f '%Lp' "$CFG_C/hooks/log-events.sh")"
[ "$mode_notify" = "755" ] || fail "notify.sh mode is $mode_notify, expected 755"
[ "$mode_log" = "755" ] || fail "log-events.sh mode is $mode_log, expected 755"
fi
# Modify a script locally, then re-run --install-builtins hooks: it always
# forces (no confirmation prompt on non-tty stdout), so the local edit must
# be overwritten back to the packaged content.
echo "# LOCALLY MODIFIED" > "$CFG_C/hooks/notify.sh"
COYOTE_CONFIG_DIR="$CFG_C" "$BIN" --install-builtins hooks > "$WORKDIR/install2.out" 2>&1
grep -q "LOCALLY MODIFIED" "$CFG_C/hooks/notify.sh" \
&& fail "force reinstall did not overwrite the locally modified notify.sh"
grep -q '^#!/usr/bin/env bash' "$CFG_C/hooks/notify.sh" \
|| fail "notify.sh was not restored to the packaged script"
echo "PASS: --install-builtins hooks installs both scripts 0755 and force-overwrites local edits"
echo "== Scenario C2: cold-start auto-bootstrap fills the hooks dir but never forces =="
CFG_C2="$WORKDIR/c2"
mkdir -p "$CFG_C2/hooks"
write_dryrun_config "$CFG_C2"
# Every coyote invocation calls install_builtins() (force=false) before
# dispatching any flag, so a plain run on a config dir with no hooks/ yet
# must auto-populate it...
rmdir "$CFG_C2/hooks" 2>/dev/null || true
COYOTE_CONFIG_DIR="$CFG_C2" "$BIN" --no-stream "say exactly: hi" > /dev/null
[ -f "$CFG_C2/hooks/notify.sh" ] || fail "cold-start run did not auto-install notify.sh"
# ...but once a file exists, the non-force path must NEVER touch it again.
echo "# LOCALLY MODIFIED" > "$CFG_C2/hooks/notify.sh"
COYOTE_CONFIG_DIR="$CFG_C2" "$BIN" --no-stream "say exactly: hi" > /dev/null
grep -q "LOCALLY MODIFIED" "$CFG_C2/hooks/notify.sh" \
|| fail "a plain (non --install-builtins) invocation overwrote a locally modified hook script"
echo "PASS: plain invocations auto-bootstrap missing hooks but never force-overwrite existing ones"
echo "== Scenario C3: installed log-events.sh fires end-to-end via the documented relative path =="
# Exercises the exact wiring promised in the script headers:
# command: ./hooks/log-events.sh (resolves against the config dir)
# and the script's own contract: COYOTE_HOOK_LOG override honored, log file
# created 0600, COYOTE_SECRET_* values excluded from the snapshot.
CFG_C3="$WORKDIR/c3"
mkdir -p "$CFG_C3"
HOOK_LOG="$WORKDIR/c3-hook.log"
write_dryrun_config "$CFG_C3" "hooks:" \
" turn.completed:" \
" - name: log-events" \
" command: ./hooks/log-events.sh"
COYOTE_CONFIG_DIR="$CFG_C3" "$BIN" --install-builtins hooks > /dev/null 2>&1
COYOTE_CONFIG_DIR="$CFG_C3" COYOTE_HOOK_LOG="$HOOK_LOG" COYOTE_SECRET_PROBE=hunter2 \
"$BIN" --no-stream "say exactly: hi" > /dev/null
sleep 1
[ -f "$HOOK_LOG" ] || fail "installed log-events.sh did not write to COYOTE_HOOK_LOG via relative-path wiring"
[ "$(grep -c '^=== ' "$HOOK_LOG")" = "1" ] \
|| fail "expected exactly one event block in the hook log, got: $(cat "$HOOK_LOG")"
grep -q 'COYOTE_EVENT=turn.completed' "$HOOK_LOG" \
|| fail "hook log missing COYOTE_EVENT=turn.completed: $(cat "$HOOK_LOG")"
grep -q 'hunter2' "$HOOK_LOG" \
&& fail "COYOTE_SECRET_* value leaked into the hook log"
grep -q 'COYOTE_SECRET_' "$HOOK_LOG" \
&& fail "COYOTE_SECRET_ variable name leaked into the hook log"
if command -v stat > /dev/null 2>&1; then
mode_hook_log="$(stat -c '%a' "$HOOK_LOG" 2>/dev/null || stat -f '%Lp' "$HOOK_LOG")"
[ "$mode_hook_log" = "600" ] || fail "hook log mode is $mode_hook_log, expected 600"
fi
echo "PASS: relative-path wiring fires the installed log-events.sh; log is 0600 with secrets excluded"
echo "== Scenario D: --agent <bundled> --info fires zero agent.* events =="
CFG_D="$WORKDIR/d"
mkdir -p "$CFG_D/agents"
AGENT_LOG="$WORKDIR/agent-events.log"
write_dryrun_config "$CFG_D" "hooks:" \
" agent.started:" \
" - name: probe-agent" \
" command: \"echo AGENT_STARTED >> $AGENT_LOG\"" \
" agent.completed:" \
" - name: probe-agent" \
" command: \"echo AGENT_DONE >> $AGENT_LOG\"" \
" agent.failed:" \
" - name: probe-agent" \
" command: \"echo AGENT_FAILED >> $AGENT_LOG\""
AGENT_NAME="$(ls "$REPO_ROOT/assets/agents" | head -1)"
cp -r "$REPO_ROOT/assets/agents/$AGENT_NAME" "$CFG_D/agents/$AGENT_NAME"
COYOTE_CONFIG_DIR="$CFG_D" "$BIN" --agent "$AGENT_NAME" --info > /dev/null 2>&1
sleep 1
[ -s "$AGENT_LOG" ] && fail "expected zero agent.* events from --agent --info, got: $(cat "$AGENT_LOG")"
echo "PASS: --agent $AGENT_NAME --info fired zero agent.* hook events"
echo
echo "ALL SCENARIOS PASSED"
+10
View File
@@ -609,6 +609,10 @@ mod tests {
parse(&["--install-builtins", "mcp_config"]).install_builtins,
Some(AssetCategory::McpConfig)
);
assert_eq!(
parse(&["--install-builtins", "hooks"]).install_builtins,
Some(AssetCategory::Hooks)
);
}
#[test]
@@ -620,12 +624,14 @@ mod tests {
assert!(category.get_name_and_aliases().any(|n| n == "mcp_config"));
assert!(AssetCategory::NAMES.contains(&"mcp-config"));
assert!(!AssetCategory::NAMES.contains(&"mcp_config"));
assert!(AssetCategory::NAMES.contains(&"hooks"));
let filter = InstallFilter::McpConfig.to_possible_value().unwrap();
assert_eq!(filter.get_name(), "mcp-config");
assert!(filter.get_name_and_aliases().any(|n| n == "mcp_config"));
assert!(InstallFilter::NAMES.contains(&"mcp-config"));
assert!(!InstallFilter::NAMES.contains(&"mcp_config"));
assert!(InstallFilter::NAMES.contains(&"hooks"));
}
#[test]
@@ -685,6 +691,10 @@ mod tests {
.filter,
Some(InstallFilter::McpConfig)
);
assert_eq!(
parse(&["--install", "https://github.com/x/y", "--filter", "hooks"]).filter,
Some(InstallFilter::Hooks)
);
}
#[test]
+497 -9
View File
@@ -2,6 +2,7 @@ use super::*;
use super::access_token::{distrust_access_token, get_access_token};
use crate::config::{RenderMode, paths};
use crate::hooks::{self, HookEvent};
use crate::{
config::{AppConfig, Input, RequestContext},
function::{FunctionDeclaration, ToolCall, ToolResult, eval_tool_calls},
@@ -20,7 +21,7 @@ use reqwest::{Client as ReqwestClient, RequestBuilder};
use serde::{Deserialize, Serialize};
use serde_json::{Value, json};
use std::sync::LazyLock;
use std::time::Duration;
use std::time::{Duration, Instant};
use tokio::sync::mpsc::unbounded_channel;
pub const MODELS_YAML: &str = include_str!("../../models.yaml");
@@ -580,6 +581,47 @@ pub async fn create_openai_compatible_client_config(
Ok(Some((model, clients)))
}
fn llm_request_extras(client: &dyn Client) -> Vec<(&'static str, String)> {
vec![
("COYOTE_LLM_PROVIDER", client.name().to_string()),
("COYOTE_LLM_MODEL", client.model().id()),
]
}
fn llm_completed_extras(client: &dyn Client, started_at: Instant) -> Vec<(&'static str, String)> {
let mut extras = llm_request_extras(client);
extras.push((
"COYOTE_LLM_DURATION_MS",
started_at.elapsed().as_millis().to_string(),
));
extras
}
fn llm_failed_extras(
client: &dyn Client,
err: Option<&anyhow::Error>,
aborted: bool,
) -> Vec<(&'static str, String)> {
let mut extras = llm_request_extras(client);
if aborted {
extras.push(("COYOTE_LLM_ERROR_KIND", "aborted".to_string()));
} else if let Some(api_err) = err.and_then(|err| err.downcast_ref::<ApiStatusError>()) {
extras.push(("COYOTE_LLM_ERROR_KIND", "api_error".to_string()));
extras.push(("COYOTE_LLM_STATUS", api_err.status.to_string()));
} else {
extras.push(("COYOTE_LLM_ERROR_KIND", "other".to_string()));
}
extras.push((
"COYOTE_LLM_ERROR",
// `{:#}` renders the whole context chain, so the root cause survives
// the `context(...)` wrapping the client trait defaults apply.
err.map(|err| format!("{err:#}"))
.unwrap_or_else(|| "Aborted.".to_string()),
));
extras
}
pub async fn call_chat_completions(
input: &Input,
print: bool,
@@ -591,15 +633,28 @@ pub async fn call_chat_completions(
let is_child_agent = ctx.current_depth > 0;
let suppress_spinner = is_child_agent || ctx.render_mode == RenderMode::Silent;
let spinner_message = if suppress_spinner { "" } else { "Generating" };
hooks::fire(
HookEvent::LlmRequestStarted,
ctx,
&llm_request_extras(client),
None,
);
let request_started_at = Instant::now();
let ret = abortable_run_with_spinner(
client.chat_completions(input.clone()),
spinner_message,
abort_signal,
abort_signal.clone(),
)
.await;
match ret {
Ok(ret) => {
hooks::fire(
HookEvent::LlmRequestCompleted,
ctx,
&llm_completed_extras(client, request_started_at),
None,
);
let ChatCompletionsOutput {
mut text,
tool_calls,
@@ -617,7 +672,15 @@ pub async fn call_chat_completions(
ctx.record_token_usage(usage, client.model());
finish_completion(ctx, text, tool_calls, thinking).await
}
Err(err) => Err(err),
Err(err) => {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, Some(&err), abort_signal.aborted()),
None,
);
Err(err)
}
}
}
@@ -634,6 +697,13 @@ pub async fn call_chat_completions_streaming(
handler.set_silent(true);
}
hooks::fire(
HookEvent::LlmRequestStarted,
ctx,
&llm_request_extras(client),
None,
);
let request_started_at = Instant::now();
let (send_ret, render_ret) = tokio::join!(
client.chat_completions_streaming(input, &mut handler),
render_stream(rx, client.app_config(), abort_signal.clone(), silent),
@@ -649,12 +719,32 @@ pub async fn call_chat_completions_streaming(
ctx.record_token_usage(usage, client.model());
if aborted_ctrld {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, None, true),
None,
);
bail!("Aborted.");
}
render_ret?;
if let Err(err) = render_ret {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, Some(&err), false),
None,
);
return Err(err);
}
if aborted_ctrlc {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, None, true),
None,
);
if !ctx.working_mode.is_repl() || ctx.session.is_none() {
bail!("Aborted.");
}
@@ -678,6 +768,12 @@ pub async fn call_chat_completions_streaming(
match send_ret {
Ok(_) => {
hooks::fire(
HookEvent::LlmRequestCompleted,
ctx,
&llm_completed_extras(client, request_started_at),
None,
);
if !silent && !text.is_empty() && !text.ends_with('\n') {
println!();
}
@@ -687,6 +783,12 @@ pub async fn call_chat_completions_streaming(
if !silent && !text.is_empty() {
println!();
}
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, Some(&err), false),
None,
);
Err(err)
}
}
@@ -709,10 +811,36 @@ pub async fn call_chat_completions_streaming_quiet(
ctx: &mut RequestContext,
abort_signal: AbortSignal,
) -> Result<(String, Vec<ToolResult>)> {
hooks::fire(
HookEvent::LlmRequestStarted,
ctx,
&llm_request_extras(client),
None,
);
let request_started_at = Instant::now();
if client.model().no_stream() {
let output = tokio::select! {
ret = client.chat_completions(input.clone()) => ret?,
_ = wait_abort_signal(&abort_signal) => bail!("Aborted."),
ret = client.chat_completions(input.clone()) => match ret {
Ok(output) => output,
Err(err) => {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, Some(&err), false),
None,
);
return Err(err);
}
},
_ = wait_abort_signal(&abort_signal) => {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, None, true),
None,
);
bail!("Aborted.")
}
};
let ChatCompletionsOutput {
text,
@@ -722,8 +850,20 @@ pub async fn call_chat_completions_streaming_quiet(
} = output;
ctx.record_token_usage(usage, client.model());
if abort_signal.aborted() {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, None, true),
None,
);
bail!("Aborted.");
}
hooks::fire(
HookEvent::LlmRequestCompleted,
ctx,
&llm_completed_extras(client, request_started_at),
None,
);
return finish_completion(ctx, text, tool_calls, thinking).await;
}
@@ -739,10 +879,30 @@ pub async fn call_chat_completions_streaming_quiet(
ctx.record_token_usage(usage, client.model());
if abort_signal.aborted() {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, None, true),
None,
);
bail!("Aborted.");
}
send_ret?;
if let Err(err) = send_ret {
hooks::fire(
HookEvent::LlmRequestFailed,
ctx,
&llm_failed_extras(client, Some(&err), false),
None,
);
return Err(err);
}
hooks::fire(
HookEvent::LlmRequestCompleted,
ctx,
&llm_completed_extras(client, request_started_at),
None,
);
finish_completion(ctx, text, tool_calls, thinking).await
}
@@ -1003,6 +1163,7 @@ mod tests {
use super::super::access_token::{is_rejected, set_access_token};
use crate::config::{AppState, WorkingMode};
use anyhow::Error;
use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering};
use tokio::time::Instant;
@@ -1219,8 +1380,8 @@ mod tests {
/// Wrapped in `.context(...)` so every test below proves the downcast
/// works through an anyhow context chain, as in the trait methods.
fn api_status_error(status: u16) -> anyhow::Error {
anyhow::Error::new(ApiStatusError {
fn api_status_error(status: u16) -> Error {
Error::new(ApiStatusError {
status,
message: format!("error (status: {status})"),
})
@@ -1559,6 +1720,333 @@ mod tests {
assert_paced_reply(&text, &results);
}
/// Fake provider whose every request fails with an [`ApiStatusError`],
/// wrapped in the same context chain the real trait defaults produce.
struct FailingStatusClient {
config: AppConfig,
model: Model,
status: u16,
}
impl FailingStatusClient {
fn new(status: u16) -> Self {
Self {
config: AppConfig::default(),
model: Model::default(),
status,
}
}
fn error(&self) -> Error {
Error::new(ApiStatusError {
status: self.status,
message: format!("error (status: {})", self.status),
})
}
}
#[async_trait::async_trait]
impl Client for FailingStatusClient {
fn app_config(&self) -> &AppConfig {
&self.config
}
fn extra_config(&self) -> Option<&ExtraConfig> {
None
}
fn patch_config(&self) -> Option<&RequestPatch> {
None
}
fn name(&self) -> &str {
"failing-status"
}
fn model(&self) -> &Model {
&self.model
}
async fn chat_completions_inner(
&self,
_client: &ReqwestClient,
_data: ChatCompletionsData,
) -> Result<ChatCompletionsOutput> {
Err(self.error())
}
async fn chat_completions_streaming_inner(
&self,
_client: &ReqwestClient,
_handler: &mut SseHandler,
_data: ChatCompletionsData,
) -> Result<()> {
Err(self.error())
}
}
fn llm_hooks_map(marker: &str) -> hooks::HooksMap {
[
"llm.request.started",
"llm.request.completed",
"llm.request.failed",
]
.into_iter()
.map(|event| {
(
event.to_string(),
vec![crate::hooks::HookDef {
name: marker.to_string(),
command: "true".to_string(),
}],
)
})
.collect()
}
fn llm_hooked_ctx(marker: &str) -> RequestContext {
let mut app = AppState::test_default();
app.config = Arc::new(AppConfig {
hooks: llm_hooks_map(marker),
..Default::default()
});
let mut ctx = RequestContext::new(Arc::new(app), WorkingMode::Cmd);
ctx.render_mode = RenderMode::Silent;
ctx
}
fn llm_captures(marker: &str) -> Vec<hooks::test_sink::Capture> {
hooks::test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == marker)
.collect()
}
#[tokio::test(start_paused = true)]
#[serial_test::serial]
async fn plain_transport_fires_llm_request_started_and_completed() {
let marker = "llm-hooks-plain-ok-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let mut client = paced_client(false);
client.nonstream_replies = true;
let _guard = hooks::test_sink::install();
call_chat_completions(
&input,
false,
false,
&client,
&mut ctx,
create_abort_signal(),
)
.await
.unwrap();
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::LlmRequestStarted);
assert_eq!(
captures[0]
.envs
.get("COYOTE_LLM_PROVIDER")
.map(String::as_str),
Some("paced-stream")
);
assert!(captures[0].envs.contains_key("COYOTE_LLM_MODEL"));
assert_eq!(captures[1].event, HookEvent::LlmRequestCompleted);
assert!(captures[1].envs.contains_key("COYOTE_LLM_DURATION_MS"));
assert!(!captures[1].envs.contains_key("COYOTE_LLM_ERROR_KIND"));
}
#[tokio::test]
#[serial_test::serial]
async fn plain_transport_fires_llm_request_failed_with_api_error_kind() {
let marker = "llm-hooks-plain-api-err-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = FailingStatusClient::new(429);
let _guard = hooks::test_sink::install();
let err = call_chat_completions(
&input,
false,
false,
&client,
&mut ctx,
create_abort_signal(),
)
.await
.unwrap_err();
assert!(
err.downcast_ref::<ApiStatusError>().is_some(),
"hooks must not change the error: {err:?}"
);
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::LlmRequestStarted);
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::LlmRequestFailed);
assert_eq!(
failed.envs.get("COYOTE_LLM_ERROR_KIND").map(String::as_str),
Some("api_error")
);
assert_eq!(
failed.envs.get("COYOTE_LLM_STATUS").map(String::as_str),
Some("429")
);
assert!(
failed
.envs
.get("COYOTE_LLM_ERROR")
.unwrap()
.contains("error (status: 429)")
);
assert!(!failed.envs.contains_key("COYOTE_LLM_DURATION_MS"));
}
#[tokio::test(start_paused = true)]
#[serial_test::serial]
async fn streaming_transport_fires_llm_request_started_and_completed() {
let marker = "llm-hooks-stream-ok-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = paced_client(false);
let _guard = hooks::test_sink::install();
call_chat_completions_streaming(&input, &client, &mut ctx, create_abort_signal())
.await
.unwrap();
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::LlmRequestStarted);
assert_eq!(captures[1].event, HookEvent::LlmRequestCompleted);
assert!(captures[1].envs.contains_key("COYOTE_LLM_DURATION_MS"));
}
#[tokio::test(start_paused = true)]
#[serial_test::serial]
async fn streaming_transport_abort_fires_llm_request_failed_aborted() {
let marker = "llm-hooks-stream-abort-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = paced_client(true);
let abort = create_abort_signal();
let trigger = abort.clone();
tokio::spawn(async move {
tokio::time::sleep(PACED_GAP * PACED_DELTAS as u32 + Duration::from_secs(30)).await;
trigger.set_ctrlc();
});
let _guard = hooks::test_sink::install();
let err = call_chat_completions_streaming(&input, &client, &mut ctx, abort)
.await
.expect_err("an aborted stream must not succeed");
assert_eq!(err.to_string(), "Aborted.");
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::LlmRequestFailed);
assert_eq!(
failed.envs.get("COYOTE_LLM_ERROR_KIND").map(String::as_str),
Some("aborted")
);
assert!(!failed.envs.contains_key("COYOTE_LLM_STATUS"));
}
#[tokio::test(start_paused = true)]
#[serial_test::serial]
async fn quiet_transport_fires_llm_request_started_and_completed() {
let marker = "llm-hooks-quiet-ok-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = paced_client(false);
let _guard = hooks::test_sink::install();
call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal())
.await
.unwrap();
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::LlmRequestStarted);
assert_eq!(
captures[0]
.envs
.get("COYOTE_LLM_PROVIDER")
.map(String::as_str),
Some("paced-stream")
);
assert_eq!(captures[1].event, HookEvent::LlmRequestCompleted);
assert!(captures[1].envs.contains_key("COYOTE_LLM_DURATION_MS"));
}
#[tokio::test(start_paused = true)]
#[serial_test::serial]
async fn quiet_transport_abort_fires_llm_request_failed_aborted() {
let marker = "llm-hooks-quiet-abort-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = paced_client(true);
let abort = create_abort_signal();
let trigger = abort.clone();
tokio::spawn(async move {
tokio::time::sleep(PACED_GAP * PACED_DELTAS as u32 + Duration::from_secs(30)).await;
trigger.set_ctrlc();
});
let _guard = hooks::test_sink::install();
let err = call_chat_completions_streaming_quiet(&input, &client, &mut ctx, abort)
.await
.expect_err("an aborted stream must not succeed");
assert_eq!(err.to_string(), "Aborted.");
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::LlmRequestFailed);
assert_eq!(
failed.envs.get("COYOTE_LLM_ERROR_KIND").map(String::as_str),
Some("aborted")
);
assert_eq!(
failed.envs.get("COYOTE_LLM_ERROR").map(String::as_str),
Some("Aborted.")
);
}
#[tokio::test]
#[serial_test::serial]
async fn quiet_transport_fires_llm_request_failed_with_api_error_kind() {
let marker = "llm-hooks-quiet-api-err-x7x";
let mut ctx = llm_hooked_ctx(marker);
let input = Input::from_str(&ctx, "hi", None).unwrap();
let client = FailingStatusClient::new(500);
let _guard = hooks::test_sink::install();
let err =
call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal())
.await
.unwrap_err();
assert!(err.downcast_ref::<ApiStatusError>().is_some());
let captures = llm_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::LlmRequestFailed);
assert_eq!(
failed.envs.get("COYOTE_LLM_ERROR_KIND").map(String::as_str),
Some("api_error")
);
assert_eq!(
failed.envs.get("COYOTE_LLM_STATUS").map(String::as_str),
Some("500")
);
}
/// Minimal client for exercising `patch_request_data`: only the model
/// and the client-level patch config matter.
struct PatchProbeClient {
+165
View File
@@ -1,5 +1,6 @@
use super::*;
use crate::hooks::HooksMap;
use crate::{
client::Model,
config::memory,
@@ -12,6 +13,7 @@ use crate::{
};
use super::rag_cache::RagKey;
use crate::config::builtin_manifest;
use crate::config::paths;
use crate::config::prompts::{
DEFAULT_JOB_INSTRUCTIONS, DEFAULT_SPAWN_INSTRUCTIONS, DEFAULT_TEAMMATE_INSTRUCTIONS,
@@ -27,6 +29,7 @@ use fancy_regex::Captures;
use inquire::{Text, validator::Validation};
use rust_embed::Embed;
use serde::{Deserialize, Serialize};
use std::collections::BTreeSet;
use std::{env, ffi::OsStr, path::Path};
const DEFAULT_AGENT_NAME: &str = "rag";
@@ -46,6 +49,20 @@ pub type AgentVariables = IndexMap<String, String>;
#[folder = "assets/agents/"]
struct AgentAssets;
/// Splits an embedded asset path of the form `<agent>/hooks/<file>` into
/// its agent and hook-file names. Only direct children of `hooks/` count:
/// they are the only shape the builtin manifest tracks.
fn parse_direct_hook_path(path: &str) -> Option<(&str, &str)> {
let (agent, rest) = path.split_once('/')?;
let hook = direct_hook_name(rest)?;
Some((agent, hook))
}
fn direct_hook_name(rest: &str) -> Option<&str> {
let name = rest.strip_prefix("hooks/")?;
(!name.is_empty() && !name.contains('/')).then_some(name)
}
#[derive(Debug, Clone)]
pub struct Agent {
name: String,
@@ -70,6 +87,7 @@ impl Agent {
paths::agents_data_dir().display()
);
let mut written_hooks: HashMap<String, BTreeSet<String>> = HashMap::new();
for file in AgentAssets::iter() {
debug!("Processing agent file: {}", file.as_ref());
@@ -96,6 +114,12 @@ impl Agent {
info!("Creating agent file: {}", file_path.display());
let mut agent_file = File::create(&file_path)?;
agent_file.write_all(content.as_bytes())?;
if let Some((agent, hook)) = parse_direct_hook_path(file.as_ref()) {
written_hooks
.entry(agent.to_string())
.or_default()
.insert(hook.to_string());
}
#[cfg(unix)]
if is_script {
@@ -134,6 +158,27 @@ impl Agent {
}
}
// Hook filenames are open-ended (unlike AGENT_DEFINITION_FILES), so
// each bundled agent's hooks/ directory reconciles through its
// builtin manifest: only files the installer previously shipped are
// removal candidates, and user-created files in the same directory
// are never touched. Only direct children of hooks/ are tracked;
// nested shipped files install but are not reconciled.
for (agent, files) in &bundled_files {
let shipped: BTreeSet<String> = files
.iter()
.filter_map(|rest| direct_hook_name(rest))
.map(str::to_string)
.collect();
let written = written_hooks.remove(agent.as_str()).unwrap_or_default();
let hooks_dir = paths::agents_data_dir().join(agent).join("hooks");
if let Err(err) =
builtin_manifest::reconcile_builtin_dir(&hooks_dir, &shipped, &written)
{
warn!("Failed to reconcile builtin hooks for agent '{agent}': {err}");
}
}
Ok(())
}
@@ -639,6 +684,14 @@ impl Agent {
self.config.compression_model.as_deref()
}
pub fn hooks(&self) -> &HooksMap {
&self.config.hooks
}
pub fn global_hooks(&self) -> &[String] {
&self.config.global_hooks
}
pub fn is_dynamic_instructions(&self) -> bool {
self.config.dynamic_instructions
}
@@ -848,6 +901,10 @@ pub struct AgentConfig {
pub mcp_tools: Option<IndexMap<String, Vec<String>>>,
#[serde(default)]
pub global_tools: Vec<String>,
#[serde(default)]
pub hooks: HooksMap,
#[serde(default)]
pub global_hooks: Vec<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub skills_enabled: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -917,6 +974,8 @@ impl AgentConfig {
reasoning_effort: graph.reasoning_effort.clone(),
description: graph.description.clone(),
global_tools: graph.global_tools.clone(),
hooks: graph.hooks.clone(),
global_hooks: graph.global_hooks.clone(),
mcp_servers: graph.mcp_servers.clone(),
mcp_tools: graph.mcp_tools.clone(),
skills_enabled: graph.skills_enabled,
@@ -965,6 +1024,14 @@ impl AgentConfig {
{
self.global_tools = v;
}
if let Ok(v) = env::var(with_prefix("global_hooks")) {
match serde_json::from_str(&v) {
Ok(v) => self.global_hooks = v,
Err(err) => {
debug!("Ignoring malformed global_hooks env override for agent '{name}': {err}")
}
}
}
if let Ok(v) = env::var(with_prefix("mcp_servers"))
&& let Ok(v) = serde_json::from_str(&v)
{
@@ -1295,6 +1362,16 @@ pub fn complete_agent_variables(agent_name: &str) -> Vec<(String, Option<String>
mod tests {
use super::*;
#[test]
fn parse_direct_hook_path_accepts_only_direct_hook_children() {
assert_eq!(parse_direct_hook_path("x/hooks/a.sh"), Some(("x", "a.sh")));
assert_eq!(parse_direct_hook_path("x/hooks/nested/a.sh"), None);
assert_eq!(parse_direct_hook_path("hooks/a.sh"), None);
assert_eq!(parse_direct_hook_path("x/hooks/"), None);
assert_eq!(parse_direct_hook_path("x/config.yaml"), None);
assert_eq!(parse_direct_hook_path("config.yaml"), None);
}
#[test]
fn agent_config_parses_from_yaml() {
let yaml = r#"
@@ -1336,6 +1413,92 @@ variables:
assert_eq!(config.variables[0].name, "username");
}
#[test]
fn agent_config_hooks_and_global_hooks_round_trip() {
let yaml = "\
name: hooked
instructions: hi
hooks:
tool.started:
- name: notify
command: ./hooks/notify.sh
global_hooks:
- tool.started.notify
";
let config: AgentConfig = serde_yaml::from_str(yaml).unwrap();
assert_eq!(config.hooks["tool.started"][0].name, "notify");
assert_eq!(config.hooks["tool.started"][0].command, "./hooks/notify.sh");
assert_eq!(config.global_hooks, vec!["tool.started.notify"]);
let serialized = serde_yaml::to_string(&config).unwrap();
let reparsed: AgentConfig = serde_yaml::from_str(&serialized).unwrap();
assert_eq!(reparsed.hooks, config.hooks);
assert_eq!(reparsed.global_hooks, config.global_hooks);
}
#[test]
fn load_envs_overrides_global_hooks() {
let yaml = "name: hooks-env-probe\ninstructions: hi\nglobal_hooks:\n - initial.hook\n";
let mut config: AgentConfig = serde_yaml::from_str(yaml).unwrap();
let env_name = normalize_env_name("hooks-env-probe_global_hooks");
let prev = env::var_os(&env_name);
unsafe {
env::set_var(
&env_name,
r#"["tool.started.notify","turn.completed.webhook"]"#,
)
};
config.load_envs(&AppConfig::default());
assert_eq!(
config.global_hooks,
vec!["tool.started.notify", "turn.completed.webhook"]
);
unsafe { env::set_var(&env_name, "not json") };
config.load_envs(&AppConfig::default());
assert_eq!(
config.global_hooks,
vec!["tool.started.notify", "turn.completed.webhook"]
);
unsafe {
match prev {
Some(v) => env::set_var(&env_name, v),
None => env::remove_var(&env_name),
}
}
}
#[test]
fn from_graph_carries_hooks_and_global_hooks() {
let yaml = "\
name: g
start: e
hooks:
turn.completed:
- name: webhook
command: curl -s https://example.com/hook
global_hooks:
- turn.completed.webhook
nodes:
e:
id: e
type: end
output: done
";
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
let config = AgentConfig::from_graph("g", &graph);
assert_eq!(config.hooks, graph.hooks);
assert_eq!(config.global_hooks, graph.global_hooks);
assert_eq!(config.hooks["turn.completed"][0].name, "webhook");
assert_eq!(config.global_hooks, vec!["turn.completed.webhook"]);
}
#[test]
fn agent_config_defaults() {
let yaml = "name: minimal\ninstructions: hi\n";
@@ -1350,6 +1513,8 @@ variables:
assert_eq!(config.escalation_timeout, 0);
assert!(config.mcp_servers.is_empty());
assert!(config.global_tools.is_empty());
assert!(config.hooks.is_empty());
assert!(config.global_hooks.is_empty());
assert!(config.conversation_starters.is_empty());
assert!(config.variables.is_empty());
assert!(config.model_id.is_none());
+88
View File
@@ -1,4 +1,5 @@
use crate::client::{ClientConfig, Model, ModelType, list_models};
use crate::hooks::HooksMap;
use crate::render::{MarkdownRender, RenderOptions};
use crate::utils::{IS_STDOUT_TERMINAL, NO_COLOR, decode_bin, drain_stale_tty_input, get_env_name};
@@ -52,6 +53,9 @@ pub struct AppConfig {
pub enabled_mcp_servers: Option<Vec<String>>,
pub mcp_tools: Option<IndexMap<String, Vec<String>>>,
#[serde(default)]
pub hooks: HooksMap,
pub auto_continue: bool,
pub max_auto_continues: usize,
pub inject_todo_instructions: bool,
@@ -143,6 +147,8 @@ impl Default for AppConfig {
enabled_mcp_servers: None,
mcp_tools: None,
hooks: Default::default(),
auto_continue: false,
max_auto_continues: 10,
inject_todo_instructions: true,
@@ -235,6 +241,8 @@ impl AppConfig {
enabled_mcp_servers: config.enabled_mcp_servers,
mcp_tools: config.mcp_tools,
hooks: config.hooks,
auto_continue: config.auto_continue,
max_auto_continues: config.max_auto_continues,
inject_todo_instructions: config.inject_todo_instructions,
@@ -572,6 +580,15 @@ impl AppConfig {
self.enabled_mcp_servers = v.map(|raw| super::csv_to_vec(&raw));
}
if let Ok(v) = env::var(get_env_name("hooks")) {
match serde_json::from_str(&v) {
Ok(v) => self.hooks = v,
Err(err) => {
debug!("Ignoring malformed hooks env override: {err}")
}
}
}
if let Some(v) = super::read_env_value::<String>(&get_env_name("repl_prelude")) {
self.repl_prelude = v;
}
@@ -788,6 +805,77 @@ mod tests {
assert_eq!(app.clients.len(), 1);
}
#[test]
fn from_config_copies_hooks() {
let mut hooks = HooksMap::new();
hooks.insert(
"turn.completed".to_string(),
vec![crate::hooks::HookDef {
name: "notify".to_string(),
command: "./hooks/notify.sh".to_string(),
}],
);
let cfg = Config {
model_id: "test-model".to_string(),
clients: vec![ClientConfig::default()],
hooks: hooks.clone(),
..Config::default()
};
let app = AppConfig::from_config(cfg).unwrap();
assert_eq!(app.hooks, hooks);
}
#[test]
fn from_config_defaults_hooks_to_empty() {
let cfg = Config {
model_id: "test-model".to_string(),
clients: vec![ClientConfig::default()],
..Config::default()
};
let app = AppConfig::from_config(cfg).unwrap();
assert!(app.hooks.is_empty());
}
#[test]
#[serial_test::serial]
fn load_envs_overrides_hooks() {
let env_name = get_env_name("hooks");
let prev = env::var_os(&env_name);
let mut hooks = HooksMap::new();
hooks.insert(
"turn.completed".to_string(),
vec![crate::hooks::HookDef {
name: "notify".to_string(),
command: "./hooks/notify.sh".to_string(),
}],
);
let mut app = AppConfig::default();
unsafe { env::set_var(&env_name, serde_json::to_string(&hooks).unwrap()) };
app.load_envs();
assert_eq!(app.hooks, hooks);
unsafe { env::set_var(&env_name, "[not json") };
app.load_envs();
assert_eq!(app.hooks, hooks);
unsafe { env::remove_var(&env_name) };
app.load_envs();
assert_eq!(app.hooks, hooks);
unsafe {
match prev {
Some(v) => env::set_var(&env_name, v),
None => env::remove_var(&env_name),
}
}
}
#[test]
fn from_config_copies_mapping_fields() {
let mut cfg = Config {
+293
View File
@@ -0,0 +1,293 @@
//! Per-directory tracking of files the builtin installer shipped, so a later
//! release that stops shipping an asset can remove the stale copy without
//! ever touching user-created files that live in the same directory.
//!
//! Unlike agent definition files, hook filenames are open-ended: no closed
//! candidate list can distinguish "previously bundled" from "user-authored".
//! The manifest records exactly what the installer shipped; anything absent
//! from it is user-owned and untouchable.
use crate::config::ensure_parent_exists;
use crate::function::write_file_atomic;
use anyhow::{Context, Result};
use std::collections::BTreeSet;
use std::fs;
use std::path::Path;
pub(crate) const BUILTIN_MANIFEST_FILE: &str = ".builtin-manifest";
pub(crate) fn is_builtin_manifest_name(name: &str) -> bool {
name.trim_end_matches(['.', ' '])
.to_uppercase()
.eq_ignore_ascii_case(BUILTIN_MANIFEST_FILE)
}
/// Reconciles `dir` against the currently shipped set of builtin filenames:
/// a file is removed ONLY if it appears in the previous manifest AND is
/// absent from `shipped`. The manifest is then rewritten to the names this
/// installer actually owns: files written this run (`written`), plus shipped
/// names it already owned, plus stale files whose removal failed (kept so a
/// later run retries). A shipped name the installer declined to write (e.g., a
/// pre-existing user file with a colliding name) is never claimed.
///
/// Failure modes degrade toward keeping files: a missing or unreadable
/// manifest yields no deletions, and entries that are not plain filenames
/// are ignored.
pub(crate) fn reconcile_builtin_dir(
dir: &Path,
shipped: &BTreeSet<String>,
written: &BTreeSet<String>,
) -> Result<()> {
let previous = read_manifest(dir);
let mut retained: BTreeSet<String> = written.clone();
retained.extend(shipped.intersection(&previous).cloned());
for name in previous.difference(shipped) {
let path = dir.join(name);
if !path.is_file() {
continue;
}
info!(
"Removing stale builtin file no longer shipped: {}",
path.display()
);
if let Err(err) = fs::remove_file(&path) {
warn!(
"Failed to remove stale builtin file {}: {err}",
path.display()
);
retained.insert(name.clone());
}
}
write_manifest(dir, &retained)
}
fn read_manifest(dir: &Path) -> BTreeSet<String> {
let path = dir.join(BUILTIN_MANIFEST_FILE);
let content = match fs::read_to_string(&path) {
Ok(content) => content,
Err(err) => {
debug!(
"No readable builtin manifest at {} ({err}); skipping stale-file removal",
path.display()
);
return BTreeSet::new();
}
};
content
.lines()
.map(str::trim)
.filter(|line| {
if line.is_empty() || is_builtin_manifest_name(line) {
return false;
}
if !is_plain_file_name(line) {
debug!(
"Ignoring suspicious builtin manifest entry in {}: {line:?}",
path.display()
);
return false;
}
true
})
.map(str::to_string)
.collect()
}
/// Manifest entries name direct children of the manifest's directory and
/// nothing else; anything path-like is rejected so a corrupted or crafted
/// manifest can never direct a deletion outside that directory. The same
/// component rules the bundle store applies to recorded paths are reused so
/// Windows drive prefixes (`C:x`), ADS colons, trailing dots/spaces, and
/// reserved device names are rejected too.
fn is_plain_file_name(name: &str) -> bool {
!name.contains(['/', '\\'])
&& name != "."
&& name != ".."
&& super::install_remote::is_safe_component(name)
}
fn write_manifest(dir: &Path, names: &BTreeSet<String>) -> Result<()> {
let path = dir.join(BUILTIN_MANIFEST_FILE);
if names.is_empty() {
if path.is_file() {
fs::remove_file(&path)
.with_context(|| format!("failed to remove builtin manifest {}", path.display()))?;
}
return Ok(());
}
let mut content = names.iter().cloned().collect::<Vec<_>>().join("\n");
content.push('\n');
ensure_parent_exists(&path)?;
write_file_atomic(&path, &content, None)
}
#[cfg(test)]
mod tests {
use super::*;
use crate::utils;
use std::path::PathBuf;
fn fresh_dir(label: &str) -> PathBuf {
let dir = utils::temp_file(label, "");
fs::create_dir_all(&dir).unwrap();
dir
}
fn shipped(names: &[&str]) -> BTreeSet<String> {
names.iter().map(|n| n.to_string()).collect()
}
#[test]
fn removes_only_manifest_listed_files_absent_from_shipped() {
let dir = fresh_dir("builtin-manifest-stale-");
fs::write(dir.join(BUILTIN_MANIFEST_FILE), "old.sh\nkept.sh\n").unwrap();
fs::write(dir.join("old.sh"), "stale").unwrap();
fs::write(dir.join("kept.sh"), "current").unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
reconcile_builtin_dir(&dir, &shipped(&["kept.sh"]), &shipped(&[])).unwrap();
assert!(!dir.join("old.sh").exists(), "stale bundled file removed");
assert!(dir.join("kept.sh").exists(), "still-shipped file kept");
assert!(dir.join("user.sh").exists(), "unlisted user file untouched");
assert_eq!(
fs::read_to_string(dir.join(BUILTIN_MANIFEST_FILE)).unwrap(),
"kept.sh\n"
);
let _ = fs::remove_dir_all(&dir);
}
#[test]
fn missing_manifest_deletes_nothing_and_writes_shipped_set() {
let dir = fresh_dir("builtin-manifest-missing-");
fs::write(dir.join("user.sh"), "user-owned").unwrap();
reconcile_builtin_dir(&dir, &shipped(&["new.sh"]), &shipped(&["new.sh"])).unwrap();
assert!(dir.join("user.sh").exists());
assert_eq!(
fs::read_to_string(dir.join(BUILTIN_MANIFEST_FILE)).unwrap(),
"new.sh\n"
);
let _ = fs::remove_dir_all(&dir);
}
/// A shipped name the installer skipped (pre-existing user file with a
/// colliding name) is never claimed by the manifest, so a later release
/// dropping that hook can never delete the user's file.
#[test]
fn skipped_preexisting_user_file_is_never_claimed() {
let dir = fresh_dir("builtin-manifest-unclaimed-");
fs::write(dir.join("user.sh"), "user-owned").unwrap();
reconcile_builtin_dir(&dir, &shipped(&["user.sh"]), &shipped(&[])).unwrap();
assert!(
!dir.join(BUILTIN_MANIFEST_FILE).exists(),
"nothing was written this run and nothing was owned before"
);
reconcile_builtin_dir(&dir, &shipped(&[]), &shipped(&[])).unwrap();
assert!(
dir.join("user.sh").exists(),
"the unclaimed user file survives the hook being dropped"
);
let _ = fs::remove_dir_all(&dir);
}
#[test]
fn suspicious_manifest_entries_never_direct_deletions() {
let dir = fresh_dir("builtin-manifest-suspicious-");
let outside = fresh_dir("builtin-manifest-outside-");
let escape = format!(
"../{}/victim.sh",
outside.file_name().unwrap().to_string_lossy()
);
fs::write(
dir.join(BUILTIN_MANIFEST_FILE),
format!("{escape}\nsub/nested.sh\nC:victim.sh\ntrailing.\n..\n.\n\n"),
)
.unwrap();
fs::write(outside.join("victim.sh"), "keep").unwrap();
fs::create_dir_all(dir.join("sub")).unwrap();
fs::write(dir.join("sub/nested.sh"), "keep").unwrap();
reconcile_builtin_dir(&dir, &BTreeSet::new(), &BTreeSet::new()).unwrap();
assert!(outside.join("victim.sh").exists());
assert!(dir.join("sub/nested.sh").exists());
let _ = fs::remove_dir_all(&dir);
let _ = fs::remove_dir_all(&outside);
}
#[test]
fn unreadable_manifest_deletes_nothing() {
let dir = fresh_dir("builtin-manifest-unreadable-");
fs::write(dir.join(BUILTIN_MANIFEST_FILE), [0xff, 0xfe, 0x00]).unwrap();
fs::write(dir.join("old.sh"), "keep").unwrap();
reconcile_builtin_dir(&dir, &shipped(&["new.sh"]), &shipped(&["new.sh"])).unwrap();
assert!(
dir.join("old.sh").exists(),
"a bad manifest must fail safe toward keeping files"
);
assert_eq!(
fs::read_to_string(dir.join(BUILTIN_MANIFEST_FILE)).unwrap(),
"new.sh\n",
"manifest self-heals to the current shipped set"
);
let _ = fs::remove_dir_all(&dir);
}
#[test]
fn empty_shipped_set_removes_manifest_and_creates_nothing() {
let dir = fresh_dir("builtin-manifest-empty-");
fs::write(dir.join(BUILTIN_MANIFEST_FILE), "old.sh\n").unwrap();
fs::write(dir.join("old.sh"), "stale").unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
reconcile_builtin_dir(&dir, &BTreeSet::new(), &BTreeSet::new()).unwrap();
assert!(!dir.join("old.sh").exists());
assert!(dir.join("user.sh").exists());
assert!(!dir.join(BUILTIN_MANIFEST_FILE).exists());
let absent = dir.join("never-created");
reconcile_builtin_dir(&absent, &BTreeSet::new(), &BTreeSet::new()).unwrap();
assert!(!absent.exists(), "no directory is created for an empty set");
let _ = fs::remove_dir_all(&dir);
}
#[test]
fn manifest_listing_itself_never_deletes_it() {
let dir = fresh_dir("builtin-manifest-self-");
fs::write(
dir.join(BUILTIN_MANIFEST_FILE),
format!("{BUILTIN_MANIFEST_FILE}\n.BUILTIN-MANIFEST\nkept.sh\n"),
)
.unwrap();
fs::write(dir.join("kept.sh"), "current").unwrap();
reconcile_builtin_dir(&dir, &shipped(&["kept.sh"]), &shipped(&[])).unwrap();
assert!(dir.join(BUILTIN_MANIFEST_FILE).exists());
assert!(dir.join("kept.sh").exists());
let _ = fs::remove_dir_all(&dir);
}
#[test]
fn manifest_name_matcher_covers_filesystem_aliases() {
assert!(is_builtin_manifest_name(".builtin-manifest"));
assert!(is_builtin_manifest_name(".BUILTIN-MANIFEST"));
assert!(is_builtin_manifest_name(".Builtin-Manifest."));
assert!(is_builtin_manifest_name(".builtin-manifest "));
assert!(is_builtin_manifest_name(".builtin-manifeſt"));
assert!(!is_builtin_manifest_name("builtin-manifest"));
assert!(!is_builtin_manifest_name(".builtin-manifesto"));
assert!(!is_builtin_manifest_name("notify.sh"));
}
}
+7
View File
@@ -1183,6 +1183,13 @@ mod tests {
assert_eq!(resolved.name, "y/mcp_config");
assert_eq!(resolved.qualified_from.as_deref(), Some("mcp_config"));
let resolved = store
.resolve_bundle_name("https://github.com/z/hooks", None)
.unwrap();
assert_eq!(resolved.name, "z/hooks");
assert_eq!(resolved.qualified_from.as_deref(), Some("hooks"));
}
#[test]
+324 -5
View File
@@ -2,6 +2,7 @@ use super::bundles::{
BundleStore, FileAction, FileRecord, InstallMetadata, McpAction, McpServerRecord, hash_bytes,
hash_file,
};
use crate::config::builtin_manifest::is_builtin_manifest_name;
use crate::config::{AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths};
#[cfg(not(windows))]
use crate::function::Language;
@@ -32,7 +33,7 @@ pub fn install_remote(git_url: &str, filter: Option<InstallFilter>, force: bool)
if layout.is_empty() {
println!(
"No recognized assets found in {git_url}. Expected one or more of: \
agents/, roles/, skills/, macros/, functions/tools/, mcp.json"
agents/, roles/, skills/, macros/, functions/tools/, hooks/, mcp.json"
);
return Ok(());
}
@@ -805,7 +806,7 @@ fn is_safe_relative_path(path: &str) -> bool {
/// Rejects names Windows refuses or silently rewrites (alternate data stream
/// colons, reserved device names, trailing dots or spaces) so a recorded path
/// denotes the same regular file on every platform.
fn is_safe_component(name: &str) -> bool {
pub(crate) fn is_safe_component(name: &str) -> bool {
!name.contains(':')
&& !name.ends_with('.')
&& !name.ends_with(' ')
@@ -1270,6 +1271,7 @@ struct RemoteLayout {
roles: Option<PathBuf>,
skills: Option<PathBuf>,
macros: Option<PathBuf>,
hooks: Option<PathBuf>,
functions_tools: Option<PathBuf>,
mcp_json: Option<PathBuf>,
manifest: Option<BundleManifest>,
@@ -1282,6 +1284,7 @@ impl RemoteLayout {
&& self.roles.is_none()
&& self.skills.is_none()
&& self.macros.is_none()
&& self.hooks.is_none()
&& self.functions_tools.is_none()
&& self.mcp_json.is_none()
}
@@ -1313,6 +1316,11 @@ fn scan_remote_layout(root: &Path) -> Result<RemoteLayout> {
layout.macros = Some(macros);
}
let hooks = root.join("hooks");
if hooks.is_dir() {
layout.hooks = Some(hooks);
}
let root_mcp = root.join("mcp.json");
if root_mcp.is_file() {
layout.mcp_json = Some(root_mcp);
@@ -1540,6 +1548,10 @@ fn apply_filter(mut layout: RemoteLayout, filter: Option<InstallFilter>) -> Remo
functions_tools: layout.functions_tools.take(),
..base
},
InstallFilter::Hooks => RemoteLayout {
hooks: layout.hooks.take(),
..base
},
InstallFilter::McpConfig => RemoteLayout {
mcp_json: layout.mcp_json.take(),
..base
@@ -1591,6 +1603,7 @@ enum TopCategory {
Roles,
Skills,
Macros,
Hooks,
FunctionsTools,
}
@@ -1601,6 +1614,7 @@ impl TopCategory {
TopCategory::Roles => "roles",
TopCategory::Skills => "skills",
TopCategory::Macros => "macros",
TopCategory::Hooks => "hooks",
TopCategory::FunctionsTools => "functions/tools",
}
}
@@ -1619,6 +1633,8 @@ enum PlannedKind {
struct PlannedFile {
src: PathBuf,
dst: PathBuf,
/// Path relative to the category's source directory in the bundle.
rel: PathBuf,
kind: PlannedKind,
top_category: TopCategory,
}
@@ -1660,6 +1676,9 @@ fn plan_changes(layout: &RemoteLayout) -> Result<InstallPlan> {
&mut files,
)?;
}
if let Some(src_dir) = &layout.hooks {
plan_dir_into(src_dir, &paths::hooks_dir(), TopCategory::Hooks, &mut files)?;
}
if let Some(src_dir) = &layout.functions_tools {
plan_dir_into(
src_dir,
@@ -1686,7 +1705,28 @@ fn plan_dir_into(
for src in walk_files(src_dir)? {
let rel = src
.strip_prefix(src_dir)
.expect("walk_files only returns paths under src_dir");
.expect("walk_files only returns paths under src_dir")
.to_path_buf();
// The builtin installer's per-directory manifests drive stale-file
// deletion; letting a bundle install one would hand it control over
// which local files the next builtin reconcile removes. Every path
// component is checked (a bundle-shipped DIRECTORY of that name would
// permanently occupy the manifest path and disable reconciliation),
// under the filesystem-alias-aware match of is_builtin_manifest_name.
let is_builtin_manifest = rel.components().any(|c| match c.as_os_str().to_str() {
Some(name) => is_builtin_manifest_name(name),
// A non-UTF-8 name can never equal the ASCII manifest constant.
None => false,
});
if is_builtin_manifest {
log::warn!(
"Ignoring bundle file {} ({}): the name is reserved for the builtin installer",
rel.display(),
category.label()
);
continue;
}
if category == TopCategory::Skills {
let skill_name = rel
@@ -1707,11 +1747,12 @@ fn plan_dir_into(
})?;
}
let dst = dst_dir.join(rel);
let dst = dst_dir.join(&rel);
let kind = classify_file(&src, &dst)?;
out.push(PlannedFile {
src,
dst,
rel,
kind,
top_category: category,
});
@@ -1790,6 +1831,7 @@ fn print_plan_summary(plan: &InstallPlan) {
TopCategory::Roles,
TopCategory::Skills,
TopCategory::Macros,
TopCategory::Hooks,
TopCategory::FunctionsTools,
] {
let new_ = count_kind(plan, cat, PlannedKind::New);
@@ -1808,6 +1850,33 @@ fn print_plan_summary(plan: &InstallPlan) {
println!("{line}");
}
}
for line in hook_script_lines(plan) {
println!("{line}");
}
}
/// Bundle-shipped hook scripts execute on the user's machine once wired into
/// config, so every planned file landing under a hooks/ directory is called
/// out individually. Visibility only: installing them needs no extra approval.
fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
plan.files
.iter()
.filter(|planned| {
planned.top_category == TopCategory::Hooks
|| planned
.rel
.parent()
.is_some_and(|parent| parent.components().any(|c| c.as_os_str() == "hooks"))
})
.map(|planned| {
let rel = planned.rel.to_string_lossy().replace('\\', "/");
format!(
"bundle defines hook script: {}/{rel}",
planned.top_category.label()
)
})
.collect()
}
fn count_kind(plan: &InstallPlan, cat: TopCategory, kind: PlannedKind) -> usize {
@@ -2349,6 +2418,8 @@ fn print_secret_summary(added: &[String], deferred: &[String]) {
#[cfg(test)]
mod tests {
use super::*;
use crate::config;
use crate::config::builtin_manifest::BUILTIN_MANIFEST_FILE;
use crate::sandbox::SANDBOX_ENV_FLAG;
use crate::utils::get_env_name;
use serial_test::serial;
@@ -2551,6 +2622,7 @@ mod tests {
roles: Some(PathBuf::from("r")),
skills: Some(PathBuf::from("s")),
macros: Some(PathBuf::from("m")),
hooks: Some(PathBuf::from("h")),
functions_tools: Some(PathBuf::from("f")),
mcp_json: Some(PathBuf::from("j")),
..RemoteLayout::default()
@@ -2560,6 +2632,7 @@ mod tests {
assert!(out.agents.is_some() && out.roles.is_some() && out.skills.is_some());
assert!(out.macros.is_some() && out.functions_tools.is_some() && out.mcp_json.is_some());
assert!(out.hooks.is_some());
}
#[test]
@@ -2638,6 +2711,37 @@ mod tests {
assert!(out.functions_tools.is_none() && out.mcp_json.is_none());
}
#[test]
fn apply_filter_hooks_keeps_only_hooks() {
let l = RemoteLayout {
agents: Some(PathBuf::from("a")),
roles: Some(PathBuf::from("r")),
skills: Some(PathBuf::from("s")),
macros: Some(PathBuf::from("m")),
hooks: Some(PathBuf::from("h")),
functions_tools: Some(PathBuf::from("f")),
mcp_json: Some(PathBuf::from("j")),
..RemoteLayout::default()
};
let out = apply_filter(l, Some(InstallFilter::Hooks));
assert_eq!(out.hooks, Some(PathBuf::from("h")));
assert!(!out.is_empty());
assert!(out.agents.is_none() && out.roles.is_none() && out.skills.is_none());
assert!(out.macros.is_none() && out.functions_tools.is_none() && out.mcp_json.is_none());
}
#[test]
fn remote_layout_with_only_hooks_is_not_empty() {
let l = RemoteLayout {
hooks: Some(PathBuf::from("h")),
..RemoteLayout::default()
};
assert!(!l.is_empty());
}
#[test]
fn walk_files_skips_dot_git_and_collects_regular_files() {
let root = fresh_temp_dir("walk-test-");
@@ -2684,6 +2788,7 @@ mod tests {
fs::create_dir_all(root.join("roles")).unwrap();
fs::create_dir_all(root.join("skills")).unwrap();
fs::create_dir_all(root.join("macros")).unwrap();
fs::create_dir_all(root.join("hooks")).unwrap();
fs::create_dir_all(root.join("functions/tools")).unwrap();
touch(&root.join("functions/mcp.json"));
touch(&root.join("README.md"));
@@ -2693,11 +2798,29 @@ mod tests {
assert!(layout.roles.is_some());
assert!(layout.skills.is_some());
assert!(layout.macros.is_some());
assert!(layout.hooks.is_some());
assert!(layout.functions_tools.is_some());
assert!(layout.mcp_json.is_some());
let _ = fs::remove_dir_all(&root);
}
#[test]
fn scan_remote_layout_finds_hooks_only() {
let root = fresh_temp_dir("scan-hooks-only-");
fs::create_dir_all(root.join("hooks")).unwrap();
touch(&root.join("hooks/notify.sh"));
let layout = scan_remote_layout(&root).unwrap();
assert_eq!(layout.hooks, Some(root.join("hooks")));
assert!(layout.agents.is_none());
assert!(layout.roles.is_none());
assert!(layout.macros.is_none());
assert!(layout.functions_tools.is_none());
assert!(layout.mcp_json.is_none());
let _ = fs::remove_dir_all(&root);
}
#[test]
fn scan_remote_layout_finds_root_mcp_json() {
let root = fresh_temp_dir("scan-root-mcp-test-");
@@ -2752,6 +2875,71 @@ mod tests {
let _ = fs::remove_dir_all(&root);
}
#[test]
fn plan_dir_into_skips_bundle_shipped_builtin_manifests() {
let root = fresh_temp_dir("plan-manifest-skip-");
let src_dir = root.join("hooks");
touch(&src_dir.join("notify.sh"));
touch(&src_dir.join(BUILTIN_MANIFEST_FILE));
touch(&src_dir.join(format!("sub/{BUILTIN_MANIFEST_FILE}")));
// Case and trailing-dot variants alias the real manifest path on
// case-insensitive filesystems; they live in separate subdirs so this
// fixture also builds on such filesystems without name collisions.
touch(&src_dir.join("sub2/.BUILTIN-MANIFEST"));
touch(&src_dir.join("sub3/.Builtin-Manifest."));
// A DIRECTORY of the manifest name would occupy the manifest path and
// disable reconciliation; a Unicode case-fold alias (U+017F ſ → s)
// aliases it on case-insensitive APFS/NTFS volumes.
touch(&src_dir.join("sub4/.builtin-manifest/payload.sh"));
touch(&src_dir.join("sub5/.builtin-manifeſt"));
let dst_dir = root.join("dst");
let mut files = Vec::new();
plan_dir_into(&src_dir, &dst_dir, TopCategory::Hooks, &mut files).unwrap();
let rels: Vec<&Path> = files.iter().map(|p| p.rel.as_path()).collect();
assert_eq!(
rels,
vec![Path::new("notify.sh")],
"a bundle must never install a builtin manifest: it would hand the \
bundle control over builtin stale-file deletion"
);
let _ = fs::remove_dir_all(&root);
}
#[test]
fn hook_script_lines_cover_every_hooks_dir_and_nothing_else() {
let file = |rel: &str, cat: TopCategory| PlannedFile {
src: PathBuf::from("src").join(rel),
dst: PathBuf::from("dst").join(rel),
rel: PathBuf::from(rel),
kind: PlannedKind::New,
top_category: cat,
};
let plan = InstallPlan {
files: vec![
file("notify.sh", TopCategory::Hooks),
file("x/hooks/pre.sh", TopCategory::Agents),
file("hooks/role-hook.sh", TopCategory::Roles),
file("x/config.yaml", TopCategory::Agents),
file("reviewer.md", TopCategory::Roles),
file("hello.yaml", TopCategory::Macros),
],
mcp_json: None,
};
let lines = hook_script_lines(&plan);
assert_eq!(
lines,
vec![
"bundle defines hook script: hooks/notify.sh",
"bundle defines hook script: agents/x/hooks/pre.sh",
"bundle defines hook script: roles/hooks/role-hook.sh",
]
);
}
#[test]
fn classify_file_new_when_dst_missing() {
let dir = fresh_temp_dir("classify-new-");
@@ -3420,6 +3608,91 @@ mod tests {
let _ = fs::remove_dir_all(&src_root);
}
fn hooks_fixture_bundle(label: &str) -> (PathBuf, PathBuf) {
let src_root = fresh_temp_dir(label);
let repo = src_root.join("hooky");
write_src(&repo, "hooks/notify-me.sh", "#!/bin/sh\necho hi\n");
write_src(&repo, "roles/hooks/role-hook.sh", "#!/bin/sh\n");
write_src(&repo, "agents/x/hooks/pre.sh", "#!/bin/sh\n");
write_src(&repo, "macros/hello.yaml", "name: hello\n");
init_bundle_repo(&repo);
(src_root, repo)
}
#[cfg(unix)]
fn assert_executable(path: &Path) {
use std::os::unix::fs::PermissionsExt;
let mode = fs::metadata(path).unwrap().permissions().mode();
assert_ne!(
mode & 0o111,
0,
"{} must carry the executable bit",
path.display()
);
}
#[test]
#[serial]
fn install_remote_installs_all_hooks_dirs_and_records_hooks_category() {
let _guard = TestVaultConfigGuard::new("prov-hooks-dirs");
let (src_root, repo) = hooks_fixture_bundle("prov-hooks-dirs-src-");
install_remote(repo.to_str().unwrap(), None, false).unwrap();
let top = paths::hooks_dir().join("notify-me.sh");
let role = paths::roles_dir().join("hooks/role-hook.sh");
let agent = paths::agents_data_dir().join("x/hooks/pre.sh");
assert!(top.is_file(), "top-level hooks/ installs to hooks_dir");
assert!(role.is_file(), "roles/hooks/ installs via the roles walk");
assert!(
agent.is_file(),
"agents/<name>/hooks/ installs via the agents walk"
);
#[cfg(unix)]
{
assert_executable(&top);
assert_executable(&role);
assert_executable(&agent);
}
let store = BundleStore::load().unwrap();
let record = store.get("hooky").unwrap();
let category_of = |path: &str| {
record
.files
.iter()
.find(|f| f.path == path)
.unwrap_or_else(|| panic!("no provenance row for {path}"))
.category
.clone()
};
assert_eq!(category_of("hooks/notify-me.sh"), "hooks");
assert_eq!(category_of("roles/hooks/role-hook.sh"), "roles");
assert_eq!(category_of("agents/x/hooks/pre.sh"), "agents");
let _ = fs::remove_dir_all(&src_root);
}
#[test]
#[serial]
fn install_remote_filter_hooks_restricts_to_top_level_hooks_dir() {
let _guard = TestVaultConfigGuard::new("prov-hooks-filter");
let (src_root, repo) = hooks_fixture_bundle("prov-hooks-filter-src-");
install_remote(repo.to_str().unwrap(), Some(InstallFilter::Hooks), false).unwrap();
assert!(paths::hooks_dir().join("notify-me.sh").is_file());
assert!(!paths::roles_dir().join("hooks/role-hook.sh").exists());
assert!(!paths::agents_data_dir().join("x/hooks/pre.sh").exists());
assert!(!paths::macros_dir().join("hello.yaml").exists());
let store = BundleStore::load().unwrap();
let record = store.get("hooky").unwrap();
assert_eq!(record.files.len(), 1);
assert_eq!(record.files[0].path, "hooks/notify-me.sh");
assert_eq!(record.files[0].category, "hooks");
let _ = fs::remove_dir_all(&src_root);
}
#[test]
#[serial]
fn install_remote_without_manifest_uses_repo_slug_and_short_sha_version() {
@@ -3467,12 +3740,14 @@ mod tests {
PlannedFile {
src: src_new,
dst: dst_new.clone(),
rel: PathBuf::from("new.yaml"),
kind: PlannedKind::New,
top_category: TopCategory::Macros,
},
PlannedFile {
src: src_conflict,
dst: dst_conflict.clone(),
rel: PathBuf::from("conflict.yaml"),
kind: PlannedKind::Conflict,
top_category: TopCategory::Macros,
},
@@ -3525,6 +3800,7 @@ mod tests {
files: vec![PlannedFile {
src,
dst: dst.clone(),
rel: PathBuf::from("owned.yaml"),
kind: PlannedKind::Conflict,
top_category: TopCategory::Macros,
}],
@@ -3656,12 +3932,50 @@ mod tests {
}
let _guard = TestVaultConfigGuard::new("prov-builtin");
crate::config::install_assets(crate::config::AssetCategory::Macros).unwrap();
config::install_assets(config::AssetCategory::Macros).unwrap();
assert!(fs::read_dir(paths::macros_dir()).unwrap().next().is_some());
assert!(!paths::installed_bundles_file().exists());
}
#[test]
#[serial]
fn builtin_hooks_category_installs_example_scripts() {
if *IS_STDOUT_TERMINAL {
eprintln!(
"Skipping builtin_hooks_category_installs_example_scripts: requires non-TTY stdout"
);
return;
}
let _guard = TestVaultConfigGuard::new("prov-hooks");
let hooks_key = get_env_name("hooks_dir");
let previous_hooks = env::var_os(&hooks_key);
let hooks_dir = fresh_temp_dir("prov-hooks-dir-");
unsafe {
env::set_var(&hooks_key, &hooks_dir);
}
// Capture outcomes before restoring the env var so a failed assertion
// cannot leave the hooks-dir override pointing at a deleted temp dir.
let result = config::install_assets(AssetCategory::Hooks);
let installed = (
hooks_dir.join("notify.sh").is_file(),
hooks_dir.join("log-events.sh").is_file(),
);
unsafe {
match previous_hooks {
Some(v) => env::set_var(&hooks_key, v),
None => env::remove_var(&hooks_key),
}
}
let _ = fs::remove_dir_all(&hooks_dir);
result.unwrap();
assert!(installed.0, "notify.sh must be installed");
assert!(installed.1, "log-events.sh must be installed");
}
#[test]
#[serial]
fn update_silently_refreshes_owned_unmodified_files() {
@@ -3798,6 +4112,7 @@ mod tests {
files: vec![PlannedFile {
src,
dst: dst.clone(),
rel: PathBuf::from("owned.yaml"),
kind: PlannedKind::Conflict,
top_category: TopCategory::Macros,
}],
@@ -3844,6 +4159,7 @@ mod tests {
files: vec![PlannedFile {
src,
dst: dst.clone(),
rel: PathBuf::from("owned.yaml"),
kind: PlannedKind::Conflict,
top_category: TopCategory::Macros,
}],
@@ -3923,6 +4239,7 @@ mod tests {
files: vec![PlannedFile {
src,
dst: dst.clone(),
rel: PathBuf::from("shared.yaml"),
kind: PlannedKind::Conflict,
top_category: TopCategory::Macros,
}],
@@ -4195,6 +4512,7 @@ mod tests {
files: vec![PlannedFile {
src: dir.join("bundle-src/agents/foo/graph.yaml"),
dst: dir.join("agents/foo/graph.yaml"),
rel: PathBuf::from("foo/graph.yaml"),
kind: PlannedKind::New,
top_category: TopCategory::Agents,
}],
@@ -4237,6 +4555,7 @@ mod tests {
files: vec![PlannedFile {
src: dir.join("bundle-src/agents/foo/graph.yaml"),
dst: dir.join("agents/foo/graph.yaml"),
rel: PathBuf::from("foo/graph.yaml"),
kind: PlannedKind::New,
top_category: TopCategory::Agents,
}],
+80 -2
View File
@@ -1,6 +1,7 @@
mod agent;
mod app_config;
mod app_state;
pub(crate) mod builtin_manifest;
mod bundles;
mod input;
mod install_remote;
@@ -75,12 +76,14 @@ use crate::client::{
oauth, set_client_models_config,
};
use crate::function::{FunctionDeclaration, Functions};
use crate::hooks::HooksMap;
use crate::rag::Rag;
use crate::sandbox::SANDBOX_ENV_FLAG;
use crate::utils::*;
pub use macros::macro_execute;
use crate::config::macros::Macro;
use crate::hooks;
use crate::vault::{
GlobalVault, Vault, create_vault_password_file, interpolate_secrets, prompt_provider_choice,
};
@@ -162,6 +165,7 @@ const AGENT_GRAPH_FILE_NAME: &str = "graph.yaml";
const ROLES_DIR_NAME: &str = "roles";
const SKILLS_DIR_NAME: &str = "skills";
const MACROS_DIR_NAME: &str = "macros";
const HOOKS_DIR_NAME: &str = "hooks";
const ENV_FILE_NAME: &str = ".env";
const MESSAGES_FILE_NAME: &str = "messages.md";
const SESSIONS_DIR_NAME: &str = "sessions";
@@ -262,6 +266,9 @@ pub struct Config {
pub enabled_mcp_servers: Option<Vec<String>>,
pub mcp_tools: Option<IndexMap<String, Vec<String>>>,
#[serde(default)]
pub hooks: HooksMap,
pub auto_continue: bool,
pub max_auto_continues: usize,
pub inject_todo_instructions: bool,
@@ -350,6 +357,8 @@ impl Default for Config {
enabled_mcp_servers: None,
mcp_tools: None,
hooks: Default::default(),
auto_continue: false,
max_auto_continues: 10,
inject_todo_instructions: true,
@@ -412,6 +421,8 @@ pub fn install_builtins() -> Result<()> {
Agent::install_builtin_agents(false)?;
Macro::install_macros(false)?;
Skill::install_builtin_skills(false)?;
hooks::install_builtin_hooks(false)?;
Role::install_builtin_role_hooks(false)?;
Ok(())
}
@@ -421,12 +432,20 @@ pub enum AssetCategory {
Macros,
Functions,
Skills,
Hooks,
#[value(name = "mcp-config", alias = "mcp_config")]
McpConfig,
}
impl AssetCategory {
pub const NAMES: [&'static str; 5] = ["agents", "macros", "functions", "skills", "mcp-config"];
pub const NAMES: [&'static str; 6] = [
"agents",
"macros",
"functions",
"skills",
"hooks",
"mcp-config",
];
pub fn parse(name: &str) -> Option<Self> {
match name {
@@ -434,6 +453,7 @@ impl AssetCategory {
"macros" => Some(Self::Macros),
"functions" => Some(Self::Functions),
"skills" => Some(Self::Skills),
"hooks" => Some(Self::Hooks),
"mcp-config" | "mcp_config" => Some(Self::McpConfig),
_ => None,
}
@@ -453,17 +473,19 @@ pub enum InstallFilter {
Skills,
Macros,
Functions,
Hooks,
#[value(name = "mcp-config", alias = "mcp_config")]
McpConfig,
}
impl InstallFilter {
pub const NAMES: [&'static str; 6] = [
pub const NAMES: [&'static str; 7] = [
"agents",
"roles",
"skills",
"macros",
"functions",
"hooks",
"mcp-config",
];
@@ -474,6 +496,7 @@ impl InstallFilter {
"skills" => Some(Self::Skills),
"macros" => Some(Self::Macros),
"functions" => Some(Self::Functions),
"hooks" => Some(Self::Hooks),
"mcp-config" | "mcp_config" => Some(Self::McpConfig),
_ => None,
}
@@ -486,6 +509,7 @@ pub fn install_assets(category: AssetCategory) -> Result<()> {
AssetCategory::Macros => ("macros", paths::macros_dir()),
AssetCategory::Functions => ("functions", paths::functions_dir()),
AssetCategory::Skills => ("skills", paths::skills_dir()),
AssetCategory::Hooks => ("hooks", paths::hooks_dir()),
AssetCategory::McpConfig => ("MCP config", paths::mcp_config_file()),
};
@@ -499,6 +523,7 @@ pub fn install_assets(category: AssetCategory) -> Result<()> {
AssetCategory::Macros => Macro::install_macros(true)?,
AssetCategory::Functions => Functions::install_builtin_global_tools(true)?,
AssetCategory::Skills => Skill::install_builtin_skills(true)?,
AssetCategory::Hooks => hooks::install_builtin_hooks(true)?,
AssetCategory::McpConfig => Functions::install_mcp_config()?,
}
@@ -1393,6 +1418,59 @@ clients:
assert!(!CONFIG_TEMPLATE.contains("escalation_timeout"));
}
#[test]
fn config_template_carries_hooks_block_without_global_hooks() {
// `global_hooks` is a per-agent whitelist; the global template must never grow it.
assert!(CONFIG_TEMPLATE.contains("\nhooks:"));
assert!(!CONFIG_TEMPLATE.contains("global_hooks"));
}
#[test]
fn config_template_renders_with_empty_hooks() {
let clients = json!([{ "type": "openai", "api_key": "sk-test" }]);
let rendered = render_config_template("openai:gpt-4o", None, &clients).unwrap();
assert!(rendered.contains("\nhooks:"));
assert!(!rendered.contains("global_hooks"));
let cfg = Config::load_from_str(&rendered).unwrap();
assert!(cfg.hooks.is_empty());
}
#[test]
fn config_parses_hooks_map_preserving_order() {
let yaml = "\
hooks:
tool.started:
- name: notify
command: ./hooks/notify.sh
- name: audit
command: ./hooks/audit.sh
turn.completed:
- name: webhook
command: curl -s https://example.com/hook
";
let cfg: Config = serde_yaml::from_str(yaml).unwrap();
assert_eq!(
cfg.hooks.keys().collect::<Vec<_>>(),
vec!["tool.started", "turn.completed"]
);
assert_eq!(cfg.hooks["tool.started"].len(), 2);
assert_eq!(cfg.hooks["tool.started"][0].name, "notify");
assert_eq!(cfg.hooks["tool.started"][0].command, "./hooks/notify.sh");
assert_eq!(cfg.hooks["turn.completed"][0].name, "webhook");
}
#[test]
fn config_without_hooks_parses_unchanged() {
let cfg: Config = serde_yaml::from_str("model: openai:gpt-4o").unwrap();
assert!(cfg.hooks.is_empty());
assert_eq!(cfg.model_id, "openai:gpt-4o");
}
#[test]
fn config_enabled_macros_empty_string_is_some_empty() {
let cfg: Config = serde_yaml::from_str("enabled_macros: \"\"").unwrap();
+33 -1
View File
@@ -2,7 +2,7 @@ use super::role::Role;
use super::{
AGENT_GRAPH_FILE_NAME, AGENTS_DIR_NAME, BASH_PROMPT_UTILS_FILE_NAME, CONFIG_FILE_NAME,
ENV_FILE_NAME, FUNCTIONS_BIN_DIR_NAME, FUNCTIONS_DIR_NAME, GLOBAL_TOOLS_DIR_NAME,
GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, INSTALLED_BUNDLES_FILE_NAME,
GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, HOOKS_DIR_NAME, INSTALLED_BUNDLES_FILE_NAME,
MACROS_DIR_NAME, MCP_FILE_NAME, MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride,
RAGS_DIR_NAME, ROLES_DIR_NAME, SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME,
SBX_MIXIN_KITS_DIR_NAME, SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME,
@@ -184,6 +184,13 @@ pub fn role_file(name: &str) -> PathBuf {
roles_dir().join(format!("{name}.md"))
}
pub fn hooks_dir() -> PathBuf {
match env::var(get_env_name("hooks_dir")) {
Ok(value) => PathBuf::from(value),
Err(_) => local_dir(HOOKS_DIR_NAME),
}
}
pub fn skills_dir() -> PathBuf {
match env::var(get_env_name("skills_dir")) {
Ok(value) => PathBuf::from(value),
@@ -947,6 +954,31 @@ mod tests {
}
}
#[test]
#[serial_test::serial]
fn hooks_dir_reflects_env_var_state() {
let env_name = get_env_name("hooks_dir");
let prev = env::var_os(&env_name);
unsafe {
env::remove_var(&env_name);
}
assert_eq!(hooks_dir(), config_dir().join("hooks"));
let probe = PathBuf::from("/tmp/coyote-hooks-dir-probe");
unsafe {
env::set_var(&env_name, &probe);
}
assert_eq!(hooks_dir(), probe);
unsafe {
match prev {
Some(v) => env::set_var(&env_name, v),
None => env::remove_var(&env_name),
}
}
}
#[test]
fn list_skills_skips_invalid_directory_names() {
let unique = time::SystemTime::now()
+466
View File
@@ -50,6 +50,7 @@ use super::memory::{
DEFAULT_MEMORY_CAP_WITH_TOOLS, DEFAULT_MEMORY_CAP_WITHOUT_TOOLS, MemoryStore, WorkspaceMemory,
};
use crate::graph;
use crate::hooks::{self, HookEvent};
use anyhow::{Context, Error, Result, bail};
use colored::Colorize;
use gman::providers::SupportedProvider;
@@ -59,6 +60,7 @@ use inquire::{Confirm, MultiSelect, Text, list_option::ListOption, validator::Va
use log::warn;
use parking_lot::RwLock;
use prompts::DEFAULT_SKILL_INSTRUCTIONS;
use rand::distr::{Alphanumeric, SampleString};
use std::collections::{BTreeSet, HashMap, HashSet};
use std::fs::{File, OpenOptions, read_dir, read_to_string, remove_dir_all, remove_file};
use std::io::Write;
@@ -354,6 +356,13 @@ pub struct RequestContext {
pub session_abort: Option<AbortSignal>,
/// Synthesized `COYOTE_AGENT_ID` for a top-level agent run (headless
/// `--agent` dispatch or an interactive agent context). Scheme:
/// `top-<agent-name>-<8-char alphanumeric nonce>`; spawned sub-agents
/// carry supervisor-issued ids instead. `Some` exactly while the
/// top-level `agent.*` bracket is open.
pub top_level_agent_id: Option<String>,
pub render_mode: RenderMode,
}
@@ -398,6 +407,7 @@ impl RequestContext {
auto_continue_paused: None,
pending_prefill: None,
session_abort: None,
top_level_agent_id: None,
render_mode: RenderMode::default(),
}
}
@@ -469,6 +479,7 @@ impl RequestContext {
auto_continue_paused: None,
pending_prefill: None,
session_abort: None,
top_level_agent_id: None,
render_mode: RenderMode::default(),
};
ctx.refresh_mcp_tool_filters();
@@ -529,6 +540,9 @@ impl RequestContext {
auto_continue_paused: self.auto_continue_paused.clone(),
pending_prefill: None,
session_abort: self.session_abort.clone(),
// Branches never own the top-level `agent.*` bracket; only the
// root flow that opened it may fire its terminal event.
top_level_agent_id: None,
render_mode: self.render_mode,
}
}
@@ -585,6 +599,7 @@ impl RequestContext {
auto_continue_paused: None,
pending_prefill: None,
session_abort: None,
top_level_agent_id: None,
render_mode: parent.render_mode,
}
}
@@ -1054,6 +1069,12 @@ impl RequestContext {
}
pub fn exit_session(&mut self) -> Result<()> {
// Fired while the session is still attached so the hook sees
// COYOTE_SESSION_ID and session-held role hooks still resolve; no
// session means no event.
if self.session.is_some() {
hooks::fire(HookEvent::SessionEnded, self, &[], None);
}
if let Some(mut session) = self.session.take() {
let sessions_dir = self.sessions_dir();
session.exit(&sessions_dir, self.working_mode.is_repl())?;
@@ -4613,6 +4634,7 @@ impl RequestContext {
);
}
let mut session;
let mut created_new_session = false;
match session_name {
None | Some(TEMP_SESSION_NAME) => {
let session_file = self.session_file(TEMP_SESSION_NAME);
@@ -4622,11 +4644,13 @@ impl RequestContext {
})?;
}
session = Some(Session::new_from_ctx(self, app, TEMP_SESSION_NAME)?);
created_new_session = true;
}
Some(name) => {
let session_path = self.session_file(name);
if !session_path.exists() {
session = Some(Session::new_from_ctx(self, app, name)?);
created_new_session = true;
} else {
session = Some(Session::load_from_ctx(self, app, name, &session_path)?);
}
@@ -4709,6 +4733,9 @@ impl RequestContext {
}
self.refresh_mcp_tool_filters();
self.init_agent_session_variables(new_session)?;
if created_new_session {
hooks::fire(HookEvent::SessionStarted, self, &[], None);
}
Ok(())
}
@@ -4874,6 +4901,9 @@ impl RequestContext {
tool_tracker,
};
// Interactive switch-away closes the top-level agent bracket while
// the agent is still attached, so agent-scoped hooks resolve.
self.top_level_agent_finished(None);
if self.agent.take().is_some() {
if let Some(supervisor) = self.supervisor.clone() {
supervisor.read().cancel_recursive();
@@ -4901,6 +4931,50 @@ impl RequestContext {
Ok(())
}
pub fn top_level_agent_started(&mut self) {
if self.top_level_agent_id.is_some() {
return;
}
let Some(agent) = self.agent.as_ref() else {
return;
};
let nonce = Alphanumeric.sample_string(&mut rand::rng(), 8);
let id = format!("top-{}-{nonce}", agent.name());
self.top_level_agent_id = Some(id.clone());
hooks::fire(
HookEvent::AgentStarted,
self,
&[("COYOTE_AGENT_ID", id)],
None,
);
}
pub fn top_level_agent_finished(&mut self, error: Option<&Error>) {
let Some(id) = self.top_level_agent_id.take() else {
return;
};
match error {
None => hooks::fire(
HookEvent::AgentCompleted,
self,
&[("COYOTE_AGENT_ID", id)],
None,
),
Some(err) => hooks::fire(
HookEvent::AgentFailed,
self,
&[
("COYOTE_AGENT_ID", id),
("COYOTE_AGENT_ERROR", format!("{err:#}")),
],
None,
),
}
}
pub async fn edit_role(&mut self, app: &AppConfig, abort_signal: AbortSignal) -> Result<()> {
let role_name;
if let Some(session) = self.session.as_ref() {
@@ -5421,6 +5495,7 @@ mod tests {
use crate::config::tool_scope::test_fixtures::{FixtureServer, fixture_runtime};
use crate::function::jobs::RingBuf;
use crate::function::{ToolCall, skill};
use crate::hooks::{HookDef, HooksMap, test_sink};
use crate::mcp::{McpServer, McpServerFeatures, McpServersConfig, McpTransportType};
use crate::supervisor::{
AgentExitStatus, AgentHandle, AgentResult, JobHandle, JobResult, JobState, JobStatus,
@@ -5428,6 +5503,7 @@ mod tests {
use crate::utils;
use crate::utils::get_env_name;
use crate::vault::Vault;
use anyhow::anyhow;
use rmcp::model::PromptArgument;
use serde_json::json;
use serial_test::serial;
@@ -8230,6 +8306,349 @@ mod tests {
assert!(ctx.session.is_none());
}
fn hooks_map_of(events: &[&str], marker: &str) -> HooksMap {
let mut map = HooksMap::default();
for event in events {
map.insert(
event.to_string(),
vec![HookDef {
name: format!("{marker}-{event}"),
command: "true".to_string(),
}],
);
}
map
}
fn marker_captures(marker: &str) -> Vec<test_sink::Capture> {
test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect()
}
/// Writes an agent whose config carries agent-scoped hooks for every
/// `agent.*` event, named `<marker>-<event>`.
fn seed_agent_with_hooks(agent_name: &str, marker: &str) {
let agent_dir = paths::agent_data_dir(agent_name);
create_dir_all(&agent_dir).unwrap();
let mut config = format!("name: {agent_name}\ninstructions: hi\nhooks:\n");
for event in ["agent.started", "agent.completed", "agent.failed"] {
config.push_str(&format!(
" {event}:\n - name: {marker}-{event}\n command: 'true'\n"
));
}
write(agent_dir.join("config.yaml"), config).unwrap();
}
fn unique_name(prefix: &str) -> String {
format!(
"{prefix}_{}",
SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos()
)
}
#[test]
#[serial]
fn use_session_fires_session_started_for_new_sessions_only() {
let _guard = TestConfigDirGuard::new();
let sessions_dir = paths::local_dir("sessions");
create_dir_all(&sessions_dir).unwrap();
let _sink = test_sink::install();
let marker = "sess-started-q8n";
let mut ctx = create_test_ctx();
ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.started"], marker));
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap();
let captures = marker_captures(marker);
assert_eq!(captures.len(), 1, "a new session fires exactly one event");
assert_eq!(
captures[0]
.envs
.get("COYOTE_SESSION_ID")
.map(String::as_str),
Some("hook-fresh")
);
// Resuming a persisted session fires nothing.
write_paused_todo_session(&ctx, "hook-resume");
let mut resumed_ctx = create_test_ctx();
resumed_ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.started"], marker));
let resumed_app = resumed_ctx.app.config.clone();
run_async(resumed_ctx.use_session(&resumed_app, Some("hook-resume"), abort)).unwrap();
assert_eq!(
marker_captures(marker).len(),
1,
"resume must not fire session.started"
);
}
#[test]
#[serial]
fn exit_session_fires_session_ended_only_with_a_session() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "sess-ended-w5c";
let mut ctx = create_test_ctx();
ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.ended"], marker));
// Sessionless (headless) exit path: nothing fires.
ctx.exit_session().unwrap();
assert!(marker_captures(marker).is_empty());
ctx.session = Some(Session::default());
ctx.exit_session().unwrap();
ctx.exit_session().unwrap();
assert_eq!(
marker_captures(marker).len(),
1,
"session.ended fires exactly once, only while a session is attached"
);
}
#[test]
#[serial]
fn chat_completion_seams_fire_no_turn_events_in_child_contexts() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "child-turn-b4h";
let mut ctx = create_test_ctx();
ctx.update_app_config(|app| {
app.hooks = hooks_map_of(
&[
"turn.started",
"turn.completed",
"turn.interrupted",
"turn.failed",
],
marker,
)
});
let app = ctx.app.config.clone();
let input = Input::from_str(&ctx, "hello", None).unwrap();
ctx.before_chat_completion(&input).unwrap();
ctx.after_chat_completion(&app, &input, "output", &[])
.unwrap();
assert!(
marker_captures(marker).is_empty(),
"per-round chat-completion seams must never fire turn events"
);
}
#[test]
#[serial]
fn top_level_agent_bracket_fires_started_and_completed_once() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-ok-m2s";
let agent_name = unique_name("hook_agent");
seed_agent_with_hooks(&agent_name, marker);
let mut ctx = create_test_ctx();
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
ctx.top_level_agent_started();
// Per agent RUN, never per message: re-entering while open is a no-op.
ctx.top_level_agent_started();
ctx.top_level_agent_finished(None);
ctx.top_level_agent_finished(None);
let captures = marker_captures(marker);
let started: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.started"))
.collect();
let completed: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.completed"))
.collect();
assert_eq!(started.len(), 1);
assert_eq!(completed.len(), 1);
assert!(
captures
.iter()
.all(|capture| capture.hook_name != format!("{marker}-agent.failed"))
);
let id = started[0]
.envs
.get("COYOTE_AGENT_ID")
.expect("agent id env");
assert!(
id.starts_with(&format!("top-{agent_name}-")),
"synthesized id must follow the top-<agent>-<nonce> scheme: {id}"
);
assert_eq!(completed[0].envs.get("COYOTE_AGENT_ID"), Some(id));
assert_eq!(
started[0].envs.get("COYOTE_AGENT_NAME").map(String::as_str),
Some(agent_name.as_str())
);
}
#[test]
#[serial]
fn top_level_agent_failure_fires_agent_failed_with_error() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-err-f7d";
let agent_name = unique_name("hook_agent");
seed_agent_with_hooks(&agent_name, marker);
let mut ctx = create_test_ctx();
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
ctx.top_level_agent_started();
let err = anyhow!("dispatch exploded");
ctx.top_level_agent_finished(Some(&err));
let captures = marker_captures(marker);
let failed: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.failed"))
.collect();
assert_eq!(failed.len(), 1);
assert!(
captures
.iter()
.all(|capture| capture.hook_name != format!("{marker}-agent.completed"))
);
assert_eq!(
failed[0].envs.get("COYOTE_AGENT_ERROR").map(String::as_str),
Some("dispatch exploded")
);
assert!(failed[0].envs.contains_key("COYOTE_AGENT_ID"));
}
#[test]
#[serial]
fn top_level_agent_helpers_noop_without_agent() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-role-only-c9k";
let mut ctx = create_test_ctx();
ctx.update_app_config(|app| {
app.hooks = hooks_map_of(
&["agent.started", "agent.completed", "agent.failed"],
marker,
)
});
ctx.role = Some(Role::new("plain", "prompt"));
ctx.top_level_agent_started();
ctx.top_level_agent_finished(None);
assert!(
marker_captures(marker).is_empty(),
"a role-only context must fire no agent events"
);
assert!(ctx.top_level_agent_id.is_none());
}
#[test]
#[serial]
fn exit_agent_closes_top_level_agent_bracket() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-exit-v3p";
let agent_name = unique_name("hook_agent");
seed_agent_with_hooks(&agent_name, marker);
let mut ctx = create_test_ctx();
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
ctx.top_level_agent_started();
ctx.exit_agent(&app).unwrap();
// A later exit seam (REPL teardown) must not double-fire.
ctx.top_level_agent_finished(None);
let captures = marker_captures(marker);
assert_eq!(
captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.completed"))
.count(),
1,
"switch-away fires the terminal event exactly once"
);
}
#[test]
#[serial]
fn resolved_hooks_resolves_role_hooks_held_by_fresh_session() {
let _guard = TestConfigDirGuard::new();
let sessions_dir = paths::local_dir("sessions");
create_dir_all(&sessions_dir).unwrap();
let mut ctx = create_test_ctx();
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_session(&app, Some("role-holder"), abort)).unwrap();
let role_content = "---\nhooks:\n turn.completed:\n - name: session-role-probe\n command: role-cmd\n---\nPrompt";
ctx.use_role_obj(Role::new("hooked-role", role_content))
.unwrap();
assert!(
ctx.role.is_none(),
"use_role_obj must move the role into the active session"
);
let resolved = ctx.resolved_hooks(HookEvent::TurnCompleted);
assert_eq!(resolved.len(), 1);
assert_eq!(resolved[0].name, "session-role-probe");
assert_eq!(resolved[0].command, "role-cmd");
assert_eq!(resolved[0].cwd, paths::roles_dir());
}
#[test]
#[serial]
fn resolved_hooks_resolves_role_hooks_after_session_resume() {
let _guard = TestConfigDirGuard::new();
let sessions_dir = paths::local_dir("sessions");
create_dir_all(&sessions_dir).unwrap();
let roles_dir = paths::roles_dir();
create_dir_all(&roles_dir).unwrap();
write(
roles_dir.join("resume-hooked.md"),
"---\nhooks:\n session.ended:\n - name: resume-role-probe\n command: role-cmd\n---\nYou are hooked.",
)
.unwrap();
let seed_ctx = create_test_ctx();
let session_path = seed_ctx.session_file("resume-role");
ensure_parent_exists(&session_path).unwrap();
write(
&session_path,
"model: test-seeded:test-chat\nrole_name: resume-hooked\nmessages: []\n",
)
.unwrap();
let mut ctx = create_test_ctx();
let app = ctx.app.config.clone();
let abort = utils::create_abort_signal();
run_async(ctx.use_session(&app, Some("resume-role"), abort)).unwrap();
assert!(ctx.role.is_none());
let resolved = ctx.resolved_hooks(HookEvent::SessionEnded);
assert_eq!(resolved.len(), 1, "resumed session must restore role hooks");
assert_eq!(resolved[0].name, "resume-role-probe");
}
fn write_paused_todo_session(ctx: &RequestContext, name: &str) {
let session_path = ctx.session_file(name);
ensure_parent_exists(&session_path).unwrap();
@@ -8859,6 +9278,53 @@ mod tests {
assert!(agents_dir.join("architect").join("config.yaml").exists());
}
#[test]
#[serial]
fn install_builtin_agents_reconciles_hooks_subdir_via_manifest() {
use crate::config::builtin_manifest::BUILTIN_MANIFEST_FILE;
let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap();
let agents_dir = paths::agents_data_dir();
// Simulate an upgrade from an older install whose coder shipped a
// hook script that the current embed no longer contains: the manifest
// records it, so reconciliation removes it — while a user-created
// script in the same directory (absent from the manifest) survives.
let hooks_dir = agents_dir.join("coder").join("hooks");
create_dir_all(&hooks_dir).unwrap();
write(hooks_dir.join(BUILTIN_MANIFEST_FILE), "old-hook.sh\n").unwrap();
write(hooks_dir.join("old-hook.sh"), "stale").unwrap();
write(hooks_dir.join("user-hook.sh"), "user-owned").unwrap();
// A custom agent outside the bundle is never reconciled, even with a
// manifest present.
let custom_hooks = agents_dir.join("myagent").join("hooks");
create_dir_all(&custom_hooks).unwrap();
write(custom_hooks.join(BUILTIN_MANIFEST_FILE), "custom.sh\n").unwrap();
write(custom_hooks.join("custom.sh"), "keep").unwrap();
Agent::install_builtin_agents(false).unwrap();
assert!(
!hooks_dir.join("old-hook.sh").exists(),
"manifest-listed hook absent from the embed must be removed"
);
assert!(
hooks_dir.join("user-hook.sh").exists(),
"user-created hook files must survive reconciliation"
);
assert!(
!hooks_dir.join(BUILTIN_MANIFEST_FILE).exists(),
"an empty shipped set removes the manifest itself"
);
assert!(
custom_hooks.join("custom.sh").exists(),
"custom agents outside the bundle must survive reconciliation"
);
assert!(custom_hooks.join(BUILTIN_MANIFEST_FILE).exists());
}
#[test]
#[serial]
fn install_builtin_skills_force_overwrites_only_with_force() {
+240
View File
@@ -1,12 +1,17 @@
use super::*;
use crate::client::{Message, MessageContent, MessageRole, Model};
use crate::config::builtin_manifest;
use crate::function::write_file_atomic;
use crate::hooks::HooksMap;
use anyhow::Result;
use fancy_regex::Regex;
use rust_embed::Embed;
use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::BTreeSet;
use std::path::Path;
use std::sync::LazyLock;
pub const SHELL_ROLE: &str = "shell";
@@ -99,6 +104,8 @@ pub struct Role {
skill_instructions: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
memory: Option<bool>,
#[serde(default, skip_serializing_if = "Option::is_none")]
hooks: Option<HooksMap>,
#[serde(skip)]
model: Model,
@@ -161,6 +168,7 @@ impl Role {
role.skill_instructions = value.as_str().map(|v| v.to_string())
}
"memory" => role.memory = value.as_bool(),
"hooks" => role.hooks = parse_hooks_map(value),
_ => (),
}
}
@@ -181,6 +189,26 @@ impl Role {
.collect()
}
/// Installs builtin role hook scripts into `<roles_dir>/hooks/` and
/// reconciles ones a release stopped shipping. This is the only builtin
/// install machinery roles have: general role installation deliberately
/// does not exist (builtin roles load straight from the embed).
pub fn install_builtin_role_hooks(force: bool) -> Result<()> {
let shipped: Vec<(String, String)> = RolesAsset::iter()
.filter_map(|file| {
let name = file.as_ref().strip_prefix("hooks/")?;
if name.is_empty() || name.contains('/') {
return None;
}
let embedded = RolesAsset::get(file.as_ref())?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded.data) };
Some((name.to_string(), content.to_string()))
})
.collect();
install_and_reconcile_role_hooks(&paths::roles_dir().join("hooks"), &shipped, force)
}
pub fn has_args(&self) -> bool {
self.name.contains('#')
}
@@ -248,6 +276,10 @@ impl Role {
if let Some(memory) = self.memory {
metadata.push(format!("memory: {memory}"));
}
if let Some(hooks) = &self.hooks {
let inline = serde_json::to_string(hooks).unwrap_or_else(|_| "{}".to_string());
metadata.push(format!("hooks: {inline}"));
}
if metadata.is_empty() {
format!("{}\n", self.prompt)
} else if self.prompt.is_empty() {
@@ -380,6 +412,10 @@ impl Role {
self.memory
}
pub fn hooks(&self) -> Option<&HooksMap> {
self.hooks.as_ref()
}
pub fn skills_enabled(&self) -> Option<bool> {
self.skills_enabled
}
@@ -544,6 +580,16 @@ fn parse_mcp_tools_map(value: &Value) -> Option<IndexMap<String, Vec<String>>> {
Some(mcp_tools)
}
fn parse_hooks_map(value: &Value) -> Option<HooksMap> {
match serde_json::from_value(value.clone()) {
Ok(map) => Some(map),
Err(err) => {
debug!("Ignoring malformed hooks configuration: {err}");
None
}
}
}
fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) {
let mut text = prompt;
let mut search_input = true;
@@ -592,6 +638,46 @@ fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) {
(prompt, vec![])
}
/// Only direct children of the hooks/ directory are supported: they are the
/// only shape the builtin manifest tracks, so nested asset paths are skipped
/// by the caller rather than installed unreconciled.
fn install_and_reconcile_role_hooks(
dir: &Path,
shipped: &[(String, String)],
force: bool,
) -> Result<()> {
if !shipped.is_empty() {
info!("Installing built-in role hooks in {}", dir.display());
}
let mut written = BTreeSet::new();
for (name, content) in shipped {
let path = dir.join(name);
if path.exists() && !force {
debug!(
"Role hook file already exists, skipping: {}",
path.display()
);
continue;
}
ensure_parent_exists(&path)?;
info!("Creating role hook file: {}", path.display());
write_file_atomic(&path, content, Some(0o755))?;
written.insert(name.clone());
}
let names: BTreeSet<String> = shipped.iter().map(|(name, _)| name.clone()).collect();
// Reconciliation is best-effort housekeeping: a failure here must not
// abort startup (install_builtins), matching the agent-side policy.
if let Err(err) = builtin_manifest::reconcile_builtin_dir(dir, &names, &written) {
warn!(
"Failed to reconcile builtin role hooks in {}: {err}",
dir.display()
);
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
@@ -641,6 +727,42 @@ mod tests {
);
}
#[test]
fn role_hooks_round_trip_through_export() {
let content = "---\nhooks:\n turn.completed:\n - name: notify\n command: ./hooks/notify.sh\n---\nPrompt";
let role = Role::new("test", content);
let hooks = role.hooks().expect("hooks should be parsed");
assert_eq!(hooks["turn.completed"][0].name, "notify");
assert_eq!(hooks["turn.completed"][0].command, "./hooks/notify.sh");
let exported = role.export();
assert!(exported.contains("hooks:"));
let reparsed = Role::new("test", &exported);
assert_eq!(reparsed.hooks(), role.hooks());
assert_eq!(reparsed.prompt(), "Prompt");
}
#[test]
fn role_without_hooks_has_none_and_export_omits_key() {
let role = Role::new("test", "---\nmemory: true\n---\nPrompt");
assert!(role.hooks().is_none());
assert!(!role.export().contains("hooks:"));
}
#[test]
fn role_malformed_hooks_degrades_to_none() {
let content = "---\nhooks: [not, a, map]\n---\nPrompt";
let role = Role::new("test", content);
assert!(role.hooks().is_none());
assert_eq!(role.prompt(), "Prompt");
}
#[test]
fn role_new_no_metadata_has_none_fields() {
let role = Role::new("test", "Just a prompt");
@@ -909,4 +1031,122 @@ Input 1
assert_eq!(parse_structure_prompt(prompt), (prompt, vec![]));
}
fn fixture(entries: &[(&str, &str)]) -> Vec<(String, String)> {
entries
.iter()
.map(|(name, content)| (name.to_string(), content.to_string()))
.collect()
}
fn hooks_fixture_dir(label: &str) -> PathBuf {
let dir = temp_file(label, "");
create_dir_all(&dir).unwrap();
dir.join("hooks")
}
#[test]
fn role_hooks_install_writes_scripts_and_manifest() {
let dir = hooks_fixture_dir("role-hooks-install-");
install_and_reconcile_role_hooks(&dir, &fixture(&[("a.sh", "#!/bin/sh\n")]), false)
.unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "#!/bin/sh\n");
assert_eq!(
read_to_string(dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE)).unwrap(),
"a.sh\n"
);
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
let mode = fs::metadata(dir.join("a.sh")).unwrap().permissions().mode();
assert_eq!(mode & 0o777, 0o755, "role hook scripts install executable");
let manifest_mode = fs::metadata(dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE))
.unwrap()
.permissions()
.mode();
assert_eq!(
manifest_mode & 0o111,
0,
"the manifest must never be executable"
);
}
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test]
fn role_hooks_install_overwrites_only_with_force() {
let dir = hooks_fixture_dir("role-hooks-force-");
let shipped = fixture(&[("a.sh", "new content\n")]);
create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "SENTINEL").unwrap();
install_and_reconcile_role_hooks(&dir, &shipped, false).unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "SENTINEL");
install_and_reconcile_role_hooks(&dir, &shipped, true).unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "new content\n");
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test]
fn role_hooks_reconcile_removes_dropped_hook_and_keeps_user_files() {
let dir = hooks_fixture_dir("role-hooks-reconcile-");
install_and_reconcile_role_hooks(
&dir,
&fixture(&[("keep.sh", "#!/bin/sh\n"), ("drop.sh", "#!/bin/sh\n")]),
false,
)
.unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
install_and_reconcile_role_hooks(&dir, &fixture(&[("keep.sh", "#!/bin/sh\n")]), false)
.unwrap();
assert!(
!dir.join("drop.sh").exists(),
"dropped builtin hook removed"
);
assert!(dir.join("keep.sh").exists());
assert!(dir.join("user.sh").exists(), "user file survives reconcile");
assert_eq!(
read_to_string(dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE)).unwrap(),
"keep.sh\n"
);
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test]
fn role_hooks_never_claim_a_preexisting_user_file() {
let dir = hooks_fixture_dir("role-hooks-unclaimed-");
create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "user-owned").unwrap();
install_and_reconcile_role_hooks(&dir, &fixture(&[("a.sh", "#!/bin/sh\n")]), false)
.unwrap();
assert!(
!dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE).exists(),
"a skipped pre-existing file must not be claimed by the manifest"
);
install_and_reconcile_role_hooks(&dir, &[], false).unwrap();
assert_eq!(
read_to_string(dir.join("a.sh")).unwrap(),
"user-owned",
"the user file survives the hook being dropped from the embed"
);
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test]
fn role_hooks_empty_shipped_set_creates_nothing() {
let dir = hooks_fixture_dir("role-hooks-empty-");
install_and_reconcile_role_hooks(&dir, &[], false).unwrap();
assert!(!dir.exists(), "an empty shipped set must not create hooks/");
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
}
+64
View File
@@ -3,6 +3,7 @@ use super::todo::TodoList;
use super::*;
use crate::client::{Message, MessageContent, MessageRole, TokenUsage};
use crate::hooks::HooksMap;
use crate::render::MarkdownRender;
use anyhow::{Context, Result, bail};
@@ -113,6 +114,8 @@ pub struct Session {
#[serde(skip)]
role_prompt: String,
#[serde(skip)]
role_hooks: Option<HooksMap>,
#[serde(skip)]
name: String,
#[serde(skip)]
path: Option<String>,
@@ -193,6 +196,7 @@ impl Session {
&& let Ok(role) = ctx.retrieve_role(app, role_name)
{
session.role_prompt = role.prompt().to_string();
session.role_hooks = role.hooks().cloned();
}
session.update_tokens();
@@ -254,6 +258,13 @@ impl Session {
self.role_name.as_deref()
}
/// Hooks of the role this session holds. `set_role` snapshots them
/// because only the frontmatter-stripped role prompt is persisted;
/// `load_from_ctx` restores them by re-reading the role file on resume.
pub fn role_hooks(&self) -> Option<&HooksMap> {
self.role_hooks.as_ref()
}
pub fn dirty(&self) -> bool {
self.dirty
}
@@ -561,6 +572,7 @@ impl Session {
self.model = role.model().clone();
self.role_name = convert_option_string(role.name());
self.role_prompt = role.prompt().to_string();
self.role_hooks = role.hooks().cloned();
self.dirty = true;
self.update_tokens();
}
@@ -568,11 +580,13 @@ impl Session {
pub fn clear_role(&mut self) {
self.role_name = None;
self.role_prompt.clear();
self.role_hooks = None;
}
pub fn sync_agent(&mut self, agent: &Agent) {
self.role_name = None;
self.role_prompt = agent.interpolated_instructions();
self.role_hooks = None;
self.agent_variables = agent.variables().clone();
self.agent_instructions = self.role_prompt.clone();
if let Some(threshold) = agent.compression_threshold() {
@@ -1111,6 +1125,25 @@ mod tests {
assert!(!session.dirty());
}
#[test]
fn set_role_captures_and_clear_role_drops_role_hooks() {
let content = "---\nhooks:\n turn.completed:\n - name: role-probe\n command: role-cmd\n---\nPrompt";
let mut session = Session::default();
session.set_role(Role::new("hooked", content));
let hooks = session.role_hooks().expect("set_role must snapshot hooks");
assert_eq!(hooks["turn.completed"][0].name, "role-probe");
session.clear_role();
assert!(session.role_hooks().is_none());
session.set_role(Role::new("plain", "No frontmatter"));
assert!(
session.role_hooks().is_none(),
"a role without hooks must not leave a stale snapshot"
);
}
fn push_interrupted_turn(session: &mut Session) {
session.messages.push(Message::new(
MessageRole::User,
@@ -1233,6 +1266,37 @@ mod tests {
assert!(!yaml.contains("token_usage"));
}
#[test]
fn session_role_hooks_is_not_serialized() {
let content = "---\nhooks:\n turn.completed:\n - name: role-probe\n command: role-cmd\n---\nPrompt";
let mut session = Session::default();
session.set_role(Role::new("hooked", content));
assert!(session.role_hooks().is_some());
let yaml = serde_yaml::to_string(&session).unwrap();
assert!(
!yaml.contains("role_hooks"),
"the role-hooks snapshot must never reach session YAML"
);
assert!(
!yaml.contains("role-cmd"),
"no hook command text may leak into session YAML under any key"
);
}
#[test]
fn session_deserialization_ignores_injected_role_hooks() {
// A session file is attacker-writable data: a `role_hooks:` key
// crafted into it must never deserialize into executable hook
// config. Hooks are only ever re-read from the role file itself.
let yaml = "model: ''\nmessages: []\nrole_hooks:\n session.started:\n - name: injected\n command: touch /tmp/pwned\n";
let session: Session = serde_yaml::from_str(yaml).unwrap();
assert!(
session.role_hooks().is_none(),
"injected role_hooks in session YAML must be ignored"
);
}
#[test]
fn session_cost_accumulates_and_survives_yaml_round_trip() {
let mut session = Session::default();
+249 -2
View File
@@ -4,8 +4,9 @@ use crate::client::{Model, ModelType, call_chat_completions};
use crate::config::{
Agent, AgentVariable, AgentVariables, AppState, Input, RequestContext, Role, RoleLike,
default_max_agent_depth, effective_max_concurrent_jobs, jobs_enabled,
list_agents_with_descriptions, load_agent_variables,
list_agents_with_descriptions, load_agent_variables, paths,
};
use crate::hooks::{self, HookEvent, ResolvedHook};
use crate::supervisor::mailbox::{Envelope, EnvelopePayload, Inbox, PeerRegistry, graph_agent_id};
use crate::supervisor::notification::agent_notification;
use crate::supervisor::{AgentExitStatus, AgentHandle, AgentResult, Supervisor, TaskKind};
@@ -14,7 +15,7 @@ use crate::utils::{AbortSignal, create_abort_signal, wait_abort_signal, wait_use
use crate::graph;
use crate::repl::DEFAULT_CONTINUATION_PROMPT;
use anyhow::{Context, Result, anyhow, bail};
use chrono::Utc;
use chrono::{SecondsFormat, Utc};
use indexmap::IndexMap;
use log::{debug, warn};
use parking_lot::RwLock;
@@ -996,6 +997,86 @@ fn sync_agent_functions_to_ctx(ctx: &mut RequestContext) -> Result<()> {
Ok(())
}
/// Mirrors the base-env contract `hooks::fire` assembles from a live context,
/// for detached call sites that dispatch pre-resolved hook snapshots after
/// the context they were resolved from is gone. The names are captured by
/// the caller when it takes the snapshot; the timestamp is taken here, at
/// fire time, so it reflects when the event actually happened.
pub(crate) fn hook_base_envs(
event: HookEvent,
session_name: Option<&str>,
agent_name: Option<&str>,
) -> Vec<(String, String)> {
let mut envs = vec![
("COYOTE_EVENT".to_string(), event.as_str().to_string()),
(
"COYOTE_CONFIG_DIR".to_string(),
paths::config_dir().display().to_string(),
),
(
"COYOTE_EVENT_TIMESTAMP".to_string(),
Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true),
),
];
if let Some(name) = session_name {
envs.push(("COYOTE_SESSION_ID".to_string(), name.to_string()));
}
if let Some(name) = agent_name {
envs.push(("COYOTE_AGENT_NAME".to_string(), name.to_string()));
}
envs
}
/// Pre-resolved `agent.completed`/`agent.failed` snapshots for a spawned
/// agent's result arms, captured from the child context before the spawn
/// task takes ownership of it. Resolving against the child keeps global
/// hooks gated by the child's own whitelist, the same gate every other
/// event fired from the child's context goes through.
struct SpawnResultHooks {
completed: Vec<ResolvedHook>,
failed: Vec<ResolvedHook>,
session_name: Option<String>,
agent_id: String,
agent_name: String,
}
impl SpawnResultHooks {
fn resolve(child_ctx: &RequestContext, agent_id: &str, agent_name: &str) -> Self {
Self {
completed: child_ctx.resolved_hooks(HookEvent::AgentCompleted),
failed: child_ctx.resolved_hooks(HookEvent::AgentFailed),
session_name: child_ctx
.session
.as_ref()
.map(|session| session.name().to_string()),
agent_id: agent_id.to_string(),
agent_name: agent_name.to_string(),
}
}
/// Fires `agent.completed`, or `agent.failed` when `error` is given.
fn fire(self, error: Option<&str>) {
let mut extras = vec![
("COYOTE_AGENT_ID", self.agent_id),
("COYOTE_AGENT_NAME", self.agent_name.clone()),
];
let (event, resolved) = match error {
None => (HookEvent::AgentCompleted, self.completed),
Some(error) => {
extras.push(("COYOTE_AGENT_ERROR", error.to_string()));
(HookEvent::AgentFailed, self.failed)
}
};
hooks::fire_resolved(
event,
resolved,
hook_base_envs(event, self.session_name.as_deref(), Some(&self.agent_name)),
&extras,
None,
);
}
}
async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let agent_name = args
.get("agent")
@@ -1149,6 +1230,19 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
debug!("Spawning child agent '{agent_name}' as '{agent_id}'");
hooks::fire(
HookEvent::AgentStarted,
&child_ctx,
&[
("COYOTE_AGENT_ID", agent_id.clone()),
("COYOTE_AGENT_NAME", agent_name.clone()),
],
None,
);
// Resolved before the spawn: the result arms outlive the child context
// the snapshots come from.
let result_hooks = SpawnResultHooks::resolve(&child_ctx, &agent_id, &agent_name);
let spawn_agent_id = agent_id.clone();
let spawn_agent_name = agent_name.clone();
let spawn_abort = child_abort.clone();
@@ -1173,6 +1267,10 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
},
};
let success = agent_result.exit_status == AgentExitStatus::Completed;
match &agent_result.exit_status {
AgentExitStatus::Completed => result_hooks.fire(None),
AgentExitStatus::Failed(e) => result_hooks.fire(Some(e)),
}
spawn_notifications.push(agent_notification(
&agent_result.id,
&agent_result.agent_name,
@@ -2154,6 +2252,155 @@ mod tests {
.unwrap();
}
/// A graph agent whose graph.yaml wires `agent.*` hooks at its top
/// level. With `failing`, the start node references a script that does
/// not exist, so the child run fails at runtime — after the spawn seam
/// has already fired `agent.started`.
fn write_graph_agent_with_hooks(agent_name: &str, marker: &str, failing: bool) {
let agent_dir = paths::agent_data_dir(agent_name);
create_dir_all(&agent_dir).unwrap();
let (start, nodes) = if failing {
(
"work",
" work:\n type: script\n script: missing.sh\n next: done\n done:\n type: end\n output: done\n",
)
} else {
("done", " done:\n type: end\n output: done\n")
};
write(
agent_dir.join("graph.yaml"),
format!(
"name: {agent_name}\n\
start: {start}\n\
settings:\n\
\x20 validate_before_run: false\n\
hooks:\n\
\x20 agent.started:\n\
\x20 - name: {marker}_started\n\
\x20 command: \"true\"\n\
\x20 agent.completed:\n\
\x20 - name: {marker}_completed\n\
\x20 command: \"true\"\n\
\x20 agent.failed:\n\
\x20 - name: {marker}_failed\n\
\x20 command: \"true\"\n\
nodes:\n\
{nodes}"
),
)
.unwrap();
}
fn hook_captures_named(name: &str) -> Vec<hooks::test_sink::Capture> {
hooks::test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == name)
.collect()
}
/// Spawns an agent through the supervisor seam and awaits its spawn
/// task, returning the spawn id. The result arms fire their hooks
/// before that task returns, so the sink is complete afterwards.
fn spawn_and_collect(agent_name: &str) -> String {
run_async(async {
let mut ctx = ctx_with_supervisor(4, 3);
let spawned = handle_spawn(&mut ctx, &json!({"agent": agent_name, "prompt": "hi"}))
.await
.unwrap();
assert_eq!(spawned["status"], "ok", "{spawned}");
let id = spawned["id"].as_str().unwrap().to_string();
let handle = ctx
.supervisor
.as_ref()
.unwrap()
.write()
.take(&id)
.expect("spawned agent must be registered");
handle
.join_handle
.await
.expect("spawn task must not panic")
.expect("spawn task must return an AgentResult");
id
})
}
#[test]
#[serial]
fn spawned_agent_success_fires_started_and_completed_with_matching_id() {
let _guard = TestConfigDirGuard::new();
let _sink = hooks::test_sink::install();
let agent_name = unique_agent_name("test_hook_spawn_ok");
write_graph_agent_with_hooks(&agent_name, "t037_spawn_ok", false);
let id = spawn_and_collect(&agent_name);
let started = hook_captures_named("t037_spawn_ok_started");
assert_eq!(started.len(), 1, "{started:?}");
assert_eq!(started[0].envs["COYOTE_EVENT"], "agent.started");
assert_eq!(started[0].envs["COYOTE_AGENT_ID"], id);
assert_eq!(started[0].envs["COYOTE_AGENT_NAME"], agent_name);
let completed = hook_captures_named("t037_spawn_ok_completed");
assert_eq!(completed.len(), 1, "{completed:?}");
assert_eq!(completed[0].envs["COYOTE_EVENT"], "agent.completed");
assert_eq!(completed[0].envs["COYOTE_AGENT_ID"], id);
assert_eq!(completed[0].envs["COYOTE_AGENT_NAME"], agent_name);
assert!(hook_captures_named("t037_spawn_ok_failed").is_empty());
}
#[test]
#[serial]
fn spawned_agent_failure_fires_started_and_failed_with_matching_id() {
let _guard = TestConfigDirGuard::new();
let _sink = hooks::test_sink::install();
let agent_name = unique_agent_name("test_hook_spawn_fail");
write_graph_agent_with_hooks(&agent_name, "t037_spawn_fail", true);
let id = spawn_and_collect(&agent_name);
let started = hook_captures_named("t037_spawn_fail_started");
assert_eq!(started.len(), 1, "{started:?}");
assert_eq!(started[0].envs["COYOTE_AGENT_ID"], id);
let failed = hook_captures_named("t037_spawn_fail_failed");
assert_eq!(failed.len(), 1, "{failed:?}");
assert_eq!(failed[0].envs["COYOTE_EVENT"], "agent.failed");
assert_eq!(failed[0].envs["COYOTE_AGENT_ID"], id);
assert!(
!failed[0].envs["COYOTE_AGENT_ERROR"].is_empty(),
"a failed spawn must carry its error"
);
assert!(hook_captures_named("t037_spawn_fail_completed").is_empty());
}
#[test]
fn hook_base_envs_carries_optional_names_only_when_present() {
let envs = hook_base_envs(HookEvent::AgentCompleted, None, None);
let keys: Vec<&str> = envs.iter().map(|(key, _)| key.as_str()).collect();
assert!(keys.contains(&"COYOTE_EVENT"));
assert!(keys.contains(&"COYOTE_CONFIG_DIR"));
assert!(keys.contains(&"COYOTE_EVENT_TIMESTAMP"));
assert!(!keys.contains(&"COYOTE_SESSION_ID"));
assert!(!keys.contains(&"COYOTE_AGENT_NAME"));
assert!(
envs.iter()
.any(|(key, value)| key == "COYOTE_EVENT" && value == "agent.completed")
);
let envs = hook_base_envs(HookEvent::AgentFailed, Some("sess"), Some("bot"));
assert!(
envs.iter()
.any(|(key, value)| key == "COYOTE_SESSION_ID" && value == "sess")
);
assert!(
envs.iter()
.any(|(key, value)| key == "COYOTE_AGENT_NAME" && value == "bot")
);
}
fn run_agent_for_graph_error(
agent_name: &str,
graph_inputs: Option<HashMap<String, Value>>,
+274
View File
@@ -9,6 +9,7 @@ use crate::config::{
McpRuntime, RequestContext, effective_max_concurrent_jobs, jobs_enabled, paths,
};
use crate::graph;
use crate::hooks::{self, HookEvent, ResolvedHook};
use crate::mcp::{
MCP_DESCRIBE_META_FUNCTION_NAME_PREFIX, MCP_INVOKE_META_FUNCTION_NAME_PREFIX,
MCP_PROMPT_META_FUNCTION_NAME_PREFIX, MCP_READ_META_FUNCTION_NAME_PREFIX,
@@ -53,6 +54,63 @@ pub fn is_agent_task(supervisor: Option<&Arc<RwLock<Supervisor>>>, id: &str) ->
|| supervisor.is_some_and(|sup| sup.read().has_agent(id))
}
struct JobHookSnapshot {
completed: Vec<ResolvedHook>,
failed: Vec<ResolvedHook>,
session_name: Option<String>,
agent_name: Option<String>,
job_id: String,
tool: String,
}
impl JobHookSnapshot {
fn resolve(ctx: &RequestContext, job_id: &str, tool: &str) -> Self {
Self {
completed: ctx.resolved_hooks(HookEvent::JobCompleted),
failed: ctx.resolved_hooks(HookEvent::JobFailed),
session_name: ctx
.session
.as_ref()
.map(|session| session.name().to_string()),
agent_name: ctx.agent.as_ref().map(|agent| agent.name().to_string()),
job_id: job_id.to_string(),
tool: tool.to_string(),
}
}
/// Fires `job.completed` or `job.failed`. `fire_resolved` callers supply
/// their own base envs; the event name and timestamp are computed here so
/// they reflect the actual outcome rather than the moment the snapshot
/// was taken.
fn fire_terminal(self, success: bool, error: Option<String>) {
let (event, resolved) = if success {
(HookEvent::JobCompleted, self.completed)
} else {
(HookEvent::JobFailed, self.failed)
};
if resolved.is_empty() {
return;
}
let base_envs = hooks::base_envs_parts(
event,
self.session_name.as_deref(),
self.agent_name.as_deref(),
);
let mut extras = vec![
("COYOTE_JOB_ID", self.job_id),
("COYOTE_TOOL_NAME", self.tool),
];
if let Some(error) = error {
extras.push(("COYOTE_JOB_ERROR", error));
}
hooks::fire_resolved(event, resolved, base_envs, &extras, None);
}
}
pub struct RingBuf {
buf: Vec<u8>,
capacity: usize,
@@ -470,15 +528,27 @@ async fn handle_start(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let task_notifications = Arc::clone(&ctx.notification_queue);
let notify_id = job_id.clone();
let notify_tool = tool.clone();
let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool);
hooks::fire(
HookEvent::JobStarted,
ctx,
&[
("COYOTE_JOB_ID", job_id.clone()),
("COYOTE_TOOL_NAME", tool.clone()),
],
None,
);
tokio::spawn(async move {
let result = run_mcp_job(job_ctx, server, inner_tool, inner_args).await;
let success = result.is_ok();
let error = result.as_ref().err().map(|err| err.to_string());
task_state.lock().status = if success {
JobStatus::Completed
} else {
JobStatus::Failed
};
task_notifications.push(job_notification(&notify_id, &notify_tool, success));
hook_snapshot.fire_terminal(success, error);
result
})
} else {
@@ -488,9 +558,26 @@ async fn handle_start(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let task_notifications = Arc::clone(&ctx.notification_queue);
let notify_id = job_id.clone();
let notify_tool = tool.clone();
let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool);
hooks::fire(
HookEvent::JobStarted,
ctx,
&[
("COYOTE_JOB_ID", job_id.clone()),
("COYOTE_TOOL_NAME", tool.clone()),
],
None,
);
tokio::spawn(async move {
let result = run_process_job(snapshot, Arc::clone(&task_state), task_buf).await;
let success = matches!(&result, Ok(job_result) if job_result.exit_code == Some(0));
let error = (!success).then(|| match &result {
Err(err) => err.to_string(),
Ok(job_result) => match job_result.exit_code {
Some(code) => format!("exited with code {code}"),
None => "terminated without an exit code".to_string(),
},
});
let mut job_state = task_state.lock();
job_state.pgid = None;
job_state.status = if success {
@@ -502,6 +589,7 @@ async fn handle_start(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
drop(job_state);
task_notifications.push(job_notification(&notify_id, &notify_tool, success));
hook_snapshot.fire_terminal(success, error);
result
})
};
@@ -3119,4 +3207,190 @@ mod tests {
);
assert!(ctx.supervisor.is_none(), "no job may be spawned");
}
fn job_hooks_map(marker: &str) -> hooks::HooksMap {
["job.started", "job.completed", "job.failed"]
.into_iter()
.map(|event| {
(
event.to_string(),
vec![hooks::HookDef {
name: marker.to_string(),
command: "true".to_string(),
}],
)
})
.collect()
}
fn job_hooked_ctx(marker: &str) -> RequestContext {
let app = app_state_with_config(|config| {
config.hooks = job_hooks_map(marker);
});
RequestContext::new(app, WorkingMode::Cmd)
}
fn job_captures(marker: &str) -> Vec<hooks::test_sink::Capture> {
hooks::test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == marker)
.collect()
}
#[cfg(unix)]
#[test]
#[serial_test::serial]
fn job_hooks_fire_started_and_completed() {
run_async(async {
let marker = "job-hooks-ok-x7x";
let mut ctx = job_hooked_ctx(marker);
ctx.declared_function_names.insert("echo".into());
let _guard = hooks::test_sink::install();
let started = handle_start(&mut ctx, &json!({"tool": "echo", "arguments": {}}))
.await
.unwrap();
assert_eq!(started["status"], "ok");
let job_id = started["job_id"].as_str().unwrap().to_string();
// `job.started` is dispatched before `handle_start` returns.
let captures = job_captures(marker);
assert_eq!(captures.len(), 1, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::JobStarted);
assert_eq!(
captures[0].envs.get("COYOTE_JOB_ID").map(String::as_str),
Some(job_id.as_str())
);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("echo")
);
let collected = handle_collect(&ctx, &json!({"id": job_id})).await.unwrap();
assert_eq!(collected["status"], "completed");
let captures = job_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
let completed = &captures[1];
assert_eq!(completed.event, HookEvent::JobCompleted);
assert_eq!(
completed.envs.get("COYOTE_EVENT").map(String::as_str),
Some("job.completed")
);
assert_eq!(
completed.envs.get("COYOTE_JOB_ID").map(String::as_str),
Some(job_id.as_str())
);
assert_eq!(
completed.envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("echo")
);
assert!(completed.envs.contains_key("COYOTE_CONFIG_DIR"));
assert!(completed.envs.contains_key("COYOTE_EVENT_TIMESTAMP"));
assert!(!completed.envs.contains_key("COYOTE_SESSION_ID"));
assert!(!completed.envs.contains_key("COYOTE_JOB_ERROR"));
});
}
#[cfg(unix)]
#[test]
#[serial_test::serial]
fn job_hooks_fire_failed_with_job_error() {
run_async(async {
let marker = "job-hooks-fail-x7x";
let mut ctx = job_hooked_ctx(marker);
ctx.declared_function_names.insert("false".into());
let _guard = hooks::test_sink::install();
let started = handle_start(&mut ctx, &json!({"tool": "false", "arguments": {}}))
.await
.unwrap();
let job_id = started["job_id"].as_str().unwrap().to_string();
let collected = handle_collect(&ctx, &json!({"id": job_id})).await.unwrap();
assert_eq!(collected["status"], "failed");
let captures = job_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::JobStarted);
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::JobFailed);
assert_eq!(
failed.envs.get("COYOTE_EVENT").map(String::as_str),
Some("job.failed")
);
assert_eq!(
failed.envs.get("COYOTE_JOB_ID").map(String::as_str),
Some(job_id.as_str())
);
assert_eq!(
failed.envs.get("COYOTE_JOB_ERROR").map(String::as_str),
Some("exited with code 1")
);
});
}
#[test]
#[serial_test::serial]
fn job_hooks_fire_for_mcp_jobs() {
run_async(async {
let marker = "job-hooks-mcp-x7x";
let mut ctx = job_hooked_ctx(marker);
ctx.declared_function_names
.insert("mcp_invoke_fixture".into());
let fixture = FixtureServer::default();
let (mut runtime, _server) = fixture_runtime(fixture).await;
let mut filter = ToolFilter::default();
filter.push_layer(LayerSource::Global, &[]);
runtime.tool_filters.insert("fixture".to_string(), filter);
ctx.tool_scope.mcp_runtime = runtime;
let _guard = hooks::test_sink::install();
let started = handle_start(
&mut ctx,
&json!({"tool": "mcp_invoke_fixture", "arguments": {"tool": "dup"}}),
)
.await
.unwrap();
let job_id = started["job_id"].as_str().unwrap().to_string();
let collected = handle_collect(&ctx, &json!({"id": job_id})).await.unwrap();
assert_eq!(collected["status"], "failed");
let captures = job_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::JobStarted);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("mcp_invoke_fixture")
);
let failed = &captures[1];
assert_eq!(failed.event, HookEvent::JobFailed);
assert!(
failed
.envs
.get("COYOTE_JOB_ERROR")
.unwrap()
.contains("dup not found"),
"{failed:?}"
);
});
}
#[test]
#[serial_test::serial]
fn rejected_job_start_fires_no_hooks() {
let marker = "job-hooks-rejected-x7x";
let mut ctx = job_hooked_ctx(marker);
let _guard = hooks::test_sink::install();
let result = run_async(handle_start(
&mut ctx,
&json!({"tool": "undeclared_tool", "arguments": {}}),
))
.unwrap();
assert_eq!(result["status"], "error");
assert!(job_captures(marker).is_empty());
}
}
+332 -1
View File
@@ -17,6 +17,7 @@ use crate::{
use crate::config::ensure_parent_exists;
use crate::config::paths;
use crate::hooks::{self, HookEvent};
use crate::mcp::{
MCP_DESCRIBE_META_FUNCTION_NAME_PREFIX, MCP_INVOKE_META_FUNCTION_NAME_PREFIX,
MCP_META_FUNCTION_PREFIXES, MCP_PROMPT_META_FUNCTION_NAME_PREFIX,
@@ -1539,6 +1540,22 @@ impl ToolCall {
}
pub async fn eval(&self, ctx: &mut RequestContext) -> Result<Value> {
let result = self.eval_inner(ctx).await;
if let Err(err) = &result {
hooks::fire(
HookEvent::ToolFailed,
ctx,
&[
("COYOTE_TOOL_NAME", self.name.clone()),
("COYOTE_TOOL_ERROR", err.to_string()),
],
None,
);
}
result
}
async fn eval_inner(&self, ctx: &mut RequestContext) -> Result<Value> {
let agent = ctx.agent.clone();
let functions = ctx.tool_scope.functions.clone();
let current_depth = ctx.current_depth;
@@ -1566,7 +1583,8 @@ impl ToolCall {
);
};
cmd_args.push(json_data.to_string());
let args_json = json_data.to_string();
cmd_args.push(args_json.clone());
if (*IS_STDOUT_TERMINAL || *SHOW_TOOL_CALLS)
&& current_depth == 0
@@ -1575,6 +1593,14 @@ impl ToolCall {
println!("{}", format_call_log(&cmd_name, &cmd_args, &json_data));
}
hooks::fire(
HookEvent::ToolStarted,
ctx,
&[("COYOTE_TOOL_NAME", self.name.clone())],
Some(args_json.clone()),
);
let tool_started_at = Instant::now();
let output = match cmd_name.as_str() {
_ if cmd_name.starts_with(MCP_SEARCH_META_FUNCTION_NAME_PREFIX) => {
Self::search_mcp_tools(ctx, &cmd_name, &json_data)
@@ -1699,6 +1725,38 @@ impl ToolCall {
},
};
match output.get("tool_call_error") {
Some(error) => {
let error = error
.as_str()
.map(str::to_string)
.unwrap_or_else(|| error.to_string());
hooks::fire(
HookEvent::ToolFailed,
ctx,
&[
("COYOTE_TOOL_NAME", self.name.clone()),
("COYOTE_TOOL_ERROR", error),
],
Some(args_json),
);
}
None => {
hooks::fire(
HookEvent::ToolCompleted,
ctx,
&[
("COYOTE_TOOL_NAME", self.name.clone()),
(
"COYOTE_TOOL_DURATION_MS",
tool_started_at.elapsed().as_millis().to_string(),
),
],
Some(args_json),
);
}
}
Ok(output)
}
@@ -5271,4 +5329,277 @@ mod tests {
assert!(err.starts_with(expected), "{name}: {err}");
}
}
fn tool_hooks_map(marker: &str) -> hooks::HooksMap {
["tool.started", "tool.completed", "tool.failed"]
.into_iter()
.map(|event| {
(
event.to_string(),
vec![hooks::HookDef {
name: marker.to_string(),
command: "true".to_string(),
}],
)
})
.collect()
}
fn ctx_with_tool_hooks(marker: &str, update: impl FnOnce(&mut AppConfig)) -> RequestContext {
let mut app = AppState::test_default();
let mut config = AppConfig {
hooks: tool_hooks_map(marker),
..Default::default()
};
update(&mut config);
app.config = Arc::new(config);
RequestContext::new(Arc::new(app), WorkingMode::Cmd)
}
fn tool_captures(marker: &str) -> Vec<hooks::test_sink::Capture> {
hooks::test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == marker)
.collect()
}
#[test]
#[serial]
fn eval_fires_started_and_completed_for_builtin_tools() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-builtin-ok-x7x";
let mut ctx = ctx_with_tool_hooks(marker, |config| {
config.function_calling_support = true;
config.auto_continue = true;
});
ctx.tool_scope.functions.append_todo_functions();
let _guard = hooks::test_sink::install();
let output =
run_async(call_with_args("todo__init", json!({"goal": "g"})).eval(&mut ctx)).unwrap();
assert_eq!(output["status"], "ok");
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("todo__init")
);
assert_eq!(captures[0].payload.as_deref(), Some(r#"{"goal":"g"}"#));
assert_eq!(captures[0].cwd, paths::config_dir());
assert_eq!(captures[1].event, HookEvent::ToolCompleted);
assert!(captures[1].envs.contains_key("COYOTE_TOOL_DURATION_MS"));
assert_eq!(captures[1].payload.as_deref(), Some(r#"{"goal":"g"}"#));
}
#[test]
#[serial]
fn eval_fires_started_and_completed_for_mcp_meta_tools() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-mcp-ok-x7x";
run_async(async {
let mut result = CallToolResult::success(vec![ContentBlock::text("hi")]);
result.structured_content = Some(json!({"ok": true}));
let fixture = FixtureServer {
tool_result: Some(result),
..Default::default()
};
let (runtime, _server) = fixture_runtime(fixture).await;
let mut ctx = ctx_with_tool_hooks(marker, |_| {});
ctx.tool_scope.mcp_runtime = runtime;
ctx.tool_scope
.functions
.append_mcp_meta_functions(vec![tools_only("fixture")]);
let _guard = hooks::test_sink::install();
let output = call_with_args("mcp_invoke_fixture", json!({"tool": "dup"}))
.eval(&mut ctx)
.await
.unwrap();
assert_eq!(output["isError"], false);
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("mcp_invoke_fixture")
);
assert_eq!(captures[0].payload.as_deref(), Some(r#"{"tool":"dup"}"#));
assert_eq!(captures[1].event, HookEvent::ToolCompleted);
});
}
#[cfg(unix)]
#[test]
#[serial]
fn eval_fires_started_and_completed_for_external_command_tools() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-external-ok-x7x";
let mut ctx = ctx_with_tool_hooks(marker, |_| {});
ctx.tool_scope
.functions
.append_declaration(FunctionDeclaration {
name: "echo".to_string(),
description: String::new(),
parameters: JsonSchema::default(),
agent: false,
});
let _guard = hooks::test_sink::install();
let output = run_async(call_with_args("echo", json!({"x": 1})).eval(&mut ctx)).unwrap();
assert_eq!(output, Value::Null);
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("echo")
);
assert_eq!(captures[1].event, HookEvent::ToolCompleted);
}
#[cfg(unix)]
#[test]
#[serial]
fn eval_fires_failed_for_external_command_nonzero_exit() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-external-fail-x7x";
let mut ctx = ctx_with_tool_hooks(marker, |_| {});
ctx.tool_scope
.functions
.append_declaration(FunctionDeclaration {
name: "false".to_string(),
description: String::new(),
parameters: JsonSchema::default(),
agent: false,
});
let _guard = hooks::test_sink::install();
let output = run_async(call_with_args("false", json!({})).eval(&mut ctx)).unwrap();
assert!(output["tool_call_error"].is_string());
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(captures[1].event, HookEvent::ToolFailed);
assert!(
captures[1]
.envs
.get("COYOTE_TOOL_ERROR")
.unwrap()
.contains("exited with code 1")
);
assert!(!captures[1].envs.contains_key("COYOTE_TOOL_DURATION_MS"));
}
#[test]
#[serial]
fn eval_fires_failed_on_error_shaped_output_with_identical_result() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-error-shape-x7x";
// Default config leaves todo tools disabled, so the handler bails
// and eval shapes the failure as a `tool_call_error` output.
let mut hooked = ctx_with_tool_hooks(marker, |_| {});
hooked.tool_scope.functions.append_todo_functions();
let mut plain = RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd);
plain.tool_scope.functions.append_todo_functions();
let call = call_with_args("todo__done", json!({}));
let _guard = hooks::test_sink::install();
let hooked_output = run_async(call.eval(&mut hooked)).unwrap();
let plain_output = run_async(call.eval(&mut plain)).unwrap();
assert_eq!(
hooked_output.to_string(),
plain_output.to_string(),
"hooks must not change the tool result"
);
let error = hooked_output["tool_call_error"].as_str().unwrap();
assert!(error.starts_with("Todo tool failed:"), "{error}");
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(captures[1].event, HookEvent::ToolFailed);
assert_eq!(
captures[1]
.envs
.get("COYOTE_TOOL_ERROR")
.map(String::as_str),
Some(error)
);
}
#[test]
#[serial]
fn eval_fires_hooks_with_exact_user_prefixed_tool_name() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-user-x7x";
let mut ctx = ctx_with_tool_hooks(marker, |_| {});
ctx.tool_scope.functions.append_user_interaction_functions();
let _guard = hooks::test_sink::install();
let output = run_async(call_with_args("user__confirm", json!({})).eval(&mut ctx)).unwrap();
// Without a question the user handler errors before prompting, so the
// error-shaped output must fire started + failed with the exact name.
let error = output["tool_call_error"].as_str().unwrap();
assert!(error.starts_with("User interaction failed"), "{error}");
let captures = tool_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolStarted);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("user__confirm")
);
assert_eq!(captures[1].event, HookEvent::ToolFailed);
assert_eq!(
captures[1].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("user__confirm")
);
assert_eq!(
captures[1]
.envs
.get("COYOTE_TOOL_ERROR")
.map(String::as_str),
Some(error)
);
}
#[test]
#[serial]
fn eval_err_fires_failed_without_started_and_identical_error() {
use crate::hooks::HookEvent;
let marker = "tool-hooks-eval-err-x7x";
let mut hooked = ctx_with_tool_hooks(marker, |_| {});
let mut plain = RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd);
let call = call_with_args("nonexistent_tool", json!({}));
let _guard = hooks::test_sink::install();
let hooked_err = run_async(call.eval(&mut hooked)).unwrap_err();
let plain_err = run_async(call.eval(&mut plain)).unwrap_err();
assert_eq!(
hooked_err.to_string(),
plain_err.to_string(),
"hooks must not change the error"
);
let captures = tool_captures(marker);
assert_eq!(captures.len(), 1, "{captures:?}");
assert_eq!(captures[0].event, HookEvent::ToolFailed);
assert_eq!(
captures[0].envs.get("COYOTE_TOOL_NAME").map(String::as_str),
Some("nonexistent_tool")
);
assert!(
captures[0]
.envs
.get("COYOTE_TOOL_ERROR")
.unwrap()
.contains("Unexpected call")
);
assert!(captures[0].payload.is_none());
}
}
+18
View File
@@ -6,6 +6,7 @@ use super::types::AgentNode;
use super::wall_clock;
use crate::config::RequestContext;
use crate::function::agents::run_agent_for_graph;
use crate::hooks::{self, HookEvent};
use crate::supervisor::mailbox::{Inbox, PeerRegistry};
use crate::utils::create_abort_signal;
use anyhow::{Context, Error, Result, anyhow};
@@ -35,6 +36,11 @@ impl AgentNodeExecutor {
parent_ctx: &mut RequestContext,
retire_peer_on_return: bool,
) -> Result<AgentExecutionOutcome> {
let agent_hook_id = parent_ctx
.peer_assignment
.as_ref()
.map(|(id, _)| id.clone())
.unwrap_or_else(|| node_id.to_string());
let result = run(
node_id,
node,
@@ -43,6 +49,18 @@ impl AgentNodeExecutor {
retire_peer_on_return,
)
.await;
if let Err(e) = &result {
hooks::fire(
HookEvent::AgentFailed,
parent_ctx,
&[
("COYOTE_AGENT_ID", agent_hook_id),
("COYOTE_AGENT_NAME", node.agent.clone()),
("COYOTE_AGENT_ERROR", format!("{e:#}")),
],
None,
);
}
outcome_from(node_id, node, state_manager, result)
}
}
+467 -2
View File
@@ -1,6 +1,6 @@
use super::agent::{AgentExecutionOutcome, AgentNodeExecutor};
use super::llm::{LlmExecutionOutcome, LlmNodeExecutor};
use super::logging::{GraphLogger, narrate_node_complete, narrate_node_failed};
use super::logging::{GraphLogger, narrate_node_complete, narrate_node_failed, node_type_label};
use super::map::MapNodeExecutor;
use super::rag::RagNodeExecutor;
use super::script::ScriptExecutor;
@@ -11,6 +11,8 @@ use super::user_interaction::{ApprovalNodeExecutor, InputNodeExecutor};
use super::validator::{AgentValidationContext, GraphValidator};
use super::wall_clock;
use crate::config::{AgentVariable, AgentVariables, RenderMode, RequestContext};
use crate::function::agents::hook_base_envs;
use crate::hooks::{self, HookEvent, ResolvedHook};
use crate::supervisor::mailbox::{Inbox, PeerAssignment, PeerRegistry, graph_agent_id};
use crate::utils::{AbortSignal, wait_abort_signal, wait_user_interrupt};
use anyhow::{Context, Result, anyhow, bail};
@@ -234,6 +236,16 @@ impl GraphExecutor {
})?
.clone();
logger.node_start(&node, in_super_step);
let node_hooks = NodeHookEmitter::resolve(ctx, node_id, node_type_label(&node));
hooks::fire(
HookEvent::GraphNodeStarted,
ctx,
&[
("COYOTE_NODE_ID", node_id.clone()),
("COYOTE_NODE_TYPE", node_hooks.node_type.to_string()),
],
None,
);
let branch_state = state.fork_for_branch_state();
let mut branch_ctx = ctx.fork_for_branch();
let mut peer_id: Option<String> = None;
@@ -281,6 +293,7 @@ impl GraphExecutor {
"aborted",
in_super_step,
);
node_hooks.failed(Duration::default(), "aborted");
return (
current.clone(),
branch_state,
@@ -315,6 +328,7 @@ impl GraphExecutor {
route.as_deref(),
in_super_step,
);
node_hooks.completed(elapsed, route.as_deref());
}
Ok(StepResult::End(_)) => {
narrate_node_complete(
@@ -324,6 +338,7 @@ impl GraphExecutor {
Some("END"),
in_super_step,
);
node_hooks.completed(elapsed, Some("END"));
}
Err(e) => {
narrate_node_failed(
@@ -333,6 +348,7 @@ impl GraphExecutor {
&e.to_string(),
in_super_step,
);
node_hooks.failed(elapsed, &e.to_string());
}
}
(current, state, result, elapsed)
@@ -498,6 +514,67 @@ fn provision_frontier_peers(
(Some(registry), assignments)
}
struct NodeHookEmitter {
completed: Vec<ResolvedHook>,
failed: Vec<ResolvedHook>,
session_name: Option<String>,
agent_name: Option<String>,
node_id: String,
node_type: &'static str,
}
impl NodeHookEmitter {
fn resolve(ctx: &RequestContext, node_id: &str, node_type: &'static str) -> Self {
Self {
completed: ctx.resolved_hooks(HookEvent::GraphNodeCompleted),
failed: ctx.resolved_hooks(HookEvent::GraphNodeFailed),
session_name: ctx
.session
.as_ref()
.map(|session| session.name().to_string()),
agent_name: ctx.agent.as_ref().map(|agent| agent.name().to_string()),
node_id: node_id.to_string(),
node_type,
}
}
fn extras(&self, elapsed: Duration) -> Vec<(&'static str, String)> {
vec![
("COYOTE_NODE_ID", self.node_id.clone()),
("COYOTE_NODE_TYPE", self.node_type.to_string()),
("COYOTE_NODE_DURATION_MS", elapsed.as_millis().to_string()),
]
}
fn dispatch(&self, event: HookEvent, resolved: &[ResolvedHook], extras: &[(&str, String)]) {
hooks::fire_resolved(
event,
resolved.to_vec(),
hook_base_envs(
event,
self.session_name.as_deref(),
self.agent_name.as_deref(),
),
extras,
None,
);
}
fn completed(&self, elapsed: Duration, route: Option<&str>) {
let mut extras = self.extras(elapsed);
if let Some(route) = route {
extras.push(("COYOTE_NODE_ROUTE", route.to_string()));
}
self.dispatch(HookEvent::GraphNodeCompleted, &self.completed, &extras);
}
fn failed(&self, elapsed: Duration, error: &str) {
let mut extras = self.extras(elapsed);
extras.push(("COYOTE_NODE_ERROR", error.to_string()));
self.dispatch(HookEvent::GraphNodeFailed, &self.failed, &extras);
}
}
pub(super) struct TaskCancelGuard(Vec<AbortHandle>);
impl TaskCancelGuard {
@@ -928,6 +1005,8 @@ mod tests {
max_concurrent_agents: None,
max_agent_depth: None,
global_tools: Vec::new(),
hooks: Default::default(),
global_hooks: Vec::new(),
mcp_servers: Vec::new(),
mcp_tools: None,
skills_enabled: None,
@@ -1048,9 +1127,10 @@ mod tests {
mod integration_tests {
use super::*;
use crate::config::paths;
use crate::config::{AppState, WorkingMode};
use crate::config::{Agent, AgentConfig, AppConfig, AppState, WorkingMode};
#[cfg(unix)]
use crate::function::jobs::RingBuf;
use crate::hooks::HookDef;
#[cfg(unix)]
use crate::supervisor::{JobHandle, JobResult, JobState, JobStatus, Supervisor, notification};
use crate::utils::{create_abort_signal, get_env_name, temp_file};
@@ -1066,6 +1146,33 @@ mod integration_tests {
which::which(name).is_ok()
}
fn hooks_config(entries: &[(&str, &str)]) -> hooks::HooksMap {
let mut map = hooks::HooksMap::new();
for (event, name) in entries {
map.entry((*event).to_string()).or_default().push(HookDef {
name: (*name).to_string(),
command: "true".to_string(),
});
}
map
}
fn ctx_with_global_hooks(hooks: hooks::HooksMap) -> RequestContext {
let mut app = AppState::test_default();
app.config = Arc::new(AppConfig {
hooks,
..Default::default()
});
RequestContext::new(Arc::new(app), WorkingMode::Cmd)
}
fn hook_captures_named(name: &str) -> Vec<hooks::test_sink::Capture> {
hooks::test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == name)
.collect()
}
struct TestWorkspace {
dir: PathBuf,
}
@@ -1208,6 +1315,364 @@ nodes:
assert_eq!(result, "held|held");
}
/// An agent node that fails but declares a fallback must surface the
/// failure to observers as `agent.failed`, fired at the point of
/// failure, even though the fallback then converts it into a normal
/// node completion that routes to the fallback target — so the same
/// node also yields `graph.node.completed`, never `graph.node.failed`.
#[tokio::test]
async fn agent_node_failure_with_fallback_fires_agent_failed_and_node_completed() {
let _sink = hooks::test_sink::install();
let mut ctx = ctx_with_global_hooks(hooks_config(&[
("agent.failed", "t037_fb_agent_failed"),
("graph.node.completed", "t037_fb_node_completed"),
("graph.node.failed", "t037_fb_node_failed"),
]));
// Forces the agent node to fail before it ever loads the (absent)
// agent from disk, keeping the test hermetic.
ctx.current_depth = crate::config::default_max_agent_depth();
let yaml = r#"
name: t
settings:
validate_before_run: false
start: worker
nodes:
worker:
type: agent
agent: missing_agent
prompt: p
fallback: recover
next: done
recover:
type: end
output: recovered
done:
type: end
output: done
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
let abort = create_abort_signal();
let result = GraphExecutor::new(graph, ".")
.execute(&mut ctx, abort)
.await
.unwrap_or_else(|e| panic!("executor failed: {e:#}"));
assert_eq!(result, "recovered");
let agent_failed = hook_captures_named("t037_fb_agent_failed");
assert_eq!(agent_failed.len(), 1, "{agent_failed:?}");
assert_eq!(agent_failed[0].envs["COYOTE_EVENT"], "agent.failed");
assert_eq!(agent_failed[0].envs["COYOTE_AGENT_ID"], "worker");
assert_eq!(agent_failed[0].envs["COYOTE_AGENT_NAME"], "missing_agent");
assert!(
agent_failed[0].envs["COYOTE_AGENT_ERROR"].contains("Max agent depth exceeded"),
"{:?}",
agent_failed[0].envs
);
let node_completed = hook_captures_named("t037_fb_node_completed");
let worker_completed: Vec<_> = node_completed
.iter()
.filter(|capture| capture.envs["COYOTE_NODE_ID"] == "worker")
.collect();
assert_eq!(worker_completed.len(), 1, "{node_completed:?}");
assert_eq!(worker_completed[0].envs["COYOTE_NODE_TYPE"], "agent");
assert_eq!(worker_completed[0].envs["COYOTE_NODE_ROUTE"], "recover");
let node_failed = hook_captures_named("t037_fb_node_failed");
assert!(
node_failed
.iter()
.all(|capture| capture.envs["COYOTE_NODE_ID"] != "worker"),
"a node that continues via fallback must not fire graph.node.failed: {node_failed:?}"
);
}
/// Two branches running in the same super-step each carry their own
/// node metadata, captured task-locally before the branch was spawned;
/// the distinct routes prove one branch's metadata never leaks into a
/// sibling's events.
#[tokio::test]
async fn parallel_super_step_branches_fire_per_node_metadata() {
if !cmd_available("bash") {
eprintln!("skipping: bash not available");
return;
}
let _sink = hooks::test_sink::install();
let ws = TestWorkspace::new();
for script in [
"dispatcher.sh",
"worker_a.sh",
"worker_b.sh",
"sink_a.sh",
"sink_b.sh",
] {
ws.write_script(script, "#!/bin/bash\necho '{}'\n");
}
let mut ctx = ctx_with_global_hooks(hooks_config(&[
("graph.node.started", "t037_par_started"),
("graph.node.completed", "t037_par_completed"),
]));
let yaml = r#"
name: t
start: dispatcher
nodes:
dispatcher:
type: script
script: dispatcher.sh
state_updates: {}
next: [worker_a, worker_b]
worker_a:
type: script
script: worker_a.sh
state_updates: {}
next: sink_a
worker_b:
type: script
script: worker_b.sh
state_updates: {}
next: sink_b
sink_a:
type: script
script: sink_a.sh
state_updates: {}
next: join
sink_b:
type: script
script: sink_b.sh
state_updates: {}
next: join
join:
type: end
output: done
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
let abort = create_abort_signal();
let result = GraphExecutor::new(graph, &ws.dir)
.execute(&mut ctx, abort)
.await
.unwrap_or_else(|e| panic!("executor failed: {e:#}"));
assert_eq!(result, "done");
let started = hook_captures_named("t037_par_started");
for node_id in [
"dispatcher",
"worker_a",
"worker_b",
"sink_a",
"sink_b",
"join",
] {
assert_eq!(
started
.iter()
.filter(|capture| capture.envs["COYOTE_NODE_ID"] == node_id)
.count(),
1,
"exactly one graph.node.started for '{node_id}': {started:?}"
);
}
let completed = hook_captures_named("t037_par_completed");
let completed_route = |node_id: &str| -> String {
let matches: Vec<_> = completed
.iter()
.filter(|capture| capture.envs["COYOTE_NODE_ID"] == node_id)
.collect();
assert_eq!(
matches.len(),
1,
"exactly one graph.node.completed for '{node_id}': {completed:?}"
);
let expected_type = if node_id == "join" { "end" } else { "script" };
assert_eq!(matches[0].envs["COYOTE_NODE_TYPE"], expected_type);
let duration = &matches[0].envs["COYOTE_NODE_DURATION_MS"];
let _: u128 = duration
.parse()
.unwrap_or_else(|_| panic!("non-numeric duration for '{node_id}': {duration}"));
matches[0].envs["COYOTE_NODE_ROUTE"].clone()
};
assert_eq!(completed_route("dispatcher"), "worker_a, worker_b");
assert_eq!(completed_route("worker_a"), "sink_a");
assert_eq!(completed_route("worker_b"), "sink_b");
assert_eq!(completed_route("sink_a"), "join");
assert_eq!(completed_route("sink_b"), "join");
assert_eq!(completed_route("join"), "END");
}
/// A graph agent's hooks come from graph.yaml's top level: they resolve
/// through the agent scope end-to-end, while a global hook on the same
/// event stays gated off because the agent whitelisted nothing.
#[tokio::test]
async fn graph_agent_hooks_resolve_from_graph_yaml_top_level() {
let _sink = hooks::test_sink::install();
let mut ctx =
ctx_with_global_hooks(hooks_config(&[("graph.node.completed", "t037_gy_global")]));
let yaml = r#"
name: hooked_graph
settings:
validate_before_run: false
start: done
hooks:
graph.node.started:
- name: t037_gy_started
command: "true"
graph.node.completed:
- name: t037_gy_completed
command: "true"
nodes:
done:
type: end
output: ok
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
ctx.agent = Some(Agent::test_new(AgentConfig::from_graph(
"hooked_graph",
&graph,
)));
let abort = create_abort_signal();
let result = GraphExecutor::new(graph, ".")
.execute(&mut ctx, abort)
.await
.unwrap_or_else(|e| panic!("executor failed: {e:#}"));
assert_eq!(result, "ok");
let started = hook_captures_named("t037_gy_started");
assert_eq!(started.len(), 1, "{started:?}");
assert_eq!(started[0].envs["COYOTE_EVENT"], "graph.node.started");
assert_eq!(started[0].envs["COYOTE_NODE_ID"], "done");
assert_eq!(started[0].envs["COYOTE_NODE_TYPE"], "end");
assert_eq!(started[0].envs["COYOTE_AGENT_NAME"], "hooked_graph");
let completed = hook_captures_named("t037_gy_completed");
assert_eq!(completed.len(), 1, "{completed:?}");
assert_eq!(completed[0].envs["COYOTE_NODE_ID"], "done");
assert_eq!(completed[0].envs["COYOTE_NODE_ROUTE"], "END");
assert_eq!(completed[0].envs["COYOTE_AGENT_NAME"], "hooked_graph");
assert!(
hook_captures_named("t037_gy_global").is_empty(),
"an agent context must not run global hooks it did not whitelist"
);
}
/// A node that genuinely fails — no fallback to absorb the error — fires
/// `graph.node.failed` with the full payload observers rely on: the node's
/// id and type, a numeric duration, and the error text, even though the
/// failure then propagates out of the executor.
#[tokio::test]
async fn genuine_node_failure_fires_graph_node_failed_with_payload() {
let _sink = hooks::test_sink::install();
let mut ctx = ctx_with_global_hooks(hooks_config(&[(
"graph.node.failed",
"t037_gf_node_failed",
)]));
// Forces the agent node to fail before it ever loads the (absent)
// agent from disk, keeping the test hermetic.
ctx.current_depth = crate::config::default_max_agent_depth();
let yaml = r#"
name: t
settings:
validate_before_run: false
start: worker
nodes:
worker:
type: agent
agent: missing_agent
prompt: p
next: done
done:
type: end
output: done
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
let abort = create_abort_signal();
let result = GraphExecutor::new(graph, ".")
.execute(&mut ctx, abort)
.await;
let error = result.expect_err("a node failure without a fallback must propagate");
let failed = hook_captures_named("t037_gf_node_failed");
assert_eq!(failed.len(), 1, "{failed:?}");
let envs = &failed[0].envs;
assert_eq!(envs["COYOTE_EVENT"], "graph.node.failed");
assert_eq!(envs["COYOTE_NODE_ID"], "worker");
assert_eq!(envs["COYOTE_NODE_TYPE"], "agent");
let duration = &envs["COYOTE_NODE_DURATION_MS"];
let _: u128 = duration
.parse()
.unwrap_or_else(|_| panic!("non-numeric duration: {duration}"));
assert!(!envs["COYOTE_NODE_ERROR"].is_empty(), "{envs:?}");
assert!(
format!("{error:#}").contains("Max agent depth exceeded"),
"unexpected executor error: {error:#}"
);
}
/// A malformed hook definition reaching the executor seam through
/// graph.yaml's top level — an entry whose command is blank — degrades
/// safely: the node still runs, the malformed hook is skipped with a
/// debug log, and a well-formed sibling on the same event still fires.
#[tokio::test]
async fn malformed_graph_yaml_hook_is_skipped_without_breaking_the_node() {
crate::testing::install_warn_collector();
let _sink = hooks::test_sink::install();
let mut ctx = make_ctx();
let yaml = r#"
name: hooked_graph
settings:
validate_before_run: false
start: done
hooks:
graph.node.completed:
- name: t037_mal_empty
command: " "
- name: t037_mal_valid
command: "true"
nodes:
done:
type: end
output: ok
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
ctx.agent = Some(Agent::test_new(AgentConfig::from_graph(
"hooked_graph",
&graph,
)));
let abort = create_abort_signal();
let result = GraphExecutor::new(graph, ".")
.execute(&mut ctx, abort)
.await
.unwrap_or_else(|e| panic!("executor failed: {e:#}"));
assert_eq!(result, "ok");
assert!(
hook_captures_named("t037_mal_empty").is_empty(),
"a blank-command hook must be skipped, not dispatched"
);
let valid = hook_captures_named("t037_mal_valid");
assert_eq!(valid.len(), 1, "{valid:?}");
assert_eq!(valid[0].envs["COYOTE_NODE_ID"], "done");
let debugs: Vec<String> = crate::testing::debug_messages()
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.clone();
assert!(
debugs
.iter()
.any(|message| message.contains("t037_mal_empty")
&& message.contains("empty command")),
"expected a debug log for the skipped hook: {debugs:?}"
);
}
/// Declared variables are seeded into state at run start, so the agent
/// node's strict prompt interpolation of `{{foo}}` succeeds instead of
/// bailing with "not found in state". `ctx.agent` is None here, so each
+2
View File
@@ -497,6 +497,8 @@ mod tests {
max_concurrent_agents: None,
max_agent_depth: None,
global_tools: Vec::new(),
hooks: Default::default(),
global_hooks: Vec::new(),
mcp_servers: Vec::new(),
mcp_tools: None,
skills_enabled: None,
+53
View File
@@ -1,4 +1,5 @@
use crate::config::AgentVariable;
use crate::hooks::HooksMap;
use anyhow::Result;
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
@@ -43,6 +44,12 @@ pub struct Graph {
#[serde(default)]
pub global_tools: Vec<String>,
#[serde(default)]
pub hooks: HooksMap,
#[serde(default)]
pub global_hooks: Vec<String>,
#[serde(default)]
pub mcp_servers: Vec<String>,
@@ -611,6 +618,52 @@ nodes:
}
}
#[test]
fn graph_hooks_and_global_hooks_parse_and_round_trip() {
let yaml = r#"
name: g
start: e
hooks:
tool.started:
- name: notify
command: ./hooks/notify.sh
global_hooks:
- tool.started.notify
nodes:
e:
id: e
type: end
output: done
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
assert_eq!(graph.hooks["tool.started"][0].name, "notify");
assert_eq!(graph.hooks["tool.started"][0].command, "./hooks/notify.sh");
assert_eq!(graph.global_hooks, vec!["tool.started.notify"]);
let reparsed: Graph =
serde_yaml::from_str(&serde_yaml::to_string(&graph).unwrap()).unwrap();
assert_eq!(reparsed.hooks, graph.hooks);
assert_eq!(reparsed.global_hooks, graph.global_hooks);
}
#[test]
fn graph_without_hooks_keys_parses_empty() {
let yaml = r#"
name: g
start: e
nodes:
e:
id: e
type: end
output: done
"#;
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
assert!(graph.hooks.is_empty());
assert!(graph.global_hooks.is_empty());
}
#[test]
fn agent_node_teammates_parses_and_defaults_false() {
let yaml = r#"
+2
View File
@@ -1459,6 +1459,8 @@ mod tests {
max_concurrent_agents: None,
max_agent_depth: None,
global_tools: Vec::new(),
hooks: Default::default(),
global_hooks: Vec::new(),
mcp_servers: Vec::new(),
mcp_tools: None,
skills_enabled: None,
+1760
View File
File diff suppressed because it is too large. Load diff
+68 -21
View File
@@ -4,6 +4,7 @@ mod client;
mod config;
mod function;
mod graph;
mod hooks;
mod rag;
mod render;
mod repl;
@@ -22,7 +23,8 @@ extern crate log;
use crate::cli::Cli;
use crate::client::{
ModelType, call_chat_completions, call_chat_completions_streaming, catalog, list_models, oauth,
Client, ModelType, call_chat_completions, call_chat_completions_streaming, catalog,
list_models, oauth,
};
use crate::config::instructions::WORKSPACE_INSTRUCTIONS_FILE_NAME;
use crate::config::{
@@ -35,7 +37,7 @@ use crate::config::{memory, paths};
use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail};
use crate::mcp::McpServersConfig;
use crate::render::{prompt_theme, render_error};
use crate::repl::Repl;
use crate::repl::{EXIT_HOOK_DRAIN_TIMEOUT, Repl, TurnBracket};
use crate::utils::*;
use crate::vault::{Vault, interpolate_secrets};
use anyhow::{Context, Result, anyhow, bail};
@@ -386,6 +388,12 @@ async fn run(
let app = Arc::clone(&ctx.app.config);
ctx.use_agent(app.as_ref(), agent, session, abort_signal.clone())
.await?;
// The top-level agent bracket does NOT open here: the inspection
// flags below (--list-sessions, --info, --macro, --rebuild-rag,
// --init-*) return early without running the agent, and an open
// bracket on those paths would leak `agent.started` with no
// terminal event. It opens at the dispatch sites further down,
// where an agent run is actually committed.
} else {
let app: Arc<AppConfig> = Arc::clone(&ctx.app.config);
if let Some(prompt) = &cli.temp_role {
@@ -522,9 +530,17 @@ async fn run(
return Ok(());
}
if cli.execute && !is_repl {
// Dispatch committed: open the top-level agent bracket only now,
// past every inspection-flag early return above.
ctx.top_level_agent_started();
let result = async {
let input = create_input(&ctx, text, &cli.file, abort_signal.clone()).await?;
shell_execute(&mut ctx, &SHELL, input, abort_signal.clone()).await?;
return Ok(());
shell_execute(&mut ctx, &SHELL, input, abort_signal.clone()).await
}
.await;
ctx.top_level_agent_finished(result.as_ref().err());
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
return result;
}
{
@@ -542,11 +558,23 @@ async fn run(
return acp::run_acp_server(ctx, abort_signal).await;
}
// Dispatch committed (headless directive or REPL): open the top-level
// agent bracket only now, past every inspection-flag early return
// above. The ACP server stays unbracketed deliberately. The headless
// arm closes the bracket below; the REPL closes it at teardown (or on
// an interactive agent switch).
ctx.top_level_agent_started();
match is_repl {
false => {
let result = async {
let mut input = create_input(&ctx, text, &cli.file, abort_signal.clone()).await?;
input.use_embeddings(abort_signal.clone()).await?;
start_directive(&mut ctx, input, cli.code, abort_signal).await
start_directive(&mut ctx, input, cli.code, abort_signal.clone()).await
}
.await;
ctx.top_level_agent_finished(result.as_ref().err());
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
result
}
true => {
if !*IS_STDOUT_TERMINAL {
@@ -563,6 +591,17 @@ async fn start_directive(
input: Input,
code_mode: bool,
abort_signal: AbortSignal,
) -> Result<()> {
let bracket = TurnBracket::enter(ctx);
let result = start_directive_inner(ctx, input, code_mode, abort_signal.clone()).await;
bracket.finish(ctx, &abort_signal, result)
}
async fn start_directive_inner(
ctx: &mut RequestContext,
input: Input,
code_mode: bool,
abort_signal: AbortSignal,
) -> Result<()> {
let app: Arc<AppConfig> = Arc::clone(&ctx.app.config);
ctx.session_abort = Some(abort_signal.clone());
@@ -638,22 +677,10 @@ async fn shell_execute(
abort_signal: AbortSignal,
) -> Result<()> {
let app: Arc<AppConfig> = Arc::clone(&ctx.app.config);
let client = input.create_client()?;
ctx.before_chat_completion(&input)?;
let (eval_str, _) = call_chat_completions(
&input,
false,
true,
client.as_ref(),
ctx,
abort_signal.clone(),
)
.await?;
ctx.after_chat_completion(app.as_ref(), &input, &eval_str, &[])?;
if eval_str.is_empty() {
bail!("No command generated");
}
// Each revision round is its own turn: the recursive 'r' call re-enters.
let bracket = TurnBracket::enter(ctx);
let result = shell_execute_turn(ctx, &input, abort_signal.clone()).await;
let (client, eval_str) = bracket.finish(ctx, &abort_signal, result)?;
if app.dry_run {
app.print_markdown(&eval_str)?;
return Ok(());
@@ -679,6 +706,8 @@ async fn shell_execute(
if code == 0 && app.save_shell_history {
let _ = append_to_shell_history(&shell.name, &eval_str, code);
}
ctx.top_level_agent_finished(None);
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
process::exit(code);
}
'r' => {
@@ -726,6 +755,24 @@ async fn shell_execute(
Ok(())
}
async fn shell_execute_turn(
ctx: &mut RequestContext,
input: &Input,
abort_signal: AbortSignal,
) -> Result<(Box<dyn Client>, String)> {
let app: Arc<AppConfig> = Arc::clone(&ctx.app.config);
let client = input.create_client()?;
ctx.before_chat_completion(input)?;
let (eval_str, _) =
call_chat_completions(input, false, true, client.as_ref(), ctx, abort_signal).await?;
ctx.after_chat_completion(app.as_ref(), input, &eval_str, &[])?;
if eval_str.is_empty() {
bail!("No command generated");
}
Ok((client, eval_str))
}
async fn create_input(
ctx: &RequestContext,
text: Option<String>,
+389 -5
View File
@@ -17,6 +17,7 @@ use crate::config::{
};
use crate::config::{AssetCategory, paths};
use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail};
use crate::hooks::{self, HookEvent};
use crate::render::render_error;
use crate::utils::{
AbortSignal, SHELL, abortable_run_with_spinner, create_abort_signal, dimmed_text,
@@ -41,11 +42,58 @@ use reedline::{
use reedline::{MenuBuilder, Signal};
use std::collections::HashMap;
use std::sync::LazyLock;
use std::time::Duration;
use std::{env, process, sync::Arc};
use tokio::task;
const MENU_NAME: &str = "completion_menu";
/// Bounded wait applied at process-exit seams (REPL teardown, headless and
/// shell-execute exits) so already-fired exit hooks — `session.ended`, the
/// final `turn.*` / `agent.*` events — finish spawning before the process
/// tears down. Never used mid-session.
pub(crate) const EXIT_HOOK_DRAIN_TIMEOUT: Duration = Duration::from_secs(2);
/// Brackets one user-visible turn at an outer boundary (`ask`,
/// `start_directive`, `shell_execute`): `enter` fires `turn.started`, and
/// `finish` fires exactly one of `turn.completed` / `turn.interrupted` /
/// `turn.failed`. The interrupted-vs-failed discrimination lives here and
/// nowhere else: a ctrl-c abort wins over the result, then an error means
/// failed (carrying `COYOTE_ERROR`), otherwise completed. Tool rounds,
/// auto-continue, and compression retries run inside one bracket and never
/// re-fire turn events.
pub(crate) struct TurnBracket(());
impl TurnBracket {
#[must_use]
pub(crate) fn enter(ctx: &RequestContext) -> Self {
hooks::fire(HookEvent::TurnStarted, ctx, &[], None);
Self(())
}
pub(crate) fn finish<T>(
self,
ctx: &RequestContext,
abort_signal: &AbortSignal,
result: Result<T>,
) -> Result<T> {
if abort_signal.aborted_ctrlc() {
hooks::fire(HookEvent::TurnInterrupted, ctx, &[], None);
} else {
match &result {
Ok(_) => hooks::fire(HookEvent::TurnCompleted, ctx, &[], None),
Err(err) => hooks::fire(
HookEvent::TurnFailed,
ctx,
&[("COYOTE_ERROR", format!("{err:#}"))],
None,
),
}
}
result
}
}
pub const DEFAULT_CONTINUATION_PROMPT: &str = indoc! {"
[SYSTEM REMINDER - TODO CONTINUATION]
You have incomplete tasks. Rules:
@@ -482,8 +530,12 @@ Type ".help" for additional help.
supervisor.read().cancel_recursive();
}
self.ctx.write().exit_session()?;
Ok(())
let exit_result = self.ctx.write().exit_session();
self.ctx
.write()
.top_level_agent_finished(exit_result.as_ref().err());
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
exit_result
}
fn create_editor(ctx: Arc<RwLock<RequestContext>>, app: &AppConfig) -> Result<Reedline> {
@@ -983,6 +1035,7 @@ pub async fn run_repl_command(
let app = Arc::clone(&ctx.app.config);
ctx.use_agent(app.as_ref(), agent_name, session_name, abort_signal.clone())
.await?;
ctx.top_level_agent_started();
if let Some(session) = &ctx.session {
let (compressed, active) = replay::snapshot(session);
replay::render(app.as_ref(), &compressed, &active)?;
@@ -1090,6 +1143,7 @@ pub async fn run_repl_command(
abort_signal.clone(),
)
.await?;
hooks::fire(HookEvent::SessionCompressed, ctx, &[], None);
println!("✓ Successfully compressed the session.");
}
_ => {
@@ -1433,6 +1487,17 @@ async fn ask(
abort_signal: AbortSignal,
input: Input,
with_embeddings: bool,
) -> Result<()> {
let bracket = TurnBracket::enter(ctx);
let result = ask_inner(ctx, abort_signal.clone(), input, with_embeddings).await;
bracket.finish(ctx, &abort_signal, result)
}
async fn ask_inner(
ctx: &mut RequestContext,
abort_signal: AbortSignal,
input: Input,
with_embeddings: bool,
) -> Result<()> {
let mut input = input;
let mut with_embeddings = with_embeddings;
@@ -1579,9 +1644,7 @@ async fn ask(
};
eprintln!("\n📢 {}", color.italic().paint("Compressing the session."),);
if let Err(err) = ctx.compress_session().await {
warn!("Failed to compress the session: {err}");
}
auto_compress_session(ctx).await;
if let Some(session) = ctx.session.as_mut() {
session.set_compressing(false);
}
@@ -1632,6 +1695,13 @@ fn print_turn_divider() {
println!();
}
async fn auto_compress_session(ctx: &mut RequestContext) {
match ctx.compress_session().await {
Ok(()) => hooks::fire(HookEvent::SessionCompressed, ctx, &[], None),
Err(err) => warn!("Failed to compress the session: {err}"),
}
}
fn should_continue(ctx: &RequestContext) -> bool {
let config = ctx.auto_continue_config();
ctx.app.config.function_calling_support
@@ -1939,6 +2009,320 @@ pub fn split_args_text(line: &str, is_win: bool) -> (Vec<String>, &str) {
#[cfg(test)]
mod tests {
use super::*;
use crate::client::{ClientConfig, Model};
use crate::config::{AppState, RoleLike, Session, WorkingMode};
use crate::hooks::{HookDef, HooksMap, test_sink};
use anyhow::anyhow;
use serial_test::serial;
use std::future::Future;
const TURN_EVENTS: [&str; 4] = [
"turn.started",
"turn.completed",
"turn.interrupted",
"turn.failed",
];
fn ctx_with_hooks(events: &[&str], marker: &str) -> RequestContext {
let mut hooks_map = HooksMap::default();
for event in events {
hooks_map.insert(
event.to_string(),
vec![HookDef {
name: format!("{marker}-{event}"),
command: "true".to_string(),
}],
);
}
let mut app = AppState::test_default();
app.config = Arc::new(AppConfig {
hooks: hooks_map,
..Default::default()
});
RequestContext::new(Arc::new(app), WorkingMode::Repl)
}
/// Sink captures for `marker`, ordered (started, completed, interrupted, failed).
fn turn_counts(marker: &str) -> (usize, usize, usize, usize) {
let captures = test_sink::snapshot();
let count = |event: &str| {
captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-{event}"))
.count()
};
(
count("turn.started"),
count("turn.completed"),
count("turn.interrupted"),
count("turn.failed"),
)
}
#[tokio::test]
#[serial]
async fn turn_bracket_ok_final_fires_started_and_completed() {
let _sink = test_sink::install();
let marker = "tb-ok-x7q";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = async { Ok(()) }.await;
bracket.finish(&ctx, &abort_signal, result).unwrap();
assert_eq!(turn_counts(marker), (1, 1, 0, 0));
}
#[tokio::test]
#[serial]
async fn turn_bracket_tool_rounds_then_final_fire_one_turn() {
let _sink = test_sink::install();
let marker = "tb-rounds-p2m";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = async {
// Simulates a turn that loops through tool rounds before the
// final round; rounds happen inside the bracket and never re-fire.
let mut rounds = 0;
loop {
rounds += 1;
if rounds == 3 {
break Ok(());
}
}
}
.await;
bracket.finish(&ctx, &abort_signal, result).unwrap();
assert_eq!(turn_counts(marker), (1, 1, 0, 0));
}
#[tokio::test]
#[serial]
async fn turn_bracket_error_fires_failed_with_error_env() {
let _sink = test_sink::install();
let marker = "tb-err-k4d";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = async { Err(anyhow!("turn exploded")) }.await;
let err = bracket.finish(&ctx, &abort_signal, result).unwrap_err();
assert_eq!(err.to_string(), "turn exploded");
assert_eq!(turn_counts(marker), (1, 0, 0, 1));
let captures = test_sink::snapshot();
let failed = captures
.iter()
.find(|capture| capture.hook_name == format!("{marker}-turn.failed"))
.expect("turn.failed capture");
assert_eq!(
failed.envs.get("COYOTE_ERROR").map(String::as_str),
Some("turn exploded")
);
}
#[tokio::test]
#[serial]
async fn turn_bracket_ctrlc_error_fires_interrupted_not_failed() {
let _sink = test_sink::install();
let marker = "tb-ctrlc-err-j6w";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
abort_signal.set_ctrlc();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = async { Err(anyhow!("Aborted!")) }.await;
assert!(bracket.finish(&ctx, &abort_signal, result).is_err());
assert_eq!(turn_counts(marker), (1, 0, 1, 0));
}
#[tokio::test]
#[serial]
async fn turn_bracket_ctrlc_with_ok_exit_fires_interrupted() {
let _sink = test_sink::install();
let marker = "tb-ctrlc-ok-r3v";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
abort_signal.set_ctrlc();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = async { Ok(()) }.await;
bracket.finish(&ctx, &abort_signal, result).unwrap();
assert_eq!(turn_counts(marker), (1, 0, 1, 0));
}
/// Drives a deep REPL future to completion on a thread with extra stack
/// headroom: nested `run_repl_command`/`compress_session` poll frames
/// are deep in debug builds and overflow the default test-thread stack
/// (Windows exhausts it first).
fn run_async<F>(f: F) -> F::Output
where
F: Future + Send,
F::Output: Send,
{
std::thread::scope(|scope| {
std::thread::Builder::new()
.stack_size(8 * 1024 * 1024)
.spawn_scoped(scope, || {
tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap()
.block_on(f)
})
.unwrap()
.join()
.unwrap()
})
}
#[test]
#[serial]
fn compress_command_failure_fires_no_session_compressed() {
let _sink = test_sink::install();
let marker = "compress-fail-z9t";
run_async(async {
let mut ctx = ctx_with_hooks(&["session.compressed"], marker);
ctx.session = Some(Session::default());
let abort_signal = create_abort_signal();
// Boxed: `run_repl_command`'s state machine is far larger than a
// test thread's stack.
let result = Box::pin(run_repl_command(
&mut ctx,
abort_signal,
".compress session",
))
.await;
assert!(result.is_err(), "compressing an empty session must fail");
});
assert_eq!(
test_sink::snapshot()
.iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.count(),
0,
"a failed compression must fire no session.compressed hook"
);
}
/// A ctx whose compression can actually SUCCEED offline: `dry_run`
/// makes the summarization call echo instead of hitting the network,
/// the default openai client config makes the session model
/// resolvable, and the session already holds a user exchange so
/// `compress_session` has something to summarize.
fn compressible_ctx(marker: &str) -> RequestContext {
let mut hooks_map = HooksMap::default();
hooks_map.insert(
"session.compressed".to_string(),
vec![HookDef {
name: format!("{marker}-session.compressed"),
command: "true".to_string(),
}],
);
let mut app = AppState::test_default();
app.config = Arc::new(AppConfig {
hooks: hooks_map,
dry_run: true,
clients: vec![ClientConfig::default()],
..Default::default()
});
let mut ctx = RequestContext::new(Arc::new(app), WorkingMode::Repl);
let mut session = Session::default();
session.set_model(Model::new("openai", "test-compress-model"));
ctx.session = Some(session);
let input = Input::from_str(&ctx, "hello there", None).unwrap();
ctx.session
.as_mut()
.unwrap()
.add_message(&input, "hi")
.unwrap();
ctx
}
fn session_compressed_count(marker: &str) -> usize {
test_sink::snapshot()
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-session.compressed"))
.count()
}
#[test]
#[serial]
fn compress_command_success_fires_session_compressed_once() {
let _sink = test_sink::install();
let marker = "compress-ok-w2p";
run_async(async {
let mut ctx = compressible_ctx(marker);
let abort_signal = create_abort_signal();
// Boxed: `run_repl_command`'s state machine is far larger than a
// test thread's stack.
let result = Box::pin(run_repl_command(
&mut ctx,
abort_signal,
".compress session",
))
.await;
assert!(
result.is_ok(),
"dry-run compression must succeed: {result:?}"
);
});
assert_eq!(
session_compressed_count(marker),
1,
"a successful `.compress session` must fire session.compressed exactly once"
);
}
#[test]
#[serial]
fn auto_compress_success_fires_session_compressed_once() {
// Covers the ask-loop call site through its extracted seam;
// driving `needs_compression` itself requires a full LLM turn and
// is exercised end-to-end by integration coverage, not here.
let _sink = test_sink::install();
let marker = "auto-compress-ok-f8r";
run_async(async {
let mut ctx = compressible_ctx(marker);
auto_compress_session(&mut ctx).await;
});
assert_eq!(
session_compressed_count(marker),
1,
"a successful auto-compression must fire session.compressed exactly once"
);
}
#[test]
#[serial]
fn auto_compress_failure_fires_no_session_compressed_and_swallows_error() {
let _sink = test_sink::install();
let marker = "auto-compress-fail-d3k";
run_async(async {
let mut ctx = compressible_ctx(marker);
// An empty session makes compress_session fail; the seam must
// swallow the error (no panic, no propagation) and fire nothing.
ctx.session = Some(Session::default());
auto_compress_session(&mut ctx).await;
});
assert_eq!(
session_compressed_count(marker),
0,
"a failed auto-compression must fire no session.compressed hook"
);
}
#[test]
fn test_process_command_line() {
+65
View File
@@ -200,6 +200,12 @@ pub fn discover() -> Result<Vec<DiscoveredMixin>> {
for path in collect_mixins(&paths::rags_dir(), &[ScanMode::Flat]) {
out.push(read_mixin(path)?);
}
for path in collect_mixins(&paths::roles_dir(), &[ScanMode::Flat]) {
out.push(read_mixin(path)?);
}
for path in collect_mixins(&paths::roles_dir().join("hooks"), &[ScanMode::Flat]) {
out.push(read_mixin(path)?);
}
if let Ok(cwd) = env::current_dir()
&& let Some(path) = paths::find_workspace_sbx_mixin(&cwd)
@@ -940,6 +946,65 @@ network:
let _ = fs::remove_dir_all(&root);
}
#[test]
#[serial_test::serial]
fn discover_scans_roles_and_role_hooks_mixins() {
use crate::utils::get_env_name;
let root = unique_root("discover-roles");
let roles = root.join("roles");
fs::create_dir_all(roles.join("hooks")).unwrap();
let role_mixin = roles.join("review.sbx-mixin.yaml");
let hook_mixin = roles.join("hooks").join("notify.sbx-mixin.yaml");
fs::write(&role_mixin, "kind: mixin\n").unwrap();
fs::write(&hook_mixin, "kind: mixin\n").unwrap();
// The builtin installer's hidden manifest lives in the same dir but
// is not a mixin and must never be picked up by the flat scan.
let manifest = roles
.join("hooks")
.join(crate::config::builtin_manifest::BUILTIN_MANIFEST_FILE);
fs::write(&manifest, "notify.sh\n").unwrap();
let config_env = get_env_name("config_dir");
let roles_env = get_env_name("roles_dir");
let prev_config = env::var_os(&config_env);
let prev_roles = env::var_os(&roles_env);
unsafe {
env::set_var(&config_env, &root);
env::set_var(&roles_env, &roles);
}
let discovered = discover();
unsafe {
match prev_config {
Some(v) => env::set_var(&config_env, v),
None => env::remove_var(&config_env),
}
match prev_roles {
Some(v) => env::set_var(&roles_env, v),
None => env::remove_var(&roles_env),
}
}
let found: Vec<PathBuf> = discovered
.unwrap()
.into_iter()
.map(|mixin| mixin.path)
.collect();
assert!(found.contains(&role_mixin), "missing role mixin: {found:?}");
assert!(
found.contains(&hook_mixin),
"missing role hook mixin: {found:?}"
);
assert!(
!found.contains(&manifest),
"builtin manifest must not be scanned as a mixin: {found:?}"
);
let _ = fs::remove_dir_all(&root);
}
/// `generate_rag_sbx_mixin` writes an agent-scoped RAG sidecar next to the
/// rag yaml, at `<agents>/<agent>/<rag>.sbx-mixin.yaml`. Before `SubdirFlat`
/// existed, nothing scanned that shape and attaching a Qdrant RAG from
+53 -14
View File
@@ -3,27 +3,64 @@
use log::{Level, LevelFilter, Log, Metadata, Record};
use std::sync::{Mutex, Once, OnceLock};
struct WarnCollector;
struct TestLogCollector;
static WARN_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
static DEBUG_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
/// Every warn-level message captured since the collector was installed. The
/// buffer is process-global and shared by all tests, so assert by filtering
/// for a marker unique to the test rather than on the whole buffer.
/// Debug capture is restricted to the hooks module so the buffer is not
/// flooded by debug output from the rest of the crate. Matching is
/// module-boundary-aware: only this exact module or its `::` submodules
/// qualify, so a sibling like `hooks_registry` would not.
const DEBUG_TARGET_PREFIX: &str = concat!(env!("CARGO_CRATE_NAME"), "::hooks");
fn captures_warn(metadata: &Metadata) -> bool {
metadata.level() <= Level::Warn
}
fn captures_hooks_debug(metadata: &Metadata) -> bool {
if metadata.level() != Level::Debug {
return false;
}
let target = metadata.target();
target == DEBUG_TARGET_PREFIX
|| target
.strip_prefix(DEBUG_TARGET_PREFIX)
.is_some_and(|rest| rest.starts_with("::"))
}
/// Every warn- or error-level message captured since the collector was
/// installed. The buffer is process-global and shared by all tests, so assert
/// by filtering for a marker unique to the test rather than on the whole
/// buffer.
pub(crate) fn warn_messages() -> &'static Mutex<Vec<String>> {
WARN_MESSAGES.get_or_init(Mutex::default)
}
impl Log for WarnCollector {
fn enabled(&self, metadata: &Metadata) -> bool {
metadata.level() <= Level::Warn
/// Every debug-level message from the hooks module captured since the
/// collector was installed. Same marker-filtering discipline as
/// [`warn_messages`].
pub(crate) fn debug_messages() -> &'static Mutex<Vec<String>> {
DEBUG_MESSAGES.get_or_init(Mutex::default)
}
impl Log for TestLogCollector {
fn enabled(&self, metadata: &Metadata) -> bool {
captures_warn(metadata) || captures_hooks_debug(metadata)
}
// A test that panics while holding a buffer poisons its mutex; the logger
// recovers via `into_inner` so every later test can still log.
fn log(&self, record: &Record) {
if self.enabled(record.metadata()) {
if captures_warn(record.metadata()) {
warn_messages()
.lock()
.unwrap()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.push(record.args().to_string());
} else if captures_hooks_debug(record.metadata()) {
debug_messages()
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.push(record.args().to_string());
}
}
@@ -31,13 +68,15 @@ impl Log for WarnCollector {
fn flush(&self) {}
}
/// Installs the process-wide warn collector. `log::set_logger` accepts one
/// logger per process, so every test that captures warns must install through
/// this shared entry point.
/// Installs the process-wide log collector. `log::set_logger` accepts one
/// logger per process, so every test that captures warns or hooks debug
/// output must install through this shared entry point. The max level is
/// Debug so the hooks debug capture sees its records; warn capture is
/// unaffected.
pub(crate) fn install_warn_collector() {
static INSTALL: Once = Once::new();
INSTALL.call_once(|| {
log::set_logger(&WarnCollector).expect("no other logger should be installed");
log::set_max_level(LevelFilter::Warn);
log::set_logger(&TestLogCollector).expect("no other logger should be installed");
log::set_max_level(LevelFilter::Debug);
});
}