Merge pull request #31 from Dark-Alex-17/feat/PLAN-hooks-v2

PLAN-hooks-v2: hook system follow-ups and v2 features
This commit is contained in:
Alex Clarke authored and GitHub committed 2026-09-17 15:26:41 -06:00
commit dc6c176b4f
39 files changed
+5783 -1016

No files matched your search

+6
View File
@@ -1 +1,7 @@
assets/config-template.yaml text eol=lf assets/config-template.yaml text eol=lf
assets/hooks/*.sh text eol=lf
assets/agents/** text eol=lf
assets/functions/tools/* text eol=lf
assets/functions/scripts/* text eol=lf
assets/functions/utils/*.sh text eol=lf
scripts/*.sh text eol=lf
+1
View File
@@ -56,6 +56,7 @@ Coming from [AIChat](https://github.com/sigoden/aichat)? Follow the [migration g
* [Agents](https://github.com/Dark-Alex-17/coyote/wiki/Agents): Leverage AI agents to perform complex tasks and workflows, including sub-agent spawning, teammate messaging, and user interaction tools. * [Agents](https://github.com/Dark-Alex-17/coyote/wiki/Agents): Leverage AI agents to perform complex tasks and workflows, including sub-agent spawning, teammate messaging, and user interaction tools.
* [Graph Agents](https://github.com/Dark-Alex-17/coyote/wiki/Graph-Agents): Define an agent as a declarative, YAML-driven workflow. A directed graph of typed nodes (LLM calls, scripts, approvals, user input, RAG retrieval, sub-agent spawns). * [Graph Agents](https://github.com/Dark-Alex-17/coyote/wiki/Graph-Agents): Define an agent as a declarative, YAML-driven workflow. A directed graph of typed nodes (LLM calls, scripts, approvals, user input, RAG retrieval, sub-agent spawns).
* [Background Jobs](https://github.com/Dark-Alex-17/coyote/wiki/Background-Jobs): Run long tool calls (builds, test suites, slow MCP calls) in the background with the `job__*` tools while the model keeps working, and completion arrives as a push notification. * [Background Jobs](https://github.com/Dark-Alex-17/coyote/wiki/Background-Jobs): Run long tool calls (builds, test suites, slow MCP calls) in the background with the `job__*` tools while the model keeps working, and completion arrives as a push notification.
* [Hooks](https://github.com/Dark-Alex-17/coyote/wiki/Hooks): Run your own fire-and-forget commands when lifecycle events fire. Turns, tools, LLM requests, agents, sub-agent escalations, and more (26 events). Perfect for desktop notifications, activity logging, and downstream automation; hooks never block Coyote or change its behavior.
* [Todo System](https://github.com/Dark-Alex-17/coyote/wiki/TODO-System): Built-in task tracking for improved LLM reliability with smaller models. * [Todo System](https://github.com/Dark-Alex-17/coyote/wiki/TODO-System): Built-in task tracking for improved LLM reliability with smaller models.
* [Environment Variables](https://github.com/Dark-Alex-17/coyote/wiki/Environment-Variables): Override and customize your Coyote configuration at runtime with environment variables. * [Environment Variables](https://github.com/Dark-Alex-17/coyote/wiki/Environment-Variables): Override and customize your Coyote configuration at runtime with environment variables.
* [Client Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Clients): Configuration instructions for various LLM providers. * [Client Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Clients): Configuration instructions for various LLM providers.
+16 -4
View File
@@ -31,7 +31,9 @@ in-script degradation would never fire. Each command gets
min(PER_COMMAND_TIMEOUT_SECS, remaining budget); commands the deadline min(PER_COMMAND_TIMEOUT_SECS, remaining budget); commands the deadline
leaves no budget for are recorded as {"cmd", "skipped": "deadline"} entries, leaves no budget for are recorded as {"cmd", "skipped": "deadline"} entries,
so the in-script handling stays authoritative. The env var so the in-script handling stays authoritative. The env var
ADVERSARY_RUN_CHECKS_DEADLINE_SECS overrides the deadline (test seam). ADVERSARY_RUN_CHECKS_DEADLINE_SECS overrides the deadline (test seam), and
ADVERSARY_STACK_LOCK_PATH overrides the review-stack lock file location
(isolation seam — see acquire_stack_lock).
Process hygiene: commands run through the shell. On POSIX each command gets Process hygiene: commands run through the shell. On POSIX each command gets
its own session (start_new_session) and a timeout SIGKILLs the whole process its own session (start_new_session) and a timeout SIGKILLs the whole process
@@ -51,14 +53,20 @@ GRAPH_STATE_FILE, so an inherited live state file would silently win.
import json import json
import os import os
import signal import signal
import fcntl
import subprocess import subprocess
import sys import sys
import time import time
try:
import fcntl
except ImportError: # non-POSIX (Windows): advisory flock unavailable
fcntl = None
PER_COMMAND_TIMEOUT_SECS = 900 PER_COMMAND_TIMEOUT_SECS = 900
TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300) TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300)
STACK_LOCK_PATH = os.path.expanduser("~/.cache/coyote/review-stack.lock") STACK_LOCK_PATH = os.environ.get("ADVERSARY_STACK_LOCK_PATH") or os.path.expanduser(
"~/.cache/coyote/review-stack.lock"
)
STACK_LOCK_WAIT_SECS = float(os.environ.get("ADVERSARY_STACK_LOCK_WAIT_SECS") or 1200) STACK_LOCK_WAIT_SECS = float(os.environ.get("ADVERSARY_STACK_LOCK_WAIT_SECS") or 1200)
@@ -67,7 +75,11 @@ def acquire_stack_lock():
well-known path; the probe is instructed to flock the same file around well-known path; the probe is instructed to flock the same file around
stack boot/teardown). Bounded wait so a hogged lock can never turn into a stack boot/teardown). Bounded wait so a hogged lock can never turn into a
false red verification: on timeout we proceed WITHOUT the lock and note it false red verification: on timeout we proceed WITHOUT the lock and note it
on stderr. The fd is held for the process lifetime (released on exit).""" on stderr. The fd is held for the process lifetime (released on exit).
On non-POSIX platforms (no fcntl) the lock is skipped entirely."""
if fcntl is None:
sys.stderr.write("WARN: review-stack lock unsupported on this platform; running unlocked\n")
return None
try: try:
os.makedirs(os.path.dirname(STACK_LOCK_PATH), exist_ok=True) os.makedirs(os.path.dirname(STACK_LOCK_PATH), exist_ok=True)
fd = os.open(STACK_LOCK_PATH, os.O_CREAT | os.O_RDWR) fd = os.open(STACK_LOCK_PATH, os.O_CREAT | os.O_RDWR)
+7 -6
View File
@@ -4,7 +4,8 @@
# each one carries. # each one carries.
# #
# Env vars read: # Env vars read:
# COYOTE_HOOK_LOG log file to append to (default: /tmp/coyote-hooks.log) # COYOTE_HOOK_LOG log file to append to
# (default: ${XDG_STATE_HOME:-$HOME/.local/state}/coyote/hooks.log)
# COYOTE_* everything coyote sets for the event is dumped verbatim # COYOTE_* everything coyote sets for the event is dumped verbatim
# #
# Secrets (COYOTE_SECRET_*) are excluded from the snapshot; widening the grep # Secrets (COYOTE_SECRET_*) are excluded from the snapshot; widening the grep
@@ -26,15 +27,15 @@
set -u set -u
log_file="${COYOTE_HOOK_LOG:-/tmp/coyote-hooks.log}" log_file="${COYOTE_HOOK_LOG:-${XDG_STATE_HOME:-$HOME/.local/state}/coyote/hooks.log}"
# The default lives in shared /tmp: refuse to append through a symlink someone # COYOTE_HOOK_LOG may point into a shared directory: refuse to append through
# else planted there. Point COYOTE_HOOK_LOG at a private path to avoid the # a symlink someone else planted there.
# shared directory entirely.
[ -L "$log_file" ] && exit 0 [ -L "$log_file" ] && exit 0
umask 077 umask 077
mkdir -p "$(dirname "$log_file")" 2> /dev/null || true
{ {
echo "=== $(date '+%Y-%m-%dT%H:%M:%S%z') ${COYOTE_EVENT:-unknown}" echo "=== $(date -u '+%Y-%m-%dT%H:%M:%SZ') ${COYOTE_EVENT:-unknown}"
env | grep '^COYOTE_' | grep -v '^COYOTE_SECRET_' | sort env | grep '^COYOTE_' | grep -v '^COYOTE_SECRET_' | sort
echo echo
} >> "$log_file" 2> /dev/null || true } >> "$log_file" 2> /dev/null || true
+4 -1
View File
@@ -38,5 +38,8 @@ elif command -v osascript > /dev/null 2>&1; then
else else
# coyote discards hook stdout, so aim the fallback at the controlling # coyote discards hook stdout, so aim the fallback at the controlling
# terminal when one exists; plain stdout keeps direct/manual runs working. # terminal when one exists; plain stdout keeps direct/manual runs working.
{ echo "[$title] $body" > /dev/tty; } 2> /dev/null || echo "[$title] $body" # $body carries model-influenced values: strip control bytes (including
# ESC, so ANSI/OSC sequences cannot drive the terminal) before echoing.
safe_line="$(printf '%s' "[$title] $body" | tr -d '\000-\010\013\014\016-\037\177')"
{ echo "$safe_line" > /dev/tty; } 2> /dev/null || echo "$safe_line"
fi fi
-232
View File
@@ -1,232 +0,0 @@
#!/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"
+4 -4
View File
@@ -51,12 +51,12 @@ pub(super) fn model_completer(current: &OsStr) -> Vec<CompletionCandidate> {
fn load_app_config_for_completion() -> anyhow::Result<AppConfig> { fn load_app_config_for_completion() -> anyhow::Result<AppConfig> {
let h = tokio::runtime::Handle::try_current().ok(); let h = tokio::runtime::Handle::try_current().ok();
let cfg = match h { let cfg = match h {
Some(handle) => { Some(handle) => tokio::task::block_in_place(|| {
tokio::task::block_in_place(|| handle.block_on(Config::load_with_interpolation(true)))? handle.block_on(Config::load_with_interpolation(true, true))
} })?,
None => { None => {
let rt = tokio::runtime::Runtime::new()?; let rt = tokio::runtime::Runtime::new()?;
rt.block_on(Config::load_with_interpolation(true))? rt.block_on(Config::load_with_interpolation(true, true))?
} }
}; };
AppConfig::from_config(cfg) AppConfig::from_config(cfg)
+7 -9
View File
@@ -613,9 +613,9 @@ fn llm_failed_extras(
} }
extras.push(( extras.push((
"COYOTE_LLM_ERROR", "COYOTE_LLM_ERROR",
// `{:#}` renders the whole context chain, so the root cause survives // The immediate error only, matching COYOTE_TOOL_ERROR; the context
// the `context(...)` wrapping the client trait defaults apply. // chain below it stays out of the env.
err.map(|err| format!("{err:#}")) err.map(|err| err.to_string())
.unwrap_or_else(|| "Aborted.".to_string()), .unwrap_or_else(|| "Aborted.".to_string()),
)); ));
@@ -1896,12 +1896,10 @@ mod tests {
failed.envs.get("COYOTE_LLM_STATUS").map(String::as_str), failed.envs.get("COYOTE_LLM_STATUS").map(String::as_str),
Some("429") Some("429")
); );
assert!( assert_eq!(
failed failed.envs.get("COYOTE_LLM_ERROR").map(String::as_str),
.envs Some("Failed to call chat-completions api"),
.get("COYOTE_LLM_ERROR") "the env must carry the immediate error only, not the context chain"
.unwrap()
.contains("error (status: 429)")
); );
assert!(!failed.envs.contains_key("COYOTE_LLM_DURATION_MS")); assert!(!failed.envs.contains_key("COYOTE_LLM_DURATION_MS"));
} }
+348 -22
View File
@@ -1,6 +1,6 @@
use super::*; use super::*;
use crate::hooks::HooksMap; use crate::hooks::{HooksMap, RagSyncHooks};
use crate::{ use crate::{
client::Model, client::Model,
config::memory, config::memory,
@@ -14,11 +14,13 @@ use crate::{
use super::rag_cache::RagKey; use super::rag_cache::RagKey;
use crate::config::builtin_manifest; use crate::config::builtin_manifest;
use crate::config::conflict::{self, InstallMode, StickyMode};
use crate::config::paths; use crate::config::paths;
use crate::config::prompts::{ use crate::config::prompts::{
DEFAULT_JOB_INSTRUCTIONS, DEFAULT_SPAWN_INSTRUCTIONS, DEFAULT_TEAMMATE_INSTRUCTIONS, DEFAULT_JOB_INSTRUCTIONS, DEFAULT_SPAWN_INSTRUCTIONS, DEFAULT_TEAMMATE_INSTRUCTIONS,
DEFAULT_TODO_INSTRUCTIONS, DEFAULT_USER_INTERACTION_INSTRUCTIONS, DEFAULT_TODO_INSTRUCTIONS, DEFAULT_USER_INTERACTION_INSTRUCTIONS,
}; };
use crate::function::write_file_atomic;
use crate::graph::types::RagNode; use crate::graph::types::RagNode;
use crate::graph::{Graph, GraphParser, NodeType}; use crate::graph::{Graph, GraphParser, NodeType};
use crate::mcp::McpServerFeatures; use crate::mcp::McpServerFeatures;
@@ -29,8 +31,8 @@ use fancy_regex::Captures;
use inquire::{Text, validator::Validation}; use inquire::{Text, validator::Validation};
use rust_embed::Embed; use rust_embed::Embed;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::collections::BTreeSet; use std::collections::{BTreeMap, BTreeSet};
use std::{env, ffi::OsStr, path::Path}; use std::{env, path::Path};
const DEFAULT_AGENT_NAME: &str = "rag"; const DEFAULT_AGENT_NAME: &str = "rag";
@@ -63,6 +65,101 @@ fn direct_hook_name(rest: &str) -> Option<&str> {
(!name.is_empty() && !name.contains('/')).then_some(name) (!name.is_empty() && !name.contains('/')).then_some(name)
} }
/// Installs one bundled agent's hook scripts under `<agent>/hooks/` and
/// reconciles that directory through its builtin manifest, mirroring the
/// role-side hook installer.
pub(crate) fn install_and_reconcile_agent_hooks(
agent: &str,
shipped: &[(String, String)],
mode: InstallMode,
sticky: &mut StickyMode,
) -> Result<()> {
let dir = paths::agents_data_dir().join(agent).join("hooks");
let mut written = BTreeSet::new();
for (name, content) in shipped {
let path = dir.join(name);
if path.exists()
&& !conflict::should_replace_existing(&path, content, "agents", mode, sticky)?
{
debug!(
"Agent hook file already exists, skipping: {}",
path.display()
);
continue;
}
ensure_parent_exists(&path)?;
info!("Creating agent hook file: {}", path.display());
write_file_atomic(&path, content, None)?;
set_executable_bit_if_script(&path)?;
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 the install, matching the role-side policy.
if let Err(err) = builtin_manifest::reconcile_builtin_dir(&dir, &names, &written) {
warn!("Failed to reconcile builtin hooks for agent '{agent}': {err}");
}
Ok(())
}
/// An agent the embed stopped bundling is never visited by the per-agent
/// reconcile loop, so its installed hooks and manifest would orphan forever.
/// Any agent directory that carries a builtin hooks manifest but is absent
/// from the current embed gets its manifest-owned hooks removed, each
/// deletion logged individually by the reconcile. Only the installer writes
/// manifests, but a manual copy of a builtin agent directory carries the
/// hidden manifest along: files it lists are treated as installer-owned and
/// swept, while files absent from the manifest always survive.
/// Best-effort: a failure here must not abort the install.
fn sweep_removed_agent_hooks(bundled_files: &HashMap<String, HashSet<String>>) {
let entries = match read_dir(paths::agents_data_dir()) {
Ok(entries) => entries,
Err(err) => {
warn!("Failed to scan agents dir for removed-agent hooks: {err}");
return;
}
};
for entry in entries.flatten() {
let name = entry.file_name();
let Some(name) = name.to_str() else {
continue;
};
if bundled_files.contains_key(name) {
continue;
}
// A symlinked agent dir is user-arranged: never follow it into a
// hooks tree that lives somewhere else.
if !entry
.path()
.symlink_metadata()
.is_ok_and(|metadata| metadata.is_dir())
{
continue;
}
let hooks_dir = entry.path().join("hooks");
if !hooks_dir
.join(builtin_manifest::BUILTIN_MANIFEST_FILE)
.is_file()
{
continue;
}
info!(
"Removing hooks shipped by removed built-in agent '{name}' in {}",
hooks_dir.display()
);
if let Err(err) =
builtin_manifest::reconcile_builtin_dir(&hooks_dir, &BTreeSet::new(), &BTreeSet::new())
{
warn!("Failed to reconcile builtin hooks for removed agent '{name}': {err}");
continue;
}
// remove_dir only succeeds on an empty directory, so a user file
// left behind keeps the directory in place.
let _ = fs::remove_dir(&hooks_dir);
}
}
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct Agent { pub struct Agent {
name: String, name: String,
@@ -81,28 +178,32 @@ pub struct Agent {
} }
impl Agent { impl Agent {
pub fn install_builtin_agents(force: bool) -> Result<()> { pub fn install_builtin_agents(mode: InstallMode) -> Result<()> {
info!( info!(
"Installing built-in agents in {}", "Installing built-in agents in {}",
paths::agents_data_dir().display() paths::agents_data_dir().display()
); );
let mut sticky = StickyMode::None;
let mut written_hooks: HashMap<String, BTreeSet<String>> = HashMap::new(); let mut written_hooks: HashMap<String, BTreeSet<String>> = HashMap::new();
for file in AgentAssets::iter() { for file in AgentAssets::iter() {
debug!("Processing agent file: {}", file.as_ref()); debug!("Processing agent file: {}", file.as_ref());
let embedded_file = AgentAssets::get(&file) let embedded_file = AgentAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded agent file: {}", file.as_ref()))?; .ok_or_else(|| anyhow!("Failed to load embedded agent file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) }; let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled agent asset is not valid UTF-8");
let file_path = paths::agents_data_dir().join(file.as_ref()); let file_path = paths::agents_data_dir().join(file.as_ref());
let file_extension = file_path
.extension()
.and_then(OsStr::to_str)
.map(|s| s.to_lowercase());
#[cfg_attr(not(unix), expect(unused))]
let is_script = matches!(file_extension.as_deref(), Some("sh") | Some("py"));
if file_path.exists() && !force { if file_path.exists()
&& !conflict::should_replace_existing(
&file_path,
content,
"agents",
mode,
&mut sticky,
)?
{
debug!( debug!(
"Agent file already exists, skipping: {}", "Agent file already exists, skipping: {}",
file_path.display() file_path.display()
@@ -114,18 +215,13 @@ impl Agent {
info!("Creating agent file: {}", file_path.display()); info!("Creating agent file: {}", file_path.display());
let mut agent_file = File::create(&file_path)?; let mut agent_file = File::create(&file_path)?;
agent_file.write_all(content.as_bytes())?; agent_file.write_all(content.as_bytes())?;
set_executable_bit_if_script(&file_path)?;
if let Some((agent, hook)) = parse_direct_hook_path(file.as_ref()) { if let Some((agent, hook)) = parse_direct_hook_path(file.as_ref()) {
written_hooks written_hooks
.entry(agent.to_string()) .entry(agent.to_string())
.or_default() .or_default()
.insert(hook.to_string()); .insert(hook.to_string());
} }
#[cfg(unix)]
if is_script {
use std::{fs, os::unix::fs::PermissionsExt};
fs::set_permissions(&file_path, fs::Permissions::from_mode(0o755))?;
}
} }
let mut bundled_files: HashMap<String, HashSet<String>> = HashMap::new(); let mut bundled_files: HashMap<String, HashSet<String>> = HashMap::new();
@@ -179,6 +275,32 @@ impl Agent {
} }
} }
sweep_removed_agent_hooks(&bundled_files);
Ok(())
}
pub fn install_builtin_agent_hooks(mode: InstallMode, sticky: &mut StickyMode) -> Result<()> {
let mut shipped: BTreeMap<String, Vec<(String, String)>> = BTreeMap::new();
for file in AgentAssets::iter() {
let Some((agent, rest)) = file.as_ref().split_once('/') else {
continue;
};
let hooks = shipped.entry(agent.to_string()).or_default();
let Some(hook) = direct_hook_name(rest) else {
continue;
};
let embedded = AgentAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded agent file: {}", file.as_ref()))?;
let content = std::str::from_utf8(&embedded.data)
.expect("bundled agent hook asset is not valid UTF-8");
hooks.push((hook.to_string(), content.to_string()));
}
for (agent, files) in &shipped {
install_and_reconcile_agent_hooks(agent, files, mode, sticky)?;
}
Ok(()) Ok(())
} }
@@ -234,6 +356,13 @@ impl Agent {
} }
}; };
let rag_sync_hooks = RagSyncHooks::resolve_for_agent(
&app.hooks,
&agent_config.global_hooks,
&agent_config.hooks,
name,
);
let rag = if rag_path.exists() { let rag = if rag_path.exists() {
let key = RagKey::Agent(name.to_string()); let key = RagKey::Agent(name.to_string());
let app_clone = app.clone(); let app_clone = app.clone();
@@ -266,6 +395,7 @@ impl Agent {
let app_clone = app.clone(); let app_clone = app.clone();
let rag_path_clone = rag_path.clone(); let rag_path_clone = rag_path.clone();
let abort = abort_signal.clone(); let abort = abort_signal.clone();
let sync_hooks = rag_sync_hooks.clone();
let rag = app_state let rag = app_state
.rag_cache .rag_cache
.load_with(key, || async move { .load_with(key, || async move {
@@ -276,6 +406,7 @@ impl Agent {
&document_paths, &document_paths,
abort, abort,
true, true,
sync_hooks,
) )
.await .await
}) })
@@ -299,6 +430,7 @@ impl Agent {
&loaders, &loaders,
info_flag, info_flag,
abort_signal.clone(), abort_signal.clone(),
&rag_sync_hooks,
) )
.await? .await?
} }
@@ -1019,10 +1151,13 @@ impl AgentConfig {
if let Some(v) = read_env_value::<String>(&with_prefix("reasoning_effort")) { if let Some(v) = read_env_value::<String>(&with_prefix("reasoning_effort")) {
self.reasoning_effort = v; self.reasoning_effort = v;
} }
if let Ok(v) = env::var(with_prefix("global_tools")) if let Ok(v) = env::var(with_prefix("global_tools")) {
&& let Ok(v) = serde_json::from_str(&v) match serde_json::from_str(&v) {
{ Ok(v) => self.global_tools = v,
self.global_tools = v; Err(err) => {
debug!("Ignoring malformed global_tools env override for agent '{name}': {err}")
}
}
} }
if let Ok(v) = env::var(with_prefix("global_hooks")) { if let Ok(v) = env::var(with_prefix("global_hooks")) {
match serde_json::from_str(&v) { match serde_json::from_str(&v) {
@@ -1178,6 +1313,7 @@ async fn init_graph_rags(
loaders: &HashMap<String, String>, loaders: &HashMap<String, String>,
info_flag: bool, info_flag: bool,
abort_signal: AbortSignal, abort_signal: AbortSignal,
sync_hooks: &RagSyncHooks,
) -> Result<HashMap<String, Arc<Rag>>> { ) -> Result<HashMap<String, Arc<Rag>>> {
let mut rags = HashMap::new(); let mut rags = HashMap::new();
if info_flag { if info_flag {
@@ -1254,6 +1390,7 @@ async fn init_graph_rags(
let path_clone = rag_path.clone(); let path_clone = rag_path.clone();
let name_clone = node_id.clone(); let name_clone = node_id.clone();
let abort = abort_signal.clone(); let abort = abort_signal.clone();
let sync_hooks = sync_hooks.clone();
app_state app_state
.rag_cache .rag_cache
.load_with(key, || async move { .load_with(key, || async move {
@@ -1264,6 +1401,7 @@ async fn init_graph_rags(
&document_paths, &document_paths,
&config, &config,
abort, abort,
sync_hooks,
) )
.await .await
}) })
@@ -1361,6 +1499,7 @@ pub fn complete_agent_variables(agent_name: &str) -> Vec<(String, Option<String>
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::testing;
#[test] #[test]
fn parse_direct_hook_path_accepts_only_direct_hook_children() { fn parse_direct_hook_path_accepts_only_direct_hook_children() {
@@ -1472,6 +1611,33 @@ global_hooks:
} }
} }
#[test]
#[serial_test::serial]
fn load_envs_ignores_malformed_global_tools_and_logs_debug() {
testing::install_log_collector();
let name = "gt-malformed-envtest";
let yaml = format!("name: {name}\ninstructions: hi\nglobal_tools:\n - keep_tool.sh\n");
let mut config: AgentConfig = serde_yaml::from_str(&yaml).unwrap();
let env_name = normalize_env_name(&format!("{name}_global_tools"));
let _guard = testing::EnvVarGuard::set(&env_name, "{not-json");
config.load_envs(&AppConfig::default());
assert_eq!(
config.global_tools,
vec!["keep_tool.sh"],
"a malformed override must leave the existing value untouched"
);
let debugs = testing::debug_snapshot();
assert!(
debugs.iter().any(|message| {
message.contains("Ignoring malformed global_tools env override")
&& message.contains(name)
}),
"expected a debug log for the malformed override: {debugs:?}"
);
}
#[test] #[test]
fn from_graph_carries_hooks_and_global_hooks() { fn from_graph_carries_hooks_and_global_hooks() {
let yaml = "\ let yaml = "\
@@ -1896,4 +2062,164 @@ nodes: {}
); );
assert_eq!(output, expected); assert_eq!(output, expected);
} }
use crate::testing::TestConfigDirGuard;
fn fixture(entries: &[(&str, &str)]) -> Vec<(String, String)> {
entries
.iter()
.map(|(name, content)| (name.to_string(), content.to_string()))
.collect()
}
#[test]
#[serial_test::serial]
fn agent_hooks_prompt_mode_asks_per_conflict_and_honors_sticky() {
use crate::config::conflict::prompt_script;
let _guard = TestConfigDirGuard::new("agent-hooks-prompt");
let shipped = fixture(&[("a.sh", "new content\n"), ("b.sh", "new content\n")]);
let dir = paths::agents_data_dir().join("probe").join("hooks");
create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "local a").unwrap();
fs::write(dir.join("b.sh"), "local b").unwrap();
// First conflict answered per-file, second by the sticky replace-all.
let script = prompt_script::install(&["replace-all"]);
let mut sticky = StickyMode::None;
install_and_reconcile_agent_hooks("probe", &shipped, InstallMode::Prompt, &mut sticky)
.unwrap();
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "new content\n");
assert_eq!(read_to_string(dir.join("b.sh")).unwrap(), "new content\n");
assert_eq!(sticky, StickyMode::ReplaceAll);
assert_eq!(
read_to_string(dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE)).unwrap(),
"a.sh\nb.sh\n"
);
drop(script);
// A sticky mode carried in from an earlier location suppresses all
// prompting: `--install-builtins hooks` shares one sticky scope
// across the global, role, and agent hook locations.
fs::write(dir.join("a.sh"), "local again").unwrap();
let script = prompt_script::install(&[]);
let mut sticky = StickyMode::KeepAll;
install_and_reconcile_agent_hooks("probe", &shipped, InstallMode::Prompt, &mut sticky)
.unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "local again");
drop(script);
}
#[test]
#[serial_test::serial]
fn agent_hooks_reconcile_adds_updates_and_drops_within_a_shipped_agent() {
let _guard = TestConfigDirGuard::new("agent-hooks-reconcile");
let dir = paths::agents_data_dir().join("probe").join("hooks");
install_and_reconcile_agent_hooks(
"probe",
&fixture(&[("keep.sh", "v1\n"), ("drop.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
install_and_reconcile_agent_hooks(
"probe",
&fixture(&[("keep.sh", "v2\n"), ("added.sh", "#!/bin/sh\n")]),
InstallMode::Force,
&mut StickyMode::None,
)
.unwrap();
assert_eq!(
read_to_string(dir.join("keep.sh")).unwrap(),
"v2\n",
"a still-shipped hook updates under Force"
);
assert_eq!(
read_to_string(dir.join("added.sh")).unwrap(),
"#!/bin/sh\n",
"a newly shipped hook is added"
);
assert!(
!dir.join("drop.sh").exists(),
"a dropped hook reconciles away"
);
assert_eq!(
read_to_string(dir.join("user.sh")).unwrap(),
"user-owned",
"user files in the same dir survive"
);
assert_eq!(
read_to_string(dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE)).unwrap(),
"added.sh\nkeep.sh\n"
);
}
#[test]
#[serial_test::serial]
fn install_builtin_agent_hooks_reconciles_hookless_bundled_agents() {
let _guard = TestConfigDirGuard::new("agent-hooks-hookless");
let agent = AgentAssets::iter()
.filter_map(|file| {
file.as_ref()
.split_once('/')
.map(|(agent, _)| agent.to_string())
})
.find(|agent| {
!AgentAssets::iter().any(|file| {
parse_direct_hook_path(file.as_ref())
.is_some_and(|(hook_agent, _)| hook_agent == agent)
})
})
.expect("at least one bundled agent ships no hooks");
let dir = paths::agents_data_dir().join(&agent).join("hooks");
// A manifest-owned hook left over from a release that shipped it.
install_and_reconcile_agent_hooks(
&agent,
&fixture(&[("stale.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap();
Agent::install_builtin_agent_hooks(InstallMode::Skip, &mut StickyMode::None).unwrap();
assert!(
!dir.join("stale.sh").exists(),
"a still-bundled agent with no hook assets must reconcile its hooks dir"
);
assert_eq!(read_to_string(dir.join("user.sh")).unwrap(), "user-owned");
}
#[cfg(unix)]
#[test]
#[serial_test::serial]
fn sweep_removed_agent_hooks_never_follows_symlinked_agent_dirs() {
let guard = TestConfigDirGuard::new("agent-sweep-symlink");
let real = guard.path.join("agent-elsewhere");
let hooks = real.join("hooks");
create_dir_all(&hooks).unwrap();
fs::write(
hooks.join(builtin_manifest::BUILTIN_MANIFEST_FILE),
"shipped.sh\n",
)
.unwrap();
fs::write(hooks.join("shipped.sh"), "stale").unwrap();
create_dir_all(paths::agents_data_dir()).unwrap();
std::os::unix::fs::symlink(&real, paths::agents_data_dir().join("linked")).unwrap();
sweep_removed_agent_hooks(&HashMap::new());
assert!(
hooks.join("shipped.sh").exists(),
"the sweep must never follow a symlinked agent dir"
);
assert!(hooks.join(builtin_manifest::BUILTIN_MANIFEST_FILE).exists());
}
} }
+30
View File
@@ -210,6 +210,18 @@ impl Default for AppConfig {
impl AppConfig { impl AppConfig {
pub fn from_config(config: super::Config) -> Result<Self> { pub fn from_config(config: super::Config) -> Result<Self> {
Self::from_config_impl(config, true)
}
/// Like [`Self::from_config`], but tolerates a config with no resolvable
/// model: `model_id` stays empty instead of failing with "No available
/// model". For inspection readouts (--info, --list-*, --mcp-list,
/// --list-secrets), which render model fields as absent/default.
pub fn from_config_lenient(config: super::Config) -> Result<Self> {
Self::from_config_impl(config, false)
}
fn from_config_impl(config: super::Config, require_model: bool) -> Result<Self> {
let mut app_config = Self { let mut app_config = Self {
model_id: config.model_id, model_id: config.model_id,
temperature: config.temperature, temperature: config.temperature,
@@ -306,7 +318,13 @@ impl AppConfig {
} }
app_config.setup_document_loaders(); app_config.setup_document_loaders();
app_config.setup_user_agent(); app_config.setup_user_agent();
if require_model {
app_config.resolve_model()?; app_config.resolve_model()?;
} else {
// Model-less is fine here: model_id stays empty and inspection
// callers substitute Model::default().
let _ = app_config.resolve_model();
}
app_config.validate_reasoning_effort()?; app_config.validate_reasoning_effort()?;
Ok(app_config) Ok(app_config)
} }
@@ -1269,6 +1287,18 @@ mod tests {
assert!(result.is_err()); assert!(result.is_err());
} }
#[test]
fn from_config_errors_when_no_model_available() {
let result = AppConfig::from_config(Config::default());
assert!(result.is_err());
}
#[test]
fn from_config_lenient_tolerates_missing_model() {
let app = AppConfig::from_config_lenient(Config::default()).unwrap();
assert!(app.model_id.is_empty());
}
#[test] #[test]
fn resolve_model_keeps_explicit_model_id() { fn resolve_model_keeps_explicit_model_id() {
let mut app = AppConfig { let mut app = AppConfig {
+6 -1
View File
@@ -43,6 +43,7 @@ impl AppState {
log_path: Option<PathBuf>, log_path: Option<PathBuf>,
start_mcp_servers: bool, start_mcp_servers: bool,
lenient_vault: bool, lenient_vault: bool,
inspection: bool,
abort_signal: AbortSignal, abort_signal: AbortSignal,
) -> Result<Self> { ) -> Result<Self> {
let vault = if lenient_vault { let vault = if lenient_vault {
@@ -74,7 +75,11 @@ impl AppState {
} }
} }
let mut functions = Functions::init(config.visible_tools.as_deref())?; let mut functions = if inspection {
Functions::default()
} else {
Functions::init(config.visible_tools.as_deref())?
};
if !mcp_registry.is_empty() && config.mcp_server_support { if !mcp_registry.is_empty() && config.mcp_server_support {
functions.append_mcp_meta_functions(mcp_registry.server_features()); functions.append_mcp_meta_functions(mcp_registry.server_features());
} }
+106 -1
View File
@@ -23,6 +23,22 @@ pub(crate) fn is_builtin_manifest_name(name: &str) -> bool {
.eq_ignore_ascii_case(BUILTIN_MANIFEST_FILE) .eq_ignore_ascii_case(BUILTIN_MANIFEST_FILE)
} }
/// NTFS 8.3 short names (`BUILTI~1`, `builti~1.sh`) can alias any long
/// filename on the same volume, including the manifest itself, so an entry
/// shaped like one is never accepted: it could direct a deletion at a file
/// the manifest never named.
fn is_ntfs_short_name_alias(name: &str) -> bool {
let stem = name.split('.').next().unwrap_or(name);
let Some((base, digits)) = stem.rsplit_once('~') else {
return false;
};
!base.is_empty()
&& base.len() <= 8
&& base.chars().all(|c| c.is_ascii_alphanumeric() || c == '~')
&& !digits.is_empty()
&& digits.chars().all(|c| c.is_ascii_digit())
}
/// Reconciles `dir` against the currently shipped set of builtin filenames: /// Reconciles `dir` against the currently shipped set of builtin filenames:
/// a file is removed ONLY if it appears in the previous manifest AND is /// 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 /// absent from `shipped`. The manifest is then rewritten to the names this
@@ -83,7 +99,7 @@ fn read_manifest(dir: &Path) -> BTreeSet<String> {
if line.is_empty() || is_builtin_manifest_name(line) { if line.is_empty() || is_builtin_manifest_name(line) {
return false; return false;
} }
if !is_plain_file_name(line) { if !is_plain_file_name(line) || is_ntfs_short_name_alias(line) {
debug!( debug!(
"Ignoring suspicious builtin manifest entry in {}: {line:?}", "Ignoring suspicious builtin manifest entry in {}: {line:?}",
path.display() path.display()
@@ -290,4 +306,93 @@ mod tests {
assert!(!is_builtin_manifest_name(".builtin-manifesto")); assert!(!is_builtin_manifest_name(".builtin-manifesto"));
assert!(!is_builtin_manifest_name("notify.sh")); assert!(!is_builtin_manifest_name("notify.sh"));
} }
#[test]
fn ntfs_short_name_aliases_are_detected() {
assert!(is_ntfs_short_name_alias("BUILTI~1"));
assert!(is_ntfs_short_name_alias("builti~1.sh"));
assert!(is_ntfs_short_name_alias("NOTIFY~12"));
assert!(is_ntfs_short_name_alias("A~1"));
assert!(!is_ntfs_short_name_alias("notify.sh"));
assert!(!is_ntfs_short_name_alias("my~hook.sh"));
assert!(!is_ntfs_short_name_alias("~1"));
assert!(!is_ntfs_short_name_alias("way-too-long~1"));
assert!(!is_ntfs_short_name_alias(".builtin-manifest"));
}
/// This fixture is unrealizable on NTFS: `.builtin-manifest` gets an
/// auto-generated 8.3 short name of exactly this shape (`BUILTI~1`), so
/// writing `dir/BUILTI~1` opens the manifest through its alias and
/// clobbers it instead of creating a distinct file — the precise
/// collision [`is_ntfs_short_name_alias`] defends against. Only
/// filesystems without 8.3 aliasing can host alias-shaped names as real,
/// distinct files; Windows coverage lives in
/// `ntfs_short_name_entries_never_accepted_on_windows` below.
#[cfg(unix)]
#[test]
fn ntfs_short_name_entries_never_direct_deletions() {
let dir = fresh_dir("builtin-manifest-shortname-");
fs::write(dir.join(BUILTIN_MANIFEST_FILE), "BUILTI~1\nbuilti~1.sh\n").unwrap();
fs::write(dir.join("BUILTI~1"), "keep").unwrap();
fs::write(dir.join("builti~1.sh"), "keep").unwrap();
reconcile_builtin_dir(&dir, &BTreeSet::new(), &BTreeSet::new()).unwrap();
assert!(
dir.join("BUILTI~1").exists() && dir.join("builti~1.sh").exists(),
"8.3-alias-shaped manifest entries must never be removal candidates"
);
let _ = fs::remove_dir_all(&dir);
}
/// Windows counterpart of `ntfs_short_name_entries_never_direct_deletions`:
/// alias-shaped files cannot exist as distinct fixtures here, so the guard
/// invariant is proven through the manifest filter instead. Only entries
/// accepted into the previous-manifest set can become removal candidates
/// (`previous.difference(shipped)`) or be claimed as owned
/// (`shipped ∩ previous`); listing the alias names in `shipped` makes the
/// second path observable in the rewritten manifest — a broken guard
/// would claim them. Where the volume generates 8.3 short names, the
/// self-collision that makes the unix fixture unrealizable is also
/// asserted directly; where generation is disabled, the alias simply
/// never materializes and the invariant assertions stand on their own.
#[cfg(windows)]
#[test]
fn ntfs_short_name_entries_never_accepted_on_windows() {
let dir = fresh_dir("builtin-manifest-shortname-win-");
fs::write(
dir.join(BUILTIN_MANIFEST_FILE),
"BUILTI~1\nbuilti~1.sh\nkept.sh\n",
)
.unwrap();
fs::write(dir.join("kept.sh"), "current").unwrap();
let alias = dir.join("BUILTI~1");
if alias.exists() {
assert_eq!(
fs::read_to_string(&alias).unwrap(),
fs::read_to_string(dir.join(BUILTIN_MANIFEST_FILE)).unwrap(),
"the 8.3 alias opens the manifest itself, not a distinct file"
);
}
reconcile_builtin_dir(
&dir,
&shipped(&["BUILTI~1", "builti~1.sh", "kept.sh"]),
&shipped(&[]),
)
.unwrap();
assert!(dir.join("kept.sh").exists(), "real shipped file untouched");
assert!(
dir.join(BUILTIN_MANIFEST_FILE).exists(),
"manifest survives reconciliation"
);
assert_eq!(
fs::read_to_string(dir.join(BUILTIN_MANIFEST_FILE)).unwrap(),
"kept.sh\n",
"alias-shaped entries are never accepted: neither removal candidates nor claimed"
);
let _ = fs::remove_dir_all(&dir);
}
} }
+364
View File
@@ -0,0 +1,364 @@
//! Shared keep/replace conflict resolution for installers that must not
//! silently clobber locally modified files. The bundle installer and the
//! builtin hook installers all funnel per-file conflicts through
//! [`resolve_conflict`], so the prompt wording and the sticky
//! keep-all/replace-all semantics are identical everywhere, including when
//! one sticky scope spans several install locations in a single run.
use crate::utils::IS_STDOUT_TERMINAL;
use anyhow::{Context, Result, bail};
use inquire::Select;
use std::fs;
use std::path::Path;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum InstallMode {
Skip,
Force,
Prompt,
}
/// A `keep-all`/`replace-all` answer outlives the file it was given for:
/// the caller threads one value through every subsequent conflict in the
/// same run.
#[derive(Debug, Clone, Copy, PartialEq)]
pub(crate) enum StickyMode {
None,
KeepAll,
ReplaceAll,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum ConflictAction {
Keep,
Replace,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum NonInteractive {
Bail,
Keep,
}
pub(crate) fn should_replace_existing(
dst: &Path,
content: &str,
category: &str,
mode: InstallMode,
sticky: &mut StickyMode,
) -> Result<bool> {
match mode {
InstallMode::Skip => Ok(false),
InstallMode::Force => Ok(true),
InstallMode::Prompt => {
if fs::read(dst).is_ok_and(|local| local == content.as_bytes()) {
return Ok(false);
}
match resolve_conflict(dst, category, sticky, NonInteractive::Keep)? {
ConflictAction::Keep => Ok(false),
ConflictAction::Replace => Ok(true),
}
}
}
}
pub(crate) fn resolve_conflict(
dst: &Path,
category: &str,
sticky: &mut StickyMode,
non_interactive: NonInteractive,
) -> Result<ConflictAction> {
match *sticky {
StickyMode::KeepAll => return Ok(ConflictAction::Keep),
StickyMode::ReplaceAll => return Ok(ConflictAction::Replace),
StickyMode::None => {}
}
if !is_interactive() {
match non_interactive {
NonInteractive::Keep => return Ok(ConflictAction::Keep),
NonInteractive::Bail => bail!(
"Refusing to overwrite local file {} non-interactively. \
Re-run in a terminal, with --install-force (installs), \
or with --yes (updates).",
dst.display()
),
}
}
let prompt = format!("Conflict at {} (category: {category})", dst.display());
let choice = select_choice(&prompt)?;
match choice.as_str() {
"keep" => Ok(ConflictAction::Keep),
"replace" => Ok(ConflictAction::Replace),
"keep-all" => {
*sticky = StickyMode::KeepAll;
Ok(ConflictAction::Keep)
}
"replace-all" => {
*sticky = StickyMode::ReplaceAll;
Ok(ConflictAction::Replace)
}
"abort" => bail!("Install aborted by user at conflict resolution."),
_ => unreachable!("inquire::Select returned an unexpected option"),
}
}
fn is_interactive() -> bool {
#[cfg(test)]
if let Some(forced) = prompt_script::forced_tty() {
return forced;
}
*IS_STDOUT_TERMINAL
}
fn select_choice(prompt: &str) -> Result<String> {
#[cfg(test)]
if prompt_script::active() {
return Ok(prompt_script::next_answer(prompt));
}
Select::new(
prompt,
vec!["keep", "replace", "keep-all", "replace-all", "abort"],
)
.prompt()
.map(str::to_string)
.with_context(|| "failed to read conflict choice")
}
#[cfg(test)]
pub(crate) mod prompt_script {
//! Scripted stand-in for the interactive conflict prompt.
//! `inquire::Select` cannot run under the test harness, so while a guard
//! is installed the resolver takes its terminal answer from the guard and
//! serves each prompt from a queue of scripted answers, counting every
//! prompt asked. A forced-terminal guard with an empty queue pins the
//! "never prompts" invariant: any prompt panics and the counter stays 0.
use std::sync::Mutex;
use std::sync::atomic::{AtomicUsize, Ordering};
const INACTIVE: usize = 0;
const TTY: usize = 1;
const NON_TTY: usize = 2;
static STATE: AtomicUsize = AtomicUsize::new(INACTIVE);
static ASKED: AtomicUsize = AtomicUsize::new(0);
// A test that panics while holding the queue poisons its mutex; every
// lock site recovers via `into_inner` so later tests keep working.
static ANSWERS: Mutex<Vec<String>> = Mutex::new(Vec::new());
/// Installs the script with a forced terminal until the guard drops.
/// The script is process-global, so tests using it must serialize
/// (`#[serial]`). Answers are consumed front to back; a prompt beyond
/// the scripted answers panics.
#[must_use]
pub fn install(answers: &[&str]) -> ScriptGuard {
install_with_state(TTY, answers)
}
/// Installs the script with the terminal forced absent, for pinning the
/// non-interactive behavior regardless of where the tests actually run.
#[must_use]
pub fn install_non_interactive() -> ScriptGuard {
install_with_state(NON_TTY, &[])
}
fn install_with_state(state: usize, answers: &[&str]) -> ScriptGuard {
*ANSWERS
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner()) =
answers.iter().map(|answer| answer.to_string()).collect();
ASKED.store(0, Ordering::SeqCst);
STATE.store(state, Ordering::SeqCst);
ScriptGuard
}
pub struct ScriptGuard;
impl Drop for ScriptGuard {
fn drop(&mut self) {
STATE.store(INACTIVE, Ordering::SeqCst);
ANSWERS
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.clear();
}
}
/// Number of prompts asked since the script was installed.
pub fn prompts_asked() -> usize {
ASKED.load(Ordering::SeqCst)
}
pub(super) fn active() -> bool {
STATE.load(Ordering::SeqCst) != INACTIVE
}
pub(super) fn forced_tty() -> Option<bool> {
match STATE.load(Ordering::SeqCst) {
TTY => Some(true),
NON_TTY => Some(false),
_ => None,
}
}
pub(super) fn next_answer(prompt: &str) -> String {
ASKED.fetch_add(1, Ordering::SeqCst);
let mut answers = ANSWERS
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner());
assert!(
!answers.is_empty(),
"conflict prompt asked with no scripted answer left: {prompt}"
);
answers.remove(0)
}
}
#[cfg(test)]
mod tests {
use super::*;
use serial_test::serial;
use std::path::PathBuf;
fn dst() -> PathBuf {
PathBuf::from("/tmp/coyote-conflict-tests/example.yaml")
}
#[test]
fn sticky_modes_short_circuit_without_prompting() {
let mut sticky = StickyMode::KeepAll;
assert!(matches!(
resolve_conflict(&dst(), "macros", &mut sticky, NonInteractive::Bail).unwrap(),
ConflictAction::Keep
));
let mut sticky = StickyMode::ReplaceAll;
assert!(matches!(
resolve_conflict(&dst(), "macros", &mut sticky, NonInteractive::Bail).unwrap(),
ConflictAction::Replace
));
}
#[test]
#[serial]
fn non_interactive_bail_keeps_the_bundle_error_shape() {
let _script = prompt_script::install_non_interactive();
let mut sticky = StickyMode::None;
let err =
resolve_conflict(&dst(), "macros", &mut sticky, NonInteractive::Bail).unwrap_err();
assert!(
err.to_string().contains("Refusing to overwrite local file"),
"got: {err}"
);
assert!(err.to_string().contains("example.yaml"), "got: {err}");
assert_eq!(prompt_script::prompts_asked(), 0);
}
#[test]
#[serial]
fn non_interactive_keep_resolves_without_error_or_prompt() {
let _script = prompt_script::install_non_interactive();
let mut sticky = StickyMode::None;
let action = resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap();
assert!(matches!(action, ConflictAction::Keep));
assert_eq!(sticky, StickyMode::None);
assert_eq!(prompt_script::prompts_asked(), 0);
}
#[test]
#[serial]
fn scripted_answers_drive_actions_and_sticky_transitions() {
let _script = prompt_script::install(&["keep", "replace", "replace-all"]);
let mut sticky = StickyMode::None;
assert!(matches!(
resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap(),
ConflictAction::Keep
));
assert_eq!(sticky, StickyMode::None);
assert!(matches!(
resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap(),
ConflictAction::Replace
));
assert_eq!(sticky, StickyMode::None);
assert!(matches!(
resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap(),
ConflictAction::Replace
));
assert_eq!(sticky, StickyMode::ReplaceAll);
// The sticky answer now short-circuits: no further prompt is asked.
assert!(matches!(
resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap(),
ConflictAction::Replace
));
assert_eq!(prompt_script::prompts_asked(), 3);
}
#[test]
#[serial]
fn abort_answer_bails_with_the_bundle_abort_message() {
let _script = prompt_script::install(&["abort"]);
let mut sticky = StickyMode::None;
let err = resolve_conflict(&dst(), "hooks", &mut sticky, NonInteractive::Keep).unwrap_err();
assert!(
err.to_string()
.contains("Install aborted by user at conflict resolution."),
"got: {err}"
);
}
#[test]
#[serial]
fn should_replace_existing_skips_identical_content_without_prompting() {
let _script = prompt_script::install(&[]);
let dir = crate::utils::temp_file("conflict-identical-", "");
fs::create_dir_all(&dir).unwrap();
let path = dir.join("hook.sh");
fs::write(&path, "same content").unwrap();
let mut sticky = StickyMode::None;
let replace = should_replace_existing(
&path,
"same content",
"hooks",
InstallMode::Prompt,
&mut sticky,
)
.unwrap();
let _ = fs::remove_dir_all(&dir);
assert!(!replace);
assert_eq!(prompt_script::prompts_asked(), 0);
}
#[test]
#[serial]
fn should_replace_existing_never_prompts_in_skip_or_force_mode() {
let _script = prompt_script::install(&[]);
let dir = crate::utils::temp_file("conflict-modes-", "");
fs::create_dir_all(&dir).unwrap();
let path = dir.join("hook.sh");
fs::write(&path, "local edit").unwrap();
let mut sticky = StickyMode::None;
let skip =
should_replace_existing(&path, "shipped", "hooks", InstallMode::Skip, &mut sticky);
let force =
should_replace_existing(&path, "shipped", "hooks", InstallMode::Force, &mut sticky);
let _ = fs::remove_dir_all(&dir);
assert!(!skip.unwrap());
assert!(force.unwrap());
assert_eq!(prompt_script::prompts_asked(), 0);
}
}
+28 -89
View File
@@ -3,9 +3,10 @@ use super::bundles::{
hash_file, hash_file,
}; };
use crate::config::builtin_manifest::is_builtin_manifest_name; use crate::config::builtin_manifest::is_builtin_manifest_name;
use crate::config::{AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths}; use crate::config::conflict::{ConflictAction, NonInteractive, StickyMode, resolve_conflict};
#[cfg(not(windows))] use crate::config::{
use crate::function::Language; AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths, set_executable_bit_if_script,
};
use crate::mcp::{McpServer, McpServersConfig}; use crate::mcp::{McpServer, McpServersConfig};
use crate::utils; use crate::utils;
use crate::utils::IS_STDOUT_TERMINAL; use crate::utils::IS_STDOUT_TERMINAL;
@@ -1857,18 +1858,12 @@ fn print_plan_summary(plan: &InstallPlan) {
} }
/// Bundle-shipped hook scripts execute on the user's machine once wired into /// 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 /// config, so every planned file landing in a hook location is called out
/// out individually. Visibility only: installing them needs no extra approval. /// individually. Visibility only: installing them needs no extra approval.
fn hook_script_lines(plan: &InstallPlan) -> Vec<String> { fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
plan.files plan.files
.iter() .iter()
.filter(|planned| { .filter(|planned| is_hook_location(planned))
planned.top_category == TopCategory::Hooks
|| planned
.rel
.parent()
.is_some_and(|parent| parent.components().any(|c| c.as_os_str() == "hooks"))
})
.map(|planned| { .map(|planned| {
let rel = planned.rel.to_string_lossy().replace('\\', "/"); let rel = planned.rel.to_string_lossy().replace('\\', "/");
format!( format!(
@@ -1879,6 +1874,16 @@ fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
.collect() .collect()
} }
fn is_hook_location(planned: &PlannedFile) -> bool {
let components: Vec<_> = planned.rel.components().collect();
match planned.top_category {
TopCategory::Hooks => true,
TopCategory::Agents => components.len() == 3 && components[1].as_os_str() == "hooks",
TopCategory::Roles => components.len() == 2 && components[0].as_os_str() == "hooks",
_ => false,
}
}
fn count_kind(plan: &InstallPlan, cat: TopCategory, kind: PlannedKind) -> usize { fn count_kind(plan: &InstallPlan, cat: TopCategory, kind: PlannedKind) -> usize {
plan.files plan.files
.iter() .iter()
@@ -1886,18 +1891,6 @@ fn count_kind(plan: &InstallPlan, cat: TopCategory, kind: PlannedKind) -> usize
.count() .count()
} }
#[derive(Debug, Clone, Copy, PartialEq)]
enum StickyMode {
None,
KeepAll,
ReplaceAll,
}
enum ConflictAction {
Keep,
Replace,
}
#[derive(Debug)] #[derive(Debug)]
struct ApplyReport { struct ApplyReport {
new_count: usize, new_count: usize,
@@ -1937,7 +1930,12 @@ fn apply_plan(
record_written_file(store, bundle, planned, FileAction::Replaced)?; record_written_file(store, bundle, planned, FileAction::Replaced)?;
report.refreshed_count += 1; report.refreshed_count += 1;
} }
PlannedKind::Conflict => match resolve_conflict(planned, &mut sticky)? { PlannedKind::Conflict => match resolve_conflict(
&planned.dst,
planned.top_category.label(),
&mut sticky,
NonInteractive::Bail,
)? {
ConflictAction::Keep => report.kept_count += 1, ConflictAction::Keep => report.kept_count += 1,
ConflictAction::Replace => { ConflictAction::Replace => {
write_file(&planned.src, &planned.dst)?; write_file(&planned.src, &planned.dst)?;
@@ -2037,50 +2035,6 @@ fn record_mcp_merge(store: &mut BundleStore, bundle: &str, report: &McpMergeRepo
store.record_mcp_servers(bundle, entries) store.record_mcp_servers(bundle, entries)
} }
fn resolve_conflict(planned: &PlannedFile, sticky: &mut StickyMode) -> Result<ConflictAction> {
match *sticky {
StickyMode::KeepAll => return Ok(ConflictAction::Keep),
StickyMode::ReplaceAll => return Ok(ConflictAction::Replace),
StickyMode::None => {}
}
if !*IS_STDOUT_TERMINAL {
bail!(
"Refusing to overwrite local file {} non-interactively. \
Re-run in a terminal, with --install-force (installs), \
or with --yes (updates).",
planned.dst.display()
);
}
let prompt = format!(
"Conflict at {} (category: {})",
planned.dst.display(),
planned.top_category.label()
);
let choice = Select::new(
&prompt,
vec!["keep", "replace", "keep-all", "replace-all", "abort"],
)
.prompt()
.with_context(|| "failed to read conflict choice")?;
match choice {
"keep" => Ok(ConflictAction::Keep),
"replace" => Ok(ConflictAction::Replace),
"keep-all" => {
*sticky = StickyMode::KeepAll;
Ok(ConflictAction::Keep)
}
"replace-all" => {
*sticky = StickyMode::ReplaceAll;
Ok(ConflictAction::Replace)
}
"abort" => bail!("Install aborted by user at conflict resolution."),
_ => unreachable!("inquire::Select returned an unexpected option"),
}
}
fn write_file(src: &Path, dst: &Path) -> Result<()> { fn write_file(src: &Path, dst: &Path) -> Result<()> {
if let Some(parent) = dst.parent() { if let Some(parent) = dst.parent() {
fs::create_dir_all(parent) fs::create_dir_all(parent)
@@ -2092,26 +2046,6 @@ fn write_file(src: &Path, dst: &Path) -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(unix)]
fn set_executable_bit_if_script(path: &Path) -> Result<()> {
use std::os::unix::fs::PermissionsExt;
let Some(ext) = path.extension().and_then(OsStr::to_str) else {
return Ok(());
};
if Language::from_extension(ext) == Language::Unsupported {
return Ok(());
}
fs::set_permissions(path, fs::Permissions::from_mode(0o755))
.with_context(|| format!("chmod {}", path.display()))?;
Ok(())
}
#[cfg(not(unix))]
fn set_executable_bit_if_script(_path: &Path) -> Result<()> {
Ok(())
}
#[derive(Debug)] #[derive(Debug)]
struct McpMergeReport { struct McpMergeReport {
added: Vec<String>, added: Vec<String>,
@@ -2924,6 +2858,11 @@ mod tests {
file("x/config.yaml", TopCategory::Agents), file("x/config.yaml", TopCategory::Agents),
file("reviewer.md", TopCategory::Roles), file("reviewer.md", TopCategory::Roles),
file("hello.yaml", TopCategory::Macros), file("hello.yaml", TopCategory::Macros),
file("x/hooks/nested/deep.sh", TopCategory::Agents),
file("x/sub/hooks/pre.sh", TopCategory::Agents),
file("sub/hooks/extra.sh", TopCategory::Roles),
file("my-skill/hooks/tool.sh", TopCategory::Skills),
file("my-macro/hooks/tool.sh", TopCategory::Macros),
], ],
mcp_json: None, mcp_json: None,
}; };
+14 -50
View File
@@ -83,12 +83,17 @@ pub async fn macro_execute(
.bootstrap_tools(app.as_ref(), true, abort_signal.clone()) .bootstrap_tools(app.as_ref(), true, abort_signal.clone())
.await?; .await?;
let result = async {
for step in &macro_value.steps { for step in &macro_value.steps {
let command = Macro::interpolate_command(step, &variables); let command = Macro::interpolate_command(step, &variables);
println!(">> {}", multiline_text(&command)); println!(">> {}", multiline_text(&command));
run_repl_command(&mut macro_ctx, abort_signal.clone(), &command).await?; run_repl_command(&mut macro_ctx, abort_signal.clone(), &command).await?;
} }
Ok(()) Ok(())
}
.await;
macro_ctx.top_level_agent_finished(result.as_ref().err(), Some(&abort_signal));
result
} }
struct MacroModeGuard<'a> { struct MacroModeGuard<'a> {
@@ -167,7 +172,8 @@ impl Macro {
debug!("Processing macro file: {}", file.as_ref()); debug!("Processing macro file: {}", file.as_ref());
let embedded_file = MacroAssets::get(&file) let embedded_file = MacroAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded macro file: {}", file.as_ref()))?; .ok_or_else(|| anyhow!("Failed to load embedded macro file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) }; let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled macro asset is not valid UTF-8");
let file_path = paths::macros_dir().join(file.as_ref()); let file_path = paths::macros_dir().join(file.as_ref());
if file_path.exists() && !force { if file_path.exists() && !force {
@@ -350,56 +356,14 @@ fn default_true() -> bool {
mod tests { mod tests {
use super::*; use super::*;
use crate::config::{AppState, Session, WorkingMode}; use crate::config::{AppState, Session, WorkingMode};
use crate::testing::TestConfigDirGuard;
use crate::utils::{create_abort_signal, get_env_name}; use crate::utils::{create_abort_signal, get_env_name};
use serial_test::serial; use serial_test::serial;
use std::fs::{create_dir_all, remove_dir_all, write}; use std::fs::{create_dir_all, remove_dir_all, write};
use std::future::Future; use std::future::Future;
use std::path::PathBuf;
use std::time::{SystemTime, UNIX_EPOCH}; use std::time::{SystemTime, UNIX_EPOCH};
use std::{env, str}; use std::{env, str};
struct TestConfigDirGuard {
key: String,
previous: Option<std::ffi::OsString>,
path: PathBuf,
}
impl TestConfigDirGuard {
fn new() -> Self {
let key = get_env_name("config_dir");
let previous = env::var_os(&key);
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = env::temp_dir().join(format!("coyote-macros-tests-{unique}"));
create_dir_all(&path).unwrap();
unsafe {
env::set_var(&key, &path);
}
Self {
key,
previous,
path,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
if let Some(previous) = &self.previous {
unsafe {
env::set_var(&self.key, previous);
}
} else {
unsafe {
env::remove_var(&self.key);
}
}
let _ = remove_dir_all(&self.path);
}
}
fn test_ctx() -> RequestContext { fn test_ctx() -> RequestContext {
RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd) RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd)
} }
@@ -974,7 +938,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn non_isolated_steps_run_on_live_ctx_and_mutations_persist() { fn non_isolated_steps_run_on_live_ctx_and_mutations_persist() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file( write_macro_file(
"live-macro", "live-macro",
"isolated: false\nsteps:\n - \".set temperature 0.42\"\n", "isolated: false\nsteps:\n - \".set temperature 0.42\"\n",
@@ -1006,7 +970,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn non_isolated_step_failure_aborts_and_restores_flag_and_mode() { fn non_isolated_step_failure_aborts_and_restores_flag_and_mode() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file( write_macro_file(
"fail-macro", "fail-macro",
"isolated: false\nsteps:\n - \".set temperature 0.9\"\n - \".update\"\n - \".set temperature 0.1\"\n", "isolated: false\nsteps:\n - \".set temperature 0.9\"\n - \".update\"\n - \".set temperature 0.1\"\n",
@@ -1057,7 +1021,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn non_isolated_macro_step_invoking_macro_is_rejected() { fn non_isolated_macro_step_invoking_macro_is_rejected() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file( write_macro_file(
"outer-macro", "outer-macro",
"isolated: false\nsteps:\n - \".inner-macro\"\n", "isolated: false\nsteps:\n - \".inner-macro\"\n",
@@ -1093,7 +1057,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn isolated_macro_step_runs_non_isolated_macro_inline_on_fork() { fn isolated_macro_step_runs_non_isolated_macro_inline_on_fork() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file("iso-outer-macro", "steps:\n - \".inner-macro\"\n"); write_macro_file("iso-outer-macro", "steps:\n - \".inner-macro\"\n");
write_macro_file( write_macro_file(
"inner-macro", "inner-macro",
@@ -1122,7 +1086,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn isolated_macro_still_forks_and_leaves_live_ctx_untouched() { fn isolated_macro_still_forks_and_leaves_live_ctx_untouched() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file("iso-macro", "steps:\n - \".set temperature 0.77\"\n"); write_macro_file("iso-macro", "steps:\n - \".set temperature 0.77\"\n");
let mut ctx = test_ctx(); let mut ctx = test_ctx();
ctx.session = Some(Session::default()); ctx.session = Some(Session::default());
@@ -1152,7 +1116,7 @@ variables:
#[test] #[test]
#[serial] #[serial]
fn guard_restores_prior_flag_values_after_inline_run() { fn guard_restores_prior_flag_values_after_inline_run() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file( write_macro_file(
"inner-macro", "inner-macro",
"isolated: false\nsteps:\n - \".set temperature 0.11\"\n", "isolated: false\nsteps:\n - \".set temperature 0.11\"\n",
+247 -28
View File
@@ -1,5 +1,6 @@
use crate::hooks::McpServerHooks;
use crate::mcp::{ use crate::mcp::{
ConnectedServer, JsonField, McpAuthRequired, McpServer, McpTransportType, ConnectedServer, HttpAuth, JsonField, McpAuthRequired, McpServer, McpTransportType,
is_auth_required_error, resolve_http_auth, spawn_mcp_server, is_auth_required_error, resolve_http_auth, spawn_mcp_server,
}; };
@@ -80,11 +81,6 @@ pub struct McpFactory {
} }
impl McpFactory { impl McpFactory {
pub fn try_get_active(&self, key: &McpServerKey) -> Option<Arc<ConnectedServer>> {
let map = self.active.lock();
map.get(key).and_then(|weak| weak.upgrade())
}
pub fn insert_active(&self, key: McpServerKey, handle: &Arc<ConnectedServer>) { pub fn insert_active(&self, key: McpServerKey, handle: &Arc<ConnectedServer>) {
let mut map = self.active.lock(); let mut map = self.active.lock();
map.insert(key, Arc::downgrade(handle)); map.insert(key, Arc::downgrade(handle));
@@ -95,15 +91,47 @@ impl McpFactory {
name: &str, name: &str,
spec: &McpServer, spec: &McpServer,
log_path: Option<&Path>, log_path: Option<&Path>,
hooks: &McpServerHooks,
) -> Result<Arc<ConnectedServer>> { ) -> Result<Arc<ConnectedServer>> {
let key = McpServerKey::from_spec(name, spec); let key = McpServerKey::from_spec(name, spec);
if let Some(existing) = self.try_get_active(&key) { let reconnect = {
let map = self.active.lock();
if let Some(existing) = map.get(&key).and_then(|weak| weak.upgrade()) {
return Ok(existing); return Ok(existing);
} }
map.contains_key(&key)
};
let transport = transport_label(&spec.transport_type);
let handle = match spawn_server(name, spec, log_path).await {
Ok(handle) => handle,
Err(e) => {
hooks.fire_failed(name, transport, &e, is_auth_required_error(&e));
return Err(e);
}
};
self.insert_active(key, &handle);
hooks.fire_connected(name, transport, reconnect);
Ok(handle)
}
}
fn transport_label(transport: &McpTransportType) -> &'static str {
match transport {
McpTransportType::Stdio => "stdio",
McpTransportType::Http => "http",
McpTransportType::Sse => "sse",
}
}
async fn spawn_server(
name: &str,
spec: &McpServer,
log_path: Option<&Path>,
) -> Result<Arc<ConnectedServer>> {
let (auth, auth_reason) = resolve_http_auth(name, spec).await; let (auth, auth_reason) = resolve_http_auth(name, spec).await;
let handle = spawn_mcp_server(spec, log_path, auth).await.map_err(|e| { spawn_transport(spec, log_path, auth).await.map_err(|e| {
if is_auth_required_error(&e) { if is_auth_required_error(&e) {
e.context(McpAuthRequired { e.context(McpAuthRequired {
server: name.to_string(), server: name.to_string(),
@@ -112,17 +140,39 @@ impl McpFactory {
} else { } else {
e e
} }
})?; })
self.insert_active(key, &handle);
Ok(handle)
}
} }
async fn spawn_transport(
spec: &McpServer,
log_path: Option<&Path>,
auth: HttpAuth,
) -> Result<Arc<ConnectedServer>> {
#[cfg(test)]
if let Some(result) = STUB_SPAWNS.lock().pop() {
return result;
}
spawn_mcp_server(spec, log_path, auth).await
}
/// Injected spawn outcomes, popped instead of spawning a real transport:
/// nothing in a test environment speaks MCP over stdio or HTTP, so the
/// connect path is exercised with fixture handles. Empty means spawn for
/// real.
#[cfg(test)]
static STUB_SPAWNS: Mutex<Vec<Result<Arc<ConnectedServer>>>> = Mutex::new(Vec::new());
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::config::test_fixtures::FixtureServer;
use crate::config::{AppConfig, AppState, RequestContext, WorkingMode};
use crate::hooks::{HookDef, HooksMap, test_sink};
use crate::mcp::{JsonField, McpServer, McpTransportType}; use crate::mcp::{JsonField, McpServer, McpTransportType};
use anyhow::anyhow;
use indexmap::IndexMap; use indexmap::IndexMap;
use rmcp::ServiceExt;
use serial_test::serial;
use std::collections::HashMap; use std::collections::HashMap;
fn stdio_spec( fn stdio_spec(
@@ -299,26 +349,195 @@ mod tests {
} }
} }
#[test]
fn factory_try_get_active_returns_none_when_empty() {
let factory = McpFactory::default();
let spec = stdio_spec("cmd", None, None);
let key = McpServerKey::from_spec("s", &spec);
assert!(factory.try_get_active(&key).is_none());
}
#[test]
fn factory_try_get_active_returns_none_for_unknown_key() {
let factory = McpFactory::default();
let spec = stdio_spec("cmd", None, None);
let key = McpServerKey::from_spec("s", &spec);
assert!(factory.try_get_active(&key).is_none());
}
#[test] #[test]
fn factory_default_has_empty_active_map() { fn factory_default_has_empty_active_map() {
let factory = McpFactory::default(); let factory = McpFactory::default();
let map = factory.active.lock(); let map = factory.active.lock();
assert!(map.is_empty()); assert!(map.is_empty());
} }
/// Clears injected spawn outcomes on drop so a panicking test cannot
/// leak stubs into later tests.
struct StubSpawnsGuard;
impl StubSpawnsGuard {
fn push(result: Result<Arc<ConnectedServer>>) -> Self {
STUB_SPAWNS.lock().push(result);
StubSpawnsGuard
}
}
impl Drop for StubSpawnsGuard {
fn drop(&mut self) {
STUB_SPAWNS.lock().clear();
}
}
/// An in-process client/server pair; the returned server half must stay
/// alive for the client handle to keep working.
async fn fixture_handle() -> (Arc<ConnectedServer>, impl Sized) {
let (client_io, server_io) = tokio::io::duplex(4096);
let (server, client) = tokio::join!(
FixtureServer::default().serve(server_io),
().serve(client_io)
);
(Arc::new(client.unwrap()), server.unwrap())
}
fn ctx_with_mcp_hooks(marker: &str) -> RequestContext {
let mut hooks_map = HooksMap::default();
for event in ["mcp.server.connected", "mcp.server.failed"] {
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::Cmd)
}
fn mcp_captures(marker: &str) -> Vec<test_sink::Capture> {
test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect()
}
#[tokio::test]
#[serial]
async fn acquire_fires_connected_only_on_real_spawns_and_marks_reconnects() {
let _sink = test_sink::install();
let marker = "mcp-spawn-k7d";
let ctx = ctx_with_mcp_hooks(marker);
let hooks = McpServerHooks::resolve(&ctx);
let factory = McpFactory::default();
let spec = stdio_spec("fixture-server-cmd", None, None);
let (first, _first_server) = fixture_handle().await;
let stub = StubSpawnsGuard::push(Ok(first.clone()));
let connected = factory.acquire("srv", &spec, None, &hooks).await.unwrap();
drop(stub);
assert!(Arc::ptr_eq(&connected, &first));
let captures = mcp_captures(marker);
assert_eq!(captures.len(), 1, "{captures:?}");
let capture = &captures[0];
assert_eq!(capture.hook_name, format!("{marker}-mcp.server.connected"));
assert_eq!(
capture.envs.get("COYOTE_MCP_SERVER").map(String::as_str),
Some("srv")
);
assert_eq!(
capture.envs.get("COYOTE_MCP_TRANSPORT").map(String::as_str),
Some("stdio")
);
assert!(
!capture.envs.contains_key("COYOTE_MCP_RECONNECT"),
"a first connect must not be marked as a reconnect"
);
let reused = factory.acquire("srv", &spec, None, &hooks).await.unwrap();
assert!(Arc::ptr_eq(&reused, &first));
assert_eq!(
mcp_captures(marker).len(),
1,
"reusing a live server must fire nothing"
);
// Every holder dropped: the weak in the factory dies, and the next
// acquire is a real spawn again — now marked as a reconnect.
drop(connected);
drop(reused);
drop(first);
let (second, _second_server) = fixture_handle().await;
let _stub = StubSpawnsGuard::push(Ok(second.clone()));
let respawned = factory.acquire("srv", &spec, None, &hooks).await.unwrap();
assert!(Arc::ptr_eq(&respawned, &second));
let captures = mcp_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(
captures[1]
.envs
.get("COYOTE_MCP_RECONNECT")
.map(String::as_str),
Some("true")
);
}
#[tokio::test]
#[serial]
async fn acquire_spawn_failure_fires_failed_with_the_error() {
let _sink = test_sink::install();
let marker = "mcp-fail-v2q";
let ctx = ctx_with_mcp_hooks(marker);
let hooks = McpServerHooks::resolve(&ctx);
let factory = McpFactory::default();
// No stub: the nonexistent binary drives the real spawn-error path.
let spec = stdio_spec("coyote-nonexistent-mcp-binary-v2q", None, None);
let result = factory.acquire("bad-srv", &spec, None, &hooks).await;
assert!(result.is_err());
let captures = mcp_captures(marker);
assert_eq!(captures.len(), 1, "{captures:?}");
let capture = &captures[0];
assert_eq!(capture.hook_name, format!("{marker}-mcp.server.failed"));
assert_eq!(
capture.envs.get("COYOTE_MCP_SERVER").map(String::as_str),
Some("bad-srv")
);
assert_eq!(
capture.envs.get("COYOTE_MCP_TRANSPORT").map(String::as_str),
Some("stdio")
);
assert!(
capture
.envs
.get("COYOTE_ERROR")
.is_some_and(|error| !error.is_empty())
);
assert!(
!capture.envs.contains_key("COYOTE_MCP_AUTH_REQUIRED"),
"a plain spawn failure must not be flagged as auth-required"
);
}
#[tokio::test]
#[serial]
async fn acquire_auth_failure_sets_the_auth_required_flag() {
let _sink = test_sink::install();
let marker = "mcp-auth-h4n";
let ctx = ctx_with_mcp_hooks(marker);
let hooks = McpServerHooks::resolve(&ctx);
let factory = McpFactory::default();
let spec = stdio_spec("oauth-cmd", None, None);
let _stub = StubSpawnsGuard::push(Err(anyhow!("Auth required: no stored token")));
let err = factory
.acquire("oauth-srv", &spec, None, &hooks)
.await
.unwrap_err();
assert!(
err.downcast_ref::<McpAuthRequired>().is_some(),
"the auth error must be wrapped in McpAuthRequired context"
);
let captures = mcp_captures(marker);
assert_eq!(captures.len(), 1, "{captures:?}");
let capture = &captures[0];
assert_eq!(capture.hook_name, format!("{marker}-mcp.server.failed"));
assert_eq!(
capture
.envs
.get("COYOTE_MCP_AUTH_REQUIRED")
.map(String::as_str),
Some("true")
);
}
} }
+238 -20
View File
@@ -3,6 +3,7 @@ mod app_config;
mod app_state; mod app_state;
pub(crate) mod builtin_manifest; pub(crate) mod builtin_manifest;
mod bundles; mod bundles;
pub(crate) mod conflict;
mod input; mod input;
mod install_remote; mod install_remote;
pub(crate) mod instructions; pub(crate) mod instructions;
@@ -37,6 +38,7 @@ pub use self::app_config::AppConfig;
pub use self::app_state::AppState; pub use self::app_state::AppState;
pub(crate) use self::bundles::installed_bundle_names; pub(crate) use self::bundles::installed_bundle_names;
pub use self::bundles::list_installed_bundles; pub use self::bundles::list_installed_bundles;
use self::conflict::{InstallMode, StickyMode};
pub use self::input::Input; pub use self::input::Input;
pub use self::install_remote::{ pub use self::install_remote::{
DEFAULT_GIT_HOST, install_or_update, install_or_update_from_repl_args, uninstall_bundle, DEFAULT_GIT_HOST, install_or_update, install_or_update_from_repl_args, uninstall_bundle,
@@ -417,12 +419,13 @@ impl Default for Config {
} }
pub fn install_builtins() -> Result<()> { pub fn install_builtins() -> Result<()> {
hooks::sweep_stale_payload_files();
Functions::install_builtin_global_tools(false)?; Functions::install_builtin_global_tools(false)?;
Agent::install_builtin_agents(false)?; Agent::install_builtin_agents(InstallMode::Skip)?;
Macro::install_macros(false)?; Macro::install_macros(false)?;
Skill::install_builtin_skills(false)?; Skill::install_builtin_skills(false)?;
hooks::install_builtin_hooks(false)?; hooks::install_builtin_hooks(InstallMode::Skip, &mut StickyMode::None)?;
Role::install_builtin_role_hooks(false)?; Role::install_builtin_role_hooks(InstallMode::Skip, &mut StickyMode::None)?;
Ok(()) Ok(())
} }
@@ -519,11 +522,16 @@ pub fn install_assets(category: AssetCategory) -> Result<()> {
} }
match category { match category {
AssetCategory::Agents => Agent::install_builtin_agents(true)?, AssetCategory::Agents => Agent::install_builtin_agents(InstallMode::Force)?,
AssetCategory::Macros => Macro::install_macros(true)?, AssetCategory::Macros => Macro::install_macros(true)?,
AssetCategory::Functions => Functions::install_builtin_global_tools(true)?, AssetCategory::Functions => Functions::install_builtin_global_tools(true)?,
AssetCategory::Skills => Skill::install_builtin_skills(true)?, AssetCategory::Skills => Skill::install_builtin_skills(true)?,
AssetCategory::Hooks => hooks::install_builtin_hooks(true)?, AssetCategory::Hooks => {
let mut sticky = StickyMode::None;
hooks::install_builtin_hooks(InstallMode::Prompt, &mut sticky)?;
Role::install_builtin_role_hooks(InstallMode::Prompt, &mut sticky)?;
Agent::install_builtin_agent_hooks(InstallMode::Prompt, &mut sticky)?;
}
AssetCategory::McpConfig => Functions::install_mcp_config()?, AssetCategory::McpConfig => Functions::install_mcp_config()?,
} }
@@ -544,6 +552,14 @@ fn confirm_asset_overwrite(category: AssetCategory, label: &str, target: &Path)
references) are left untouched.", references) are left untouched.",
target.display() target.display()
), ),
AssetCategory::Hooks => format!(
"Refreshing bundled hooks installs any missing bundled hook \
scripts in {}, the roles hooks directory, and each bundled \
agent's hooks directory. Where a local copy differs from the \
bundled version you are asked per file whether to keep or \
replace it; non-interactive runs keep local files.",
target.display()
),
_ => format!( _ => format!(
"Reinstalling bundled {label} overwrites every bundled {label} in \ "Reinstalling bundled {label} overwrites every bundled {label} in \
{} with this build's packaged versions. Local changes to bundled \ {} with this build's packaged versions. Local changes to bundled \
@@ -729,11 +745,17 @@ pub fn maybe_spawn_models_refresh(app: &AppConfig) {
} }
impl Config { impl Config {
pub async fn load_with_interpolation(info_flag: bool) -> Result<Self> { pub async fn load_with_interpolation(
skip_interpolation: bool,
inspection: bool,
) -> Result<Self> {
let config_path = paths::config_file(); let config_path = paths::config_file();
if env::var_os(SANDBOX_ENV_FLAG).is_some() { if env::var_os(SANDBOX_ENV_FLAG).is_some() {
if !config_path.exists() { if !config_path.exists() {
if inspection {
return Ok(Self::default());
}
create_config_file(&config_path).await?; create_config_file(&config_path).await?;
} }
@@ -748,6 +770,9 @@ impl Config {
{ {
Some(v) => (Self::load_dynamic(&v)?, String::new()), Some(v) => (Self::load_dynamic(&v)?, String::new()),
None => { None => {
if inspection {
return Ok(Self::default());
}
if *IS_STDOUT_TERMINAL { if *IS_STDOUT_TERMINAL {
create_config_file(&config_path).await?; create_config_file(&config_path).await?;
} }
@@ -758,7 +783,7 @@ impl Config {
Self::load_from_file(&config_path)? Self::load_from_file(&config_path)?
}; };
if info_flag { if skip_interpolation {
return Ok(config); return Ok(config);
} }
@@ -1120,6 +1145,32 @@ pub(crate) fn ensure_parent_exists(path: &Path) -> Result<()> {
Ok(()) Ok(())
} }
/// The one exec-bit policy for every hook/agent installer: a file is
/// executable iff `Language::from_extension` recognizes its extension.
/// Users may keep non-executable support files in hooks directories, so
/// installers must write plain and then apply this predicate — never
/// blanket-0755.
#[cfg(unix)]
pub(crate) fn set_executable_bit_if_script(path: &Path) -> Result<()> {
use crate::function::Language;
use std::os::unix::fs::PermissionsExt;
let Some(ext) = path.extension().and_then(std::ffi::OsStr::to_str) else {
return Ok(());
};
if Language::from_extension(ext) == Language::Unsupported {
return Ok(());
}
fs::set_permissions(path, fs::Permissions::from_mode(0o755))
.with_context(|| format!("chmod {}", path.display()))?;
Ok(())
}
#[cfg(not(unix))]
pub(crate) fn set_executable_bit_if_script(_path: &Path) -> Result<()> {
Ok(())
}
fn read_env_value<T>(key: &str) -> Option<Option<T>> fn read_env_value<T>(key: &str) -> Option<Option<T>>
where where
T: std::str::FromStr, T: std::str::FromStr,
@@ -1593,12 +1644,12 @@ hooks:
use std::fs; use std::fs;
use std::time; use std::time;
let unique = time::SystemTime::now() let unique = SystemTime::now()
.duration_since(time::UNIX_EPOCH) .duration_since(time::UNIX_EPOCH)
.unwrap() .unwrap()
.as_nanos(); .as_nanos();
let tmp_dir = std::env::temp_dir().join(format!("coyote-sandbox-cfg-{unique}")); let tmp_dir = env::temp_dir().join(format!("coyote-sandbox-cfg-{unique}"));
fs::create_dir_all(&tmp_dir).unwrap(); create_dir_all(&tmp_dir).unwrap();
let config_path = tmp_dir.join("config.yaml"); let config_path = tmp_dir.join("config.yaml");
fs::write( fs::write(
@@ -1608,25 +1659,25 @@ hooks:
.unwrap(); .unwrap();
let config_env = get_env_name("config_file"); let config_env = get_env_name("config_file");
let prev_config = std::env::var_os(&config_env); let prev_config = env::var_os(&config_env);
let prev_sandbox = std::env::var_os(crate::sandbox::SANDBOX_ENV_FLAG); let prev_sandbox = env::var_os(SANDBOX_ENV_FLAG);
unsafe { unsafe {
std::env::set_var(&config_env, &config_path); env::set_var(&config_env, &config_path);
std::env::set_var(crate::sandbox::SANDBOX_ENV_FLAG, "1"); env::set_var(SANDBOX_ENV_FLAG, "1");
} }
let result = Config::load_with_interpolation(false).await; let result = Config::load_with_interpolation(false, false).await;
let (_, raw) = Config::load_from_file(&config_path).unwrap(); let (_, raw) = Config::load_from_file(&config_path).unwrap();
unsafe { unsafe {
match prev_config { match prev_config {
Some(v) => std::env::set_var(&config_env, v), Some(v) => env::set_var(&config_env, v),
None => std::env::remove_var(&config_env), None => env::remove_var(&config_env),
} }
match prev_sandbox { match prev_sandbox {
Some(v) => std::env::set_var(crate::sandbox::SANDBOX_ENV_FLAG, v), Some(v) => env::set_var(SANDBOX_ENV_FLAG, v),
None => std::env::remove_var(crate::sandbox::SANDBOX_ENV_FLAG), None => env::remove_var(SANDBOX_ENV_FLAG),
} }
} }
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
@@ -1640,6 +1691,60 @@ hooks:
); );
} }
#[tokio::test]
#[serial_test::serial]
async fn info_flag_with_missing_config_skips_wizard_and_writes_nothing() {
let unique = SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = env::temp_dir().join(format!("coyote-info-bypass-{unique}"));
create_dir_all(&tmp_dir).unwrap();
let _config_dir = crate::testing::EnvVarGuard::set(get_env_name("config_dir"), &tmp_dir);
let _config_file = crate::testing::EnvVarGuard::unset(get_env_name("config_file"));
let _sandbox = crate::testing::EnvVarGuard::unset(SANDBOX_ENV_FLAG);
let _provider = crate::testing::EnvVarGuard::unset(get_env_name("provider"));
let _platform = crate::testing::EnvVarGuard::unset(get_env_name("platform"));
let result = Config::load_with_interpolation(true, true).await;
let leftover: Vec<_> = read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = fs::remove_dir_all(&tmp_dir);
result.expect("inspection flags must load a default config without a config file");
assert!(
leftover.is_empty(),
"inspection flags must not write anything: {leftover:?}"
);
}
#[tokio::test]
#[serial_test::serial]
async fn sandbox_info_flag_with_missing_config_writes_nothing() {
let unique = SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = env::temp_dir().join(format!("coyote-sandbox-info-bypass-{unique}"));
create_dir_all(&tmp_dir).unwrap();
let config_path = tmp_dir.join("config.yaml");
let _config_file =
crate::testing::EnvVarGuard::set(get_env_name("config_file"), &config_path);
let _sandbox = crate::testing::EnvVarGuard::set(SANDBOX_ENV_FLAG, "1");
let result = Config::load_with_interpolation(true, true).await;
let created = config_path.exists();
let _ = fs::remove_dir_all(&tmp_dir);
result.expect("inspection flags must load a default config in sandbox mode too");
assert!(
!created,
"inspection flags must not create the sandbox config file"
);
}
#[test] #[test]
fn unloadable_override_is_stale() { fn unloadable_override_is_stale() {
assert!(models_override_is_stale(false, Some(SystemTime::now()), 24)); assert!(models_override_is_stale(false, Some(SystemTime::now()), 24));
@@ -1719,7 +1824,7 @@ hooks:
let written = write_models_override(vec![entry()]).unwrap(); let written = write_models_override(vec![entry()]).unwrap();
assert_eq!(written, path); assert_eq!(written, path);
let mut tmp_name = path.as_os_str().to_os_string(); let mut tmp_name = path.as_os_str().to_os_string();
tmp_name.push(format!(".{}.tmp", std::process::id())); tmp_name.push(format!(".{}.tmp", process::id()));
assert!(!PathBuf::from(tmp_name).exists()); assert!(!PathBuf::from(tmp_name).exists());
let loaded = paths::local_models_override().unwrap(); let loaded = paths::local_models_override().unwrap();
assert_eq!(loaded.len(), 1); assert_eq!(loaded.len(), 1);
@@ -1737,4 +1842,117 @@ hooks:
std::panic::resume_unwind(panic); std::panic::resume_unwind(panic);
} }
} }
use crate::testing::TestConfigDirGuard;
#[test]
#[serial_test::serial]
fn install_assets_hooks_shares_one_sticky_scope_across_all_three_locations() {
use crate::config::conflict::prompt_script;
let _guard = TestConfigDirGuard::new("hooks-three-locations");
let notify = paths::hooks_dir().join("notify.sh");
let role_dir = paths::roles_dir().join("hooks");
let agent_dir = paths::agents_data_dir().join("probe").join("hooks");
let shipped = vec![("hook.sh".to_string(), "shipped\n".to_string())];
// Drives the exact three installers `install_assets(Hooks)` runs,
// threading one sticky scope through all of them.
let refresh = |sticky: &mut StickyMode| {
hooks::install_builtin_hooks(InstallMode::Prompt, sticky).unwrap();
role::install_and_reconcile_role_hooks(
&role_dir,
&shipped,
InstallMode::Prompt,
sticky,
)
.unwrap();
agent::install_and_reconcile_agent_hooks(
"probe",
&shipped,
InstallMode::Prompt,
sticky,
)
.unwrap();
};
let modify_all = || {
fs::write(&notify, "local global").unwrap();
fs::write(role_dir.join("hook.sh"), "local role").unwrap();
fs::write(agent_dir.join("hook.sh"), "local agent").unwrap();
};
let script = prompt_script::install(&[]);
refresh(&mut StickyMode::None);
assert_eq!(
prompt_script::prompts_asked(),
0,
"a clean install asks nothing"
);
drop(script);
// One replace-all at the first (global) conflict must silently
// replace the role and agent conflicts too.
modify_all();
let script = prompt_script::install(&["replace-all"]);
let mut sticky = StickyMode::None;
refresh(&mut sticky);
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(sticky, StickyMode::ReplaceAll);
assert!(
read_to_string(&notify)
.unwrap()
.starts_with("#!/usr/bin/env bash")
);
assert_eq!(
read_to_string(role_dir.join("hook.sh")).unwrap(),
"shipped\n"
);
assert_eq!(
read_to_string(agent_dir.join("hook.sh")).unwrap(),
"shipped\n"
);
drop(script);
// And one keep-all must keep every location's local edit.
modify_all();
let script = prompt_script::install(&["keep-all"]);
let mut sticky = StickyMode::None;
refresh(&mut sticky);
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(sticky, StickyMode::KeepAll);
assert_eq!(read_to_string(&notify).unwrap(), "local global");
assert_eq!(
read_to_string(role_dir.join("hook.sh")).unwrap(),
"local role"
);
assert_eq!(
read_to_string(agent_dir.join("hook.sh")).unwrap(),
"local agent"
);
drop(script);
}
#[test]
#[serial_test::serial]
fn install_builtins_never_prompts_and_keeps_local_hook_edits() {
use crate::config::conflict::prompt_script;
let _guard = TestConfigDirGuard::new("install-builtins-startup");
install_builtins().unwrap();
let notify = paths::hooks_dir().join("notify.sh");
fs::write(&notify, "# local edit").unwrap();
// A forced terminal with no scripted answers: any prompt would panic
// and the counter would move.
let script = prompt_script::install(&[]);
install_builtins().unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(read_to_string(&notify).unwrap(), "# local edit");
drop(script);
let _script = prompt_script::install_non_interactive();
install_builtins().unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(read_to_string(&notify).unwrap(), "# local edit");
}
} }
+58
View File
@@ -47,3 +47,61 @@ impl RagCache {
Ok(arc) Ok(arc)
} }
} }
#[cfg(test)]
mod tests {
use super::*;
use crate::config::AppConfig;
use crate::hooks::test_sink;
use crate::rag::RagData;
use anyhow::anyhow;
use serial_test::serial;
use std::time::{SystemTime, UNIX_EPOCH};
/// A cache hit must never reach the loader: the loader closure is where
/// a real RAG build (and its `rag.sync.*` hook dispatch) happens, so
/// returning the cached entry is what keeps warm-cache lookups silent.
#[tokio::test]
#[serial]
async fn cache_hit_returns_the_cached_rag_without_running_the_loader() {
let _sink = test_sink::install();
test_sink::drain();
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let dir = std::env::temp_dir().join(format!("coyote-rag-cache-hit-{unique}"));
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("kb.yaml");
let data = RagData {
driver: "yaml".to_string(),
// The only embedding model resolvable in tests; see
// `seed_model_registries` in the request_context tests.
embedding_model: "test-seeded:test-embedder".to_string(),
chunk_size: 1000,
chunk_overlap: 100,
top_k: 5,
..Default::default()
};
std::fs::write(&path, serde_yaml::to_string(&data).unwrap()).unwrap();
let rag = Arc::new(Rag::load(&AppConfig::default(), "kb", &path).await.unwrap());
let _ = std::fs::remove_dir_all(&dir);
let cache = RagCache::default();
let key = RagKey::Named("kb".to_string());
cache.insert(key.clone(), &rag);
let loaded = cache
.load_with(key, || async {
Err(anyhow!("a cache hit must not rebuild"))
})
.await
.unwrap();
assert!(Arc::ptr_eq(&loaded, &rag));
assert!(
test_sink::drain().is_empty(),
"a cache hit must dispatch no hooks"
);
}
}
+358 -49
View File
@@ -54,6 +54,7 @@ use crate::hooks::{self, HookEvent};
use anyhow::{Context, Error, Result, bail}; use anyhow::{Context, Error, Result, bail};
use colored::Colorize; use colored::Colorize;
use gman::providers::SupportedProvider; use gman::providers::SupportedProvider;
use hooks::{McpServerHooks, RagSyncHooks};
use indexmap::IndexMap; use indexmap::IndexMap;
use indoc::formatdoc; use indoc::formatdoc;
use inquire::{Confirm, MultiSelect, Text, list_option::ListOption, validator::Validation}; use inquire::{Confirm, MultiSelect, Text, list_option::ListOption, validator::Validation};
@@ -417,7 +418,11 @@ impl RequestContext {
working_mode: WorkingMode, working_mode: WorkingMode,
info_flag: bool, info_flag: bool,
) -> Result<Self> { ) -> Result<Self> {
let model = Model::retrieve_model(&app.config, &app.config.model_id, ModelType::Chat)?; let model = if info_flag && app.config.model_id.is_empty() {
Model::default()
} else {
Model::retrieve_model(&app.config, &app.config.model_id, ModelType::Chat)?
};
let mut functions = app.functions.clone(); let mut functions = app.functions.clone();
if working_mode.is_repl() { if working_mode.is_repl() {
@@ -1073,7 +1078,12 @@ impl RequestContext {
// COYOTE_SESSION_ID and session-held role hooks still resolve; no // COYOTE_SESSION_ID and session-held role hooks still resolve; no
// session means no event. // session means no event.
if self.session.is_some() { if self.session.is_some() {
hooks::fire(HookEvent::SessionEnded, self, &[], None); hooks::fire(
HookEvent::SessionEnded,
self,
&hooks::role_extras(self),
None,
);
} }
if let Some(mut session) = self.session.take() { if let Some(mut session) = self.session.take() {
let sessions_dir = self.sessions_dir(); let sessions_dir = self.sessions_dir();
@@ -4402,6 +4412,7 @@ impl RequestContext {
if !server_ids.is_empty() { if !server_ids.is_empty() {
let app_ref = &self.app; let app_ref = &self.app;
let mcp_hooks = McpServerHooks::resolve(self);
let acquire_all = async { let acquire_all = async {
let mut handles = Vec::new(); let mut handles = Vec::new();
let mut auth_required = Vec::new(); let mut auth_required = Vec::new();
@@ -4409,7 +4420,7 @@ impl RequestContext {
if let Some(spec) = mcp_config.mcp_servers.get(id) { if let Some(spec) = mcp_config.mcp_servers.get(id) {
match app_ref match app_ref
.mcp_factory .mcp_factory
.acquire(id, spec, app_ref.mcp_log_path.as_deref()) .acquire(id, spec, app_ref.mcp_log_path.as_deref(), &mcp_hooks)
.await .await
{ {
Ok(handle) => handles.push((id.clone(), handle)), Ok(handle) => handles.push((id.clone(), handle)),
@@ -4734,7 +4745,19 @@ impl RequestContext {
self.refresh_mcp_tool_filters(); self.refresh_mcp_tool_filters();
self.init_agent_session_variables(new_session)?; self.init_agent_session_variables(new_session)?;
if created_new_session { if created_new_session {
hooks::fire(HookEvent::SessionStarted, self, &[], None); hooks::fire(
HookEvent::SessionStarted,
self,
&hooks::role_extras(self),
None,
);
} else {
hooks::fire(
HookEvent::SessionResumed,
self,
&hooks::role_extras(self),
None,
);
} }
Ok(()) Ok(())
} }
@@ -4903,7 +4926,10 @@ impl RequestContext {
// Interactive switch-away closes the top-level agent bracket while // Interactive switch-away closes the top-level agent bracket while
// the agent is still attached, so agent-scoped hooks resolve. // the agent is still attached, so agent-scoped hooks resolve.
self.top_level_agent_finished(None); // No abort signal on purpose: switching away is a deliberate exit,
// never an interruption, even though cancel_recursive tears the
// agent's children down just below.
self.top_level_agent_finished(None, None);
if self.agent.take().is_some() { if self.agent.take().is_some() {
if let Some(supervisor) = self.supervisor.clone() { if let Some(supervisor) = self.supervisor.clone() {
supervisor.read().cancel_recursive(); supervisor.read().cancel_recursive();
@@ -4951,11 +4977,25 @@ impl RequestContext {
); );
} }
pub fn top_level_agent_finished(&mut self, error: Option<&Error>) { pub fn top_level_agent_finished(
&mut self,
error: Option<&Error>,
abort_signal: Option<&AbortSignal>,
) {
let Some(id) = self.top_level_agent_id.take() else { let Some(id) = self.top_level_agent_id.take() else {
return; return;
}; };
if abort_signal.is_some_and(|signal| signal.aborted_ctrlc()) {
hooks::fire(
HookEvent::AgentInterrupted,
self,
&[("COYOTE_AGENT_ID", id)],
None,
);
return;
}
match error { match error {
None => hooks::fire( None => hooks::fire(
HookEvent::AgentCompleted, HookEvent::AgentCompleted,
@@ -5311,6 +5351,7 @@ impl RequestContext {
let vault = self.app.vault.clone(); let vault = self.app.vault.clone();
let rag_cache = self.rag_cache(); let rag_cache = self.rag_cache();
let working_mode = self.working_mode; let working_mode = self.working_mode;
let sync_hooks = RagSyncHooks::resolve(self);
let (rag, rag_key): (Arc<Rag>, Option<RagKey>) = match rag { let (rag, rag_key): (Arc<Rag>, Option<RagKey>) = match rag {
None => { None => {
@@ -5329,6 +5370,7 @@ impl RequestContext {
&[], &[],
abort_signal.clone(), abort_signal.clone(),
false, false,
sync_hooks,
) )
.await?, .await?,
), ),
@@ -5350,7 +5392,15 @@ impl RequestContext {
if working_mode.is_cmd() { if working_mode.is_cmd() {
bail!("Unknown RAG '{name}'"); bail!("Unknown RAG '{name}'");
} }
Rag::init(&app, name, &rag_path, &[], abort_signal.clone(), true) Rag::init(
&app,
name,
&rag_path,
&[],
abort_signal.clone(),
true,
sync_hooks,
)
.await .await
} else { } else {
Rag::load_async(&app, &vault, name, &rag_path).await Rag::load_async(&app, &vault, name, &rag_path).await
@@ -5435,6 +5485,7 @@ impl RequestContext {
false, false,
&self.app.config, &self.app.config,
abort_signal, abort_signal,
RagSyncHooks::resolve(self),
) )
.await?; .await?;
self.rag = Some(Arc::new(rag)); self.rag = Some(Arc::new(rag));
@@ -5465,7 +5516,14 @@ impl RequestContext {
This will call the embedding API and may take a while.", This will call the embedding API and may take a while.",
rag.file_count() rag.file_count()
); );
rag.refresh_document_paths(&document_paths, true, true, &self.app.config, abort_signal) rag.refresh_document_paths(
&document_paths,
true,
true,
&self.app.config,
abort_signal,
RagSyncHooks::resolve(self),
)
.await?; .await?;
self.rag = Some(Arc::new(rag)); self.rag = Some(Arc::new(rag));
Ok(()) Ok(())
@@ -5491,6 +5549,7 @@ mod tests {
use crate::config::AppState; use crate::config::AppState;
use crate::config::agent::AgentConfig; use crate::config::agent::AgentConfig;
use crate::config::bundles::BundleStore; use crate::config::bundles::BundleStore;
use crate::config::conflict::InstallMode;
use crate::config::mcp_tool_policy::LayerSource; use crate::config::mcp_tool_policy::LayerSource;
use crate::config::tool_scope::test_fixtures::{FixtureServer, fixture_runtime}; use crate::config::tool_scope::test_fixtures::{FixtureServer, fixture_runtime};
use crate::function::jobs::RingBuf; use crate::function::jobs::RingBuf;
@@ -8333,7 +8392,12 @@ mod tests {
let agent_dir = paths::agent_data_dir(agent_name); let agent_dir = paths::agent_data_dir(agent_name);
create_dir_all(&agent_dir).unwrap(); create_dir_all(&agent_dir).unwrap();
let mut config = format!("name: {agent_name}\ninstructions: hi\nhooks:\n"); let mut config = format!("name: {agent_name}\ninstructions: hi\nhooks:\n");
for event in ["agent.started", "agent.completed", "agent.failed"] { for event in [
"agent.started",
"agent.completed",
"agent.interrupted",
"agent.failed",
] {
config.push_str(&format!( config.push_str(&format!(
" {event}:\n - name: {marker}-{event}\n command: 'true'\n" " {event}:\n - name: {marker}-{event}\n command: 'true'\n"
)); ));
@@ -8353,7 +8417,7 @@ mod tests {
#[test] #[test]
#[serial] #[serial]
fn use_session_fires_session_started_for_new_sessions_only() { fn use_session_discriminates_session_started_from_session_resumed() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
let sessions_dir = paths::local_dir("sessions"); let sessions_dir = paths::local_dir("sessions");
create_dir_all(&sessions_dir).unwrap(); create_dir_all(&sessions_dir).unwrap();
@@ -8361,13 +8425,17 @@ mod tests {
let marker = "sess-started-q8n"; let marker = "sess-started-q8n";
let mut ctx = create_test_ctx(); let mut ctx = create_test_ctx();
ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.started"], marker)); ctx.update_app_config(|app| {
app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker)
});
ctx.role = Some(Role::new("started-role", "Prompt"));
let app = ctx.app.config.clone(); let app = ctx.app.config.clone();
let abort = utils::create_abort_signal(); let abort = utils::create_abort_signal();
run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap(); run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap();
let captures = marker_captures(marker); let captures = marker_captures(marker);
assert_eq!(captures.len(), 1, "a new session fires exactly one event"); assert_eq!(captures.len(), 1, "a new session fires exactly one event");
assert_eq!(captures[0].hook_name, format!("{marker}-session.started"));
assert_eq!( assert_eq!(
captures[0] captures[0]
.envs .envs
@@ -8375,17 +8443,46 @@ mod tests {
.map(String::as_str), .map(String::as_str),
Some("hook-fresh") Some("hook-fresh")
); );
assert_eq!(
captures[0].envs.get("COYOTE_ROLE").map(String::as_str),
Some("started-role"),
"session.started must carry the active role"
);
// Resuming a persisted session fires nothing. // Resuming a persisted session fires session.resumed, never
write_paused_todo_session(&ctx, "hook-resume"); // session.started, and carries the role the session holds.
let session_path = ctx.session_file("hook-resume");
ensure_parent_exists(&session_path).unwrap();
write(
&session_path,
"model: test-seeded:test-chat\nrole_name: resumed-role\nmessages: []\n",
)
.unwrap();
let mut resumed_ctx = create_test_ctx(); let mut resumed_ctx = create_test_ctx();
resumed_ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.started"], marker)); resumed_ctx.update_app_config(|app| {
app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker)
});
let resumed_app = resumed_ctx.app.config.clone(); let resumed_app = resumed_ctx.app.config.clone();
run_async(resumed_ctx.use_session(&resumed_app, Some("hook-resume"), abort)).unwrap(); run_async(resumed_ctx.use_session(&resumed_app, Some("hook-resume"), abort)).unwrap();
let captures = marker_captures(marker);
let started: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-session.started"))
.collect();
let resumed: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-session.resumed"))
.collect();
assert_eq!(started.len(), 1, "resume must not fire session.started");
assert_eq!(resumed.len(), 1, "resume fires exactly one session.resumed");
assert_eq!( assert_eq!(
marker_captures(marker).len(), resumed[0].envs.get("COYOTE_SESSION_ID").map(String::as_str),
1, Some("hook-resume")
"resume must not fire session.started" );
assert_eq!(
resumed[0].envs.get("COYOTE_ROLE").map(String::as_str),
Some("resumed-role"),
"session.resumed must carry the session-held role"
); );
} }
@@ -8403,14 +8500,22 @@ mod tests {
ctx.exit_session().unwrap(); ctx.exit_session().unwrap();
assert!(marker_captures(marker).is_empty()); assert!(marker_captures(marker).is_empty());
ctx.session = Some(Session::default()); let mut session = Session::default();
session.set_role(Role::new("ended-role", "Prompt"));
ctx.session = Some(session);
ctx.exit_session().unwrap(); ctx.exit_session().unwrap();
ctx.exit_session().unwrap(); ctx.exit_session().unwrap();
let captures = marker_captures(marker);
assert_eq!( assert_eq!(
marker_captures(marker).len(), captures.len(),
1, 1,
"session.ended fires exactly once, only while a session is attached" "session.ended fires exactly once, only while a session is attached"
); );
assert_eq!(
captures[0].envs.get("COYOTE_ROLE").map(String::as_str),
Some("ended-role"),
"session.ended must fire while the session-held role is still attached"
);
} }
#[test] #[test]
@@ -8462,8 +8567,8 @@ mod tests {
ctx.top_level_agent_started(); ctx.top_level_agent_started();
// Per agent RUN, never per message: re-entering while open is a no-op. // Per agent RUN, never per message: re-entering while open is a no-op.
ctx.top_level_agent_started(); ctx.top_level_agent_started();
ctx.top_level_agent_finished(None); ctx.top_level_agent_finished(None, None);
ctx.top_level_agent_finished(None); ctx.top_level_agent_finished(None, None);
let captures = marker_captures(marker); let captures = marker_captures(marker);
let started: Vec<_> = captures let started: Vec<_> = captures
@@ -8512,7 +8617,7 @@ mod tests {
ctx.top_level_agent_started(); ctx.top_level_agent_started();
let err = anyhow!("dispatch exploded"); let err = anyhow!("dispatch exploded");
ctx.top_level_agent_finished(Some(&err)); ctx.top_level_agent_finished(Some(&err), None);
let captures = marker_captures(marker); let captures = marker_captures(marker);
let failed: Vec<_> = captures let failed: Vec<_> = captures
@@ -8532,6 +8637,124 @@ mod tests {
assert!(failed[0].envs.contains_key("COYOTE_AGENT_ID")); assert!(failed[0].envs.contains_key("COYOTE_AGENT_ID"));
} }
#[test]
#[serial]
fn top_level_agent_ctrlc_fires_agent_interrupted_not_failed() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-int-r6t";
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.clone())).unwrap();
ctx.top_level_agent_started();
abort.set_ctrlc();
// The fired signal, not the error text, decides: even with an error
// in hand the bracket reports an interruption.
let err = anyhow!("Aborted.");
ctx.top_level_agent_finished(Some(&err), Some(&abort));
let captures = marker_captures(marker);
let interrupted: Vec<_> = captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.interrupted"))
.collect();
assert_eq!(interrupted.len(), 1);
assert!(interrupted[0].envs.contains_key("COYOTE_AGENT_ID"));
assert!(
!interrupted[0].envs.contains_key("COYOTE_AGENT_ERROR"),
"an interruption is not a failure and carries no error env"
);
assert!(captures.iter().all(|capture| {
capture.hook_name != format!("{marker}-agent.failed")
&& capture.hook_name != format!("{marker}-agent.completed")
}));
}
#[test]
#[serial]
fn top_level_agent_ctrlc_without_error_fires_agent_interrupted() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-int-noerr-v4d";
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.clone())).unwrap();
ctx.top_level_agent_started();
abort.set_ctrlc();
// Signal-first: a latched ctrl-c means interrupted even when the run
// surfaced no error at all.
ctx.top_level_agent_finished(None, Some(&abort));
let captures = marker_captures(marker);
assert_eq!(
captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.interrupted"))
.count(),
1
);
assert!(
captures
.iter()
.all(|capture| capture.hook_name != format!("{marker}-agent.completed")),
"a ctrl-c'd run must not report completion"
);
}
/// The REPL's advertised exit path -- ctrl-c at an idle prompt (which
/// prints "To exit, press Ctrl+D"), then ctrl-d -- is a deliberate exit:
/// with no active tasks to cancel the prompt latch stays clear, so
/// teardown fires agent.completed, never agent.interrupted.
#[test]
#[serial]
fn idle_prompt_ctrlc_then_ctrld_exit_fires_agent_completed() {
let _guard = TestConfigDirGuard::new();
let _sink = test_sink::install();
let marker = "agent-top-idle-cc-w2b";
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.clone())).unwrap();
ctx.top_level_agent_started();
// Ctrl-c at the idle prompt: nothing to cancel, so no latch.
let supervisor = Arc::new(RwLock::new(Supervisor::new(4, 3)));
crate::repl::latch_prompt_interrupt(&abort, &supervisor);
assert!(!abort.aborted_ctrlc());
// Ctrl-d exits; teardown sees only the ctrl-d flag.
abort.set_ctrld();
ctx.top_level_agent_finished(None, Some(&abort));
let captures = marker_captures(marker);
assert_eq!(
captures
.iter()
.filter(|capture| capture.hook_name == format!("{marker}-agent.completed"))
.count(),
1
);
assert!(
captures
.iter()
.all(|capture| capture.hook_name != format!("{marker}-agent.interrupted")),
"a deliberate exit is never an interruption"
);
}
#[test] #[test]
#[serial] #[serial]
fn top_level_agent_helpers_noop_without_agent() { fn top_level_agent_helpers_noop_without_agent() {
@@ -8549,7 +8772,7 @@ mod tests {
ctx.role = Some(Role::new("plain", "prompt")); ctx.role = Some(Role::new("plain", "prompt"));
ctx.top_level_agent_started(); ctx.top_level_agent_started();
ctx.top_level_agent_finished(None); ctx.top_level_agent_finished(None, None);
assert!( assert!(
marker_captures(marker).is_empty(), marker_captures(marker).is_empty(),
@@ -8575,7 +8798,7 @@ mod tests {
ctx.exit_agent(&app).unwrap(); ctx.exit_agent(&app).unwrap();
// A later exit seam (REPL teardown) must not double-fire. // A later exit seam (REPL teardown) must not double-fire.
ctx.top_level_agent_finished(None); ctx.top_level_agent_finished(None, None);
let captures = marker_captures(marker); let captures = marker_captures(marker);
assert_eq!( assert_eq!(
@@ -9178,19 +9401,19 @@ mod tests {
fn install_builtin_agents_force_overwrites_only_with_force() { fn install_builtin_agents_force_overwrites_only_with_force() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
let file = let file =
first_file(&paths::agents_data_dir()).expect("bundled agents should be installed"); first_file(&paths::agents_data_dir()).expect("bundled agents should be installed");
write(&file, "SENTINEL").unwrap(); write(&file, "SENTINEL").unwrap();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
assert_eq!( assert_eq!(
read_to_string(&file).unwrap(), read_to_string(&file).unwrap(),
"SENTINEL", "SENTINEL",
"non-force install must not overwrite an existing file" "non-force install must not overwrite an existing file"
); );
Agent::install_builtin_agents(true).unwrap(); Agent::install_builtin_agents(InstallMode::Force).unwrap();
assert_ne!( assert_ne!(
read_to_string(&file).unwrap(), read_to_string(&file).unwrap(),
"SENTINEL", "SENTINEL",
@@ -9203,7 +9426,7 @@ mod tests {
fn install_builtin_agents_removes_stale_definition_files() { fn install_builtin_agents_removes_stale_definition_files() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
let agents_dir = paths::agents_data_dir(); let agents_dir = paths::agents_data_dir();
// Simulate an upgrade from an older install: coder used to ship // Simulate an upgrade from an older install: coder used to ship
@@ -9224,7 +9447,7 @@ mod tests {
create_dir_all(agents_dir.join("myagent")).unwrap(); create_dir_all(agents_dir.join("myagent")).unwrap();
write(agents_dir.join("myagent").join("config.yaml"), "keep").unwrap(); write(agents_dir.join("myagent").join("config.yaml"), "keep").unwrap();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
assert!( assert!(
!agents_dir.join("coder").join("config.yaml").exists(), !agents_dir.join("coder").join("config.yaml").exists(),
@@ -9264,12 +9487,12 @@ mod tests {
fn install_builtin_agents_removes_stale_graph_for_config_agent() { fn install_builtin_agents_removes_stale_graph_for_config_agent() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
let agents_dir = paths::agents_data_dir(); let agents_dir = paths::agents_data_dir();
write(agents_dir.join("architect").join("graph.yaml"), "stale").unwrap(); write(agents_dir.join("architect").join("graph.yaml"), "stale").unwrap();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
assert!( assert!(
!agents_dir.join("architect").join("graph.yaml").exists(), !agents_dir.join("architect").join("graph.yaml").exists(),
@@ -9285,7 +9508,7 @@ mod tests {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
let agents_dir = paths::agents_data_dir(); let agents_dir = paths::agents_data_dir();
// Simulate an upgrade from an older install whose coder shipped a // Simulate an upgrade from an older install whose coder shipped a
@@ -9297,14 +9520,20 @@ mod tests {
write(hooks_dir.join(BUILTIN_MANIFEST_FILE), "old-hook.sh\n").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("old-hook.sh"), "stale").unwrap();
write(hooks_dir.join("user-hook.sh"), "user-owned").unwrap(); write(hooks_dir.join("user-hook.sh"), "user-owned").unwrap();
// A custom agent outside the bundle is never reconciled, even with a // An agent directory carrying a builtin manifest but absent from the
// manifest present. // embed is a removed builtin agent: the sweep deletes exactly the
let custom_hooks = agents_dir.join("myagent").join("hooks"); // manifest-owned files, sparing anything user-created alongside.
create_dir_all(&custom_hooks).unwrap(); let removed_hooks = agents_dir.join("myagent").join("hooks");
write(custom_hooks.join(BUILTIN_MANIFEST_FILE), "custom.sh\n").unwrap(); create_dir_all(&removed_hooks).unwrap();
write(custom_hooks.join("custom.sh"), "keep").unwrap(); write(removed_hooks.join(BUILTIN_MANIFEST_FILE), "shipped.sh\n").unwrap();
write(removed_hooks.join("shipped.sh"), "stale").unwrap();
write(removed_hooks.join("user-note.sh"), "keep").unwrap();
// A user agent never has a manifest, so the sweep must not touch it.
let user_hooks = agents_dir.join("useragent").join("hooks");
create_dir_all(&user_hooks).unwrap();
write(user_hooks.join("mine.sh"), "keep").unwrap();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
assert!( assert!(
!hooks_dir.join("old-hook.sh").exists(), !hooks_dir.join("old-hook.sh").exists(),
@@ -9319,10 +9548,18 @@ mod tests {
"an empty shipped set removes the manifest itself" "an empty shipped set removes the manifest itself"
); );
assert!( assert!(
custom_hooks.join("custom.sh").exists(), !removed_hooks.join("shipped.sh").exists(),
"custom agents outside the bundle must survive reconciliation" "manifest-owned hooks of a removed builtin agent must be swept"
);
assert!(!removed_hooks.join(BUILTIN_MANIFEST_FILE).exists());
assert!(
removed_hooks.join("user-note.sh").exists(),
"user files beside a removed agent's manifest must survive the sweep"
);
assert!(
user_hooks.join("mine.sh").exists(),
"agent dirs without a manifest must never be touched"
); );
assert!(custom_hooks.join(BUILTIN_MANIFEST_FILE).exists());
} }
#[test] #[test]
@@ -9366,7 +9603,7 @@ mod tests {
#[serial] #[serial]
fn bundled_assets_pin_task_queue_guidance() { fn bundled_assets_pin_task_queue_guidance() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
// 1. Architect config: the parallel-mode "Task-queue mirroring" // 1. Architect config: the parallel-mode "Task-queue mirroring"
// section with all five HARD RULES. // section with all five HARD RULES.
@@ -9435,7 +9672,7 @@ mod tests {
#[serial] #[serial]
fn bundled_assets_pin_review_incomplete_escalation() { fn bundled_assets_pin_review_incomplete_escalation() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
// Every anchor is a single-line prefix so YAML re-wrapping cannot // Every anchor is a single-line prefix so YAML re-wrapping cannot
// break the pin; clauses that wrap differently per config are split // break the pin; clauses that wrap differently per config are split
@@ -9599,7 +9836,7 @@ mod tests {
#[serial] #[serial]
fn bundled_assets_pin_verification_commands_as_declared_variable() { fn bundled_assets_pin_verification_commands_as_declared_variable() {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
const VARIABLES_FORM: &str = const VARIABLES_FORM: &str =
"--variables {\"verification_commands\": \"[\\\"cargo test --all\\\""; "--variables {\"verification_commands\": \"[\\\"cargo test --all\\\"";
@@ -9790,7 +10027,7 @@ mod tests {
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
for (name, expected) in [ for (name, expected) in [
("architect", 10), ("architect", 10),
@@ -9909,7 +10146,7 @@ mod tests {
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
Skill::install_builtin_skills(false).unwrap(); Skill::install_builtin_skills(false).unwrap();
let mut checked = Vec::new(); let mut checked = Vec::new();
@@ -10136,7 +10373,7 @@ mod tests {
let engine_seeded = ["initial_prompt", "output", "choice", "input"]; let engine_seeded = ["initial_prompt", "output", "choice", "input"];
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new();
Agent::install_builtin_agents(false).unwrap(); Agent::install_builtin_agents(InstallMode::Skip).unwrap();
let mut checked = Vec::new(); let mut checked = Vec::new();
for entry in std::fs::read_dir(paths::agents_data_dir()).unwrap() { for entry in std::fs::read_dir(paths::agents_data_dir()).unwrap() {
@@ -10255,13 +10492,24 @@ mod tests {
// Scripts' load_state() prefers GRAPH_STATE_FILE over GRAPH_STATE, so an // Scripts' load_state() prefers GRAPH_STATE_FILE over GRAPH_STATE, so an
// inherited live state file (adversary verifying this repo) must not win. // inherited live state file (adversary verifying this repo) must not win.
// run_checks.py's review-stack flock defaults to ONE system-wide path;
// concurrent tests (or a real review running on this machine) would
// contend on it, and the losers' 5s-sleep retry loop burns the tight
// test deadlines. Every spawned script gets its own throwaway lock path.
fn adversary_script_command(script: &str, state: &serde_json::Value) -> Command { fn adversary_script_command(script: &str, state: &serde_json::Value) -> Command {
let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) static STACK_LOCK_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
let path = Path::new(env!("CARGO_MANIFEST_DIR"))
.join("assets/agents/adversary/scripts") .join("assets/agents/adversary/scripts")
.join(script); .join(script);
let lock_path = env::temp_dir().join(format!(
"coyote-adversary-test-stack-{}-{}.lock",
std::process::id(),
STACK_LOCK_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
));
let mut cmd = Command::new("python3"); let mut cmd = Command::new("python3");
cmd.arg(&path) cmd.arg(&path)
.env("GRAPH_STATE", state.to_string()) .env("GRAPH_STATE", state.to_string())
.env("ADVERSARY_STACK_LOCK_PATH", &lock_path)
.env_remove("GRAPH_STATE_FILE"); .env_remove("GRAPH_STATE_FILE");
cmd cmd
} }
@@ -10853,6 +11101,67 @@ mod tests {
); );
} }
/// ADVERSARY_STACK_LOCK_PATH must relocate the review-stack flock:
/// without it, every concurrent run_checks.py (parallel tests, or a real
/// review on the same machine) contends on the one system-wide lock path
/// and the losers' retry loop burns the total deadline budget.
#[test]
#[cfg(unix)]
fn adversary_run_checks_stack_lock_path_is_env_overridable() {
if !cmd_available("python3") || !cmd_available("sh") {
eprintln!("skipping: python3 or sh not available");
return;
}
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp = env::temp_dir().join(format!(
"coyote-adversary-run-checks-lockpath-{}-{unique}",
std::process::id()
));
create_dir_all(&tmp).unwrap();
// A not-yet-existing subdir pins the makedirs-on-override behavior.
let lock_path = tmp.join("override").join("stack.lock");
let lock_env = lock_path.to_str().unwrap();
let state = json!({"verification_commands": ["true"]});
// Two sequential runs with the same override: the lock fd is held
// only for the process lifetime, so the second run must acquire
// immediately — and both use the OVERRIDDEN location.
for run in 0..2 {
let out = run_adversary_script_env(
"run_checks.py",
&state,
&[("ADVERSARY_STACK_LOCK_PATH", lock_env)],
);
assert_eq!(
out["exec_results"][0]["exit"], 0,
"run {run}: the declared command must run to completion: {out}"
);
assert!(
lock_path.exists(),
"run {run}: the stack lock must be taken at the overridden path {}",
lock_path.display()
);
}
// No declared commands → the lock is skipped entirely: a fresh
// override path must never be created.
let skipped = tmp.join("skipped.lock");
let _ = run_adversary_script_env(
"run_checks.py",
&json!({}),
&[("ADVERSARY_STACK_LOCK_PATH", skipped.to_str().unwrap())],
);
assert!(
!skipped.exists(),
"with no declared commands the lock must be skipped, but {} was created",
skipped.display()
);
let _ = remove_dir_all(&tmp);
}
/// TOTAL_DEADLINE_SECS in run_checks.py and the run_checks node's /// TOTAL_DEADLINE_SECS in run_checks.py and the run_checks node's
/// `timeout:` in graph.yaml live in different files; if the script's /// `timeout:` in graph.yaml live in different files; if the script's
/// deadline ever creeps past the node timeout, the executor kills the /// deadline ever creeps past the node timeout, the executor kills the
+112 -16
View File
@@ -2,6 +2,7 @@ use super::*;
use crate::client::{Message, MessageContent, MessageRole, Model}; use crate::client::{Message, MessageContent, MessageRole, Model};
use crate::config::builtin_manifest; use crate::config::builtin_manifest;
use crate::config::conflict::{self, InstallMode, StickyMode};
use crate::function::write_file_atomic; use crate::function::write_file_atomic;
use crate::hooks::HooksMap; use crate::hooks::HooksMap;
@@ -179,7 +180,8 @@ impl Role {
pub fn builtin(name: &str) -> Result<Self> { pub fn builtin(name: &str) -> Result<Self> {
let content = RolesAsset::get(&format!("{name}.md")) let content = RolesAsset::get(&format!("{name}.md"))
.ok_or_else(|| anyhow!("Unknown role `{name}`"))?; .ok_or_else(|| anyhow!("Unknown role `{name}`"))?;
let content = unsafe { std::str::from_utf8_unchecked(&content.data) }; let content =
std::str::from_utf8(&content.data).expect("bundled role asset is not valid UTF-8");
Ok(Role::new(name, content)) Ok(Role::new(name, content))
} }
@@ -193,7 +195,7 @@ impl Role {
/// reconciles ones a release stopped shipping. This is the only builtin /// reconciles ones a release stopped shipping. This is the only builtin
/// install machinery roles have: general role installation deliberately /// install machinery roles have: general role installation deliberately
/// does not exist (builtin roles load straight from the embed). /// does not exist (builtin roles load straight from the embed).
pub fn install_builtin_role_hooks(force: bool) -> Result<()> { pub fn install_builtin_role_hooks(mode: InstallMode, sticky: &mut StickyMode) -> Result<()> {
let shipped: Vec<(String, String)> = RolesAsset::iter() let shipped: Vec<(String, String)> = RolesAsset::iter()
.filter_map(|file| { .filter_map(|file| {
let name = file.as_ref().strip_prefix("hooks/")?; let name = file.as_ref().strip_prefix("hooks/")?;
@@ -201,12 +203,13 @@ impl Role {
return None; return None;
} }
let embedded = RolesAsset::get(file.as_ref())?; let embedded = RolesAsset::get(file.as_ref())?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded.data) }; let content = std::str::from_utf8(&embedded.data)
.expect("bundled role hook asset is not valid UTF-8");
Some((name.to_string(), content.to_string())) Some((name.to_string(), content.to_string()))
}) })
.collect(); .collect();
install_and_reconcile_role_hooks(&paths::roles_dir().join("hooks"), &shipped, force) install_and_reconcile_role_hooks(&paths::roles_dir().join("hooks"), &shipped, mode, sticky)
} }
pub fn has_args(&self) -> bool { pub fn has_args(&self) -> bool {
@@ -641,10 +644,11 @@ fn parse_structure_prompt(prompt: &str) -> (&str, Vec<(&str, &str)>) {
/// Only direct children of the hooks/ directory are supported: they are the /// Only direct children of the hooks/ directory are supported: they are the
/// only shape the builtin manifest tracks, so nested asset paths are skipped /// only shape the builtin manifest tracks, so nested asset paths are skipped
/// by the caller rather than installed unreconciled. /// by the caller rather than installed unreconciled.
fn install_and_reconcile_role_hooks( pub(crate) fn install_and_reconcile_role_hooks(
dir: &Path, dir: &Path,
shipped: &[(String, String)], shipped: &[(String, String)],
force: bool, mode: InstallMode,
sticky: &mut StickyMode,
) -> Result<()> { ) -> Result<()> {
if !shipped.is_empty() { if !shipped.is_empty() {
info!("Installing built-in role hooks in {}", dir.display()); info!("Installing built-in role hooks in {}", dir.display());
@@ -652,7 +656,9 @@ fn install_and_reconcile_role_hooks(
let mut written = BTreeSet::new(); let mut written = BTreeSet::new();
for (name, content) in shipped { for (name, content) in shipped {
let path = dir.join(name); let path = dir.join(name);
if path.exists() && !force { if path.exists()
&& !conflict::should_replace_existing(&path, content, "roles", mode, sticky)?
{
debug!( debug!(
"Role hook file already exists, skipping: {}", "Role hook file already exists, skipping: {}",
path.display() path.display()
@@ -661,7 +667,8 @@ fn install_and_reconcile_role_hooks(
} }
ensure_parent_exists(&path)?; ensure_parent_exists(&path)?;
info!("Creating role hook file: {}", path.display()); info!("Creating role hook file: {}", path.display());
write_file_atomic(&path, content, Some(0o755))?; write_file_atomic(&path, content, None)?;
set_executable_bit_if_script(&path)?;
written.insert(name.clone()); written.insert(name.clone());
} }
@@ -1049,7 +1056,12 @@ Input 1
fn role_hooks_install_writes_scripts_and_manifest() { fn role_hooks_install_writes_scripts_and_manifest() {
let dir = hooks_fixture_dir("role-hooks-install-"); let dir = hooks_fixture_dir("role-hooks-install-");
install_and_reconcile_role_hooks(&dir, &fixture(&[("a.sh", "#!/bin/sh\n")]), false) install_and_reconcile_role_hooks(
&dir,
&fixture(&[("a.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap(); .unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "#!/bin/sh\n"); assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "#!/bin/sh\n");
@@ -1075,6 +1087,41 @@ Input 1
let _ = fs::remove_dir_all(dir.parent().unwrap()); let _ = fs::remove_dir_all(dir.parent().unwrap());
} }
/// The installer writes plain and then applies the shared exec-bit
/// predicate: users may keep non-executable support files in hooks
/// directories, so only recognized script extensions get 0755.
#[cfg(unix)]
#[test]
fn role_hooks_install_leaves_non_script_files_non_executable() {
use std::os::unix::fs::PermissionsExt;
let dir = hooks_fixture_dir("role-hooks-nonscript-");
install_and_reconcile_role_hooks(
&dir,
&fixture(&[("README.md", "docs\n"), ("run.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap();
let readme_mode = fs::metadata(dir.join("README.md"))
.unwrap()
.permissions()
.mode();
assert_eq!(
readme_mode & 0o111,
0,
"a non-script file in a hooks dir must stay non-executable"
);
let script_mode = fs::metadata(dir.join("run.sh"))
.unwrap()
.permissions()
.mode();
assert_eq!(script_mode & 0o777, 0o755);
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test] #[test]
fn role_hooks_install_overwrites_only_with_force() { fn role_hooks_install_overwrites_only_with_force() {
let dir = hooks_fixture_dir("role-hooks-force-"); let dir = hooks_fixture_dir("role-hooks-force-");
@@ -1082,14 +1129,50 @@ Input 1
create_dir_all(&dir).unwrap(); create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "SENTINEL").unwrap(); fs::write(dir.join("a.sh"), "SENTINEL").unwrap();
install_and_reconcile_role_hooks(&dir, &shipped, false).unwrap(); install_and_reconcile_role_hooks(&dir, &shipped, InstallMode::Skip, &mut StickyMode::None)
.unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "SENTINEL"); assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "SENTINEL");
install_and_reconcile_role_hooks(&dir, &shipped, true).unwrap(); install_and_reconcile_role_hooks(&dir, &shipped, InstallMode::Force, &mut StickyMode::None)
.unwrap();
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "new content\n"); assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "new content\n");
let _ = fs::remove_dir_all(dir.parent().unwrap()); let _ = fs::remove_dir_all(dir.parent().unwrap());
} }
#[test]
#[serial_test::serial]
fn role_hooks_prompt_mode_asks_per_conflict_and_honors_sticky() {
use crate::config::conflict::prompt_script;
let dir = hooks_fixture_dir("role-hooks-prompt-");
let shipped = fixture(&[("a.sh", "new content\n"), ("b.sh", "new content\n")]);
create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "local a").unwrap();
fs::write(dir.join("b.sh"), "local b").unwrap();
// First conflict answered per-file, second by the sticky replace-all.
let script = prompt_script::install(&["replace-all"]);
let mut sticky = StickyMode::None;
install_and_reconcile_role_hooks(&dir, &shipped, InstallMode::Prompt, &mut sticky).unwrap();
assert_eq!(prompt_script::prompts_asked(), 1);
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "new content\n");
assert_eq!(read_to_string(dir.join("b.sh")).unwrap(), "new content\n");
assert_eq!(sticky, StickyMode::ReplaceAll);
drop(script);
// A sticky mode carried in from an earlier location suppresses all
// prompting: `--install-builtins hooks` shares one sticky scope
// across the global, role, and agent hook locations.
fs::write(dir.join("a.sh"), "local again").unwrap();
let script = prompt_script::install(&[]);
let mut sticky = StickyMode::KeepAll;
install_and_reconcile_role_hooks(&dir, &shipped, InstallMode::Prompt, &mut sticky).unwrap();
assert_eq!(prompt_script::prompts_asked(), 0);
assert_eq!(read_to_string(dir.join("a.sh")).unwrap(), "local again");
drop(script);
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test] #[test]
fn role_hooks_reconcile_removes_dropped_hook_and_keeps_user_files() { fn role_hooks_reconcile_removes_dropped_hook_and_keeps_user_files() {
let dir = hooks_fixture_dir("role-hooks-reconcile-"); let dir = hooks_fixture_dir("role-hooks-reconcile-");
@@ -1097,12 +1180,18 @@ Input 1
install_and_reconcile_role_hooks( install_and_reconcile_role_hooks(
&dir, &dir,
&fixture(&[("keep.sh", "#!/bin/sh\n"), ("drop.sh", "#!/bin/sh\n")]), &fixture(&[("keep.sh", "#!/bin/sh\n"), ("drop.sh", "#!/bin/sh\n")]),
false, InstallMode::Skip,
&mut StickyMode::None,
) )
.unwrap(); .unwrap();
fs::write(dir.join("user.sh"), "user-owned").unwrap(); fs::write(dir.join("user.sh"), "user-owned").unwrap();
install_and_reconcile_role_hooks(&dir, &fixture(&[("keep.sh", "#!/bin/sh\n")]), false) install_and_reconcile_role_hooks(
&dir,
&fixture(&[("keep.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap(); .unwrap();
assert!( assert!(
@@ -1124,14 +1213,20 @@ Input 1
create_dir_all(&dir).unwrap(); create_dir_all(&dir).unwrap();
fs::write(dir.join("a.sh"), "user-owned").unwrap(); fs::write(dir.join("a.sh"), "user-owned").unwrap();
install_and_reconcile_role_hooks(&dir, &fixture(&[("a.sh", "#!/bin/sh\n")]), false) install_and_reconcile_role_hooks(
&dir,
&fixture(&[("a.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap(); .unwrap();
assert!( assert!(
!dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE).exists(), !dir.join(builtin_manifest::BUILTIN_MANIFEST_FILE).exists(),
"a skipped pre-existing file must not be claimed by the manifest" "a skipped pre-existing file must not be claimed by the manifest"
); );
install_and_reconcile_role_hooks(&dir, &[], false).unwrap(); install_and_reconcile_role_hooks(&dir, &[], InstallMode::Skip, &mut StickyMode::None)
.unwrap();
assert_eq!( assert_eq!(
read_to_string(dir.join("a.sh")).unwrap(), read_to_string(dir.join("a.sh")).unwrap(),
"user-owned", "user-owned",
@@ -1144,7 +1239,8 @@ Input 1
fn role_hooks_empty_shipped_set_creates_nothing() { fn role_hooks_empty_shipped_set_creates_nothing() {
let dir = hooks_fixture_dir("role-hooks-empty-"); let dir = hooks_fixture_dir("role-hooks-empty-");
install_and_reconcile_role_hooks(&dir, &[], false).unwrap(); install_and_reconcile_role_hooks(&dir, &[], InstallMode::Skip, &mut StickyMode::None)
.unwrap();
assert!(!dir.exists(), "an empty shipped set must not create hooks/"); assert!(!dir.exists(), "an empty shipped set must not create hooks/");
let _ = fs::remove_dir_all(dir.parent().unwrap()); let _ = fs::remove_dir_all(dir.parent().unwrap());
+2 -1
View File
@@ -100,7 +100,8 @@ impl Skill {
let embedded_file = SkillsAsset::get(&file) let embedded_file = SkillsAsset::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded skill file: {}", file.as_ref()))?; .ok_or_else(|| anyhow!("Failed to load embedded skill file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) }; let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled skill asset is not valid UTF-8");
let file_path = paths::skills_dir().join(file.as_ref()); let file_path = paths::skills_dir().join(file.as_ref());
if file_path.exists() && !force { if file_path.exists() && !force {
+7 -7
View File
@@ -918,7 +918,7 @@ mod tests {
use super::*; use super::*;
use crate::config::mcp_tool_policy::LayerSource; use crate::config::mcp_tool_policy::LayerSource;
use crate::function::ToolCall; use crate::function::ToolCall;
use crate::testing::{install_warn_collector, warn_messages}; use crate::testing::{install_log_collector, warn_snapshot};
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
#[test] #[test]
@@ -1083,7 +1083,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn catalog_items_warns_when_resource_listing_fails() { async fn catalog_items_warns_when_resource_listing_fails() {
install_warn_collector(); install_log_collector();
let fixture = FixtureServer { let fixture = FixtureServer {
resources_capability: true, resources_capability: true,
fail_resource_listings: true, fail_resource_listings: true,
@@ -1093,7 +1093,7 @@ mod tests {
runtime.catalog_items("fixture").await.unwrap(); runtime.catalog_items("fixture").await.unwrap();
let messages = warn_messages().lock().unwrap(); let messages = warn_snapshot();
assert!( assert!(
messages messages
.iter() .iter()
@@ -1547,7 +1547,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn prompt_catalog_degrades_when_one_server_fails() { async fn prompt_catalog_degrades_when_one_server_fails() {
install_warn_collector(); install_log_collector();
let healthy = FixtureServer { let healthy = FixtureServer {
prompts_capability: true, prompts_capability: true,
..Default::default() ..Default::default()
@@ -1564,7 +1564,7 @@ mod tests {
assert_eq!(items.len(), 1); assert_eq!(items.len(), 1);
assert_eq!(items[0].server, "fixture"); assert_eq!(items[0].server, "fixture");
let messages = warn_messages().lock().unwrap(); let messages = warn_snapshot();
assert!( assert!(
messages messages
.iter() .iter()
@@ -1794,7 +1794,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn catalog_items_warns_on_dead_context_patterns() { async fn catalog_items_warns_on_dead_context_patterns() {
install_warn_collector(); install_log_collector();
let (mut runtime, _server) = fixture_runtime(FixtureServer::default()).await; let (mut runtime, _server) = fixture_runtime(FixtureServer::default()).await;
let mut filter = single_layer_filter(LayerSource::Global, &["*"]); let mut filter = single_layer_filter(LayerSource::Global, &["*"]);
filter.push_layer(LayerSource::Session, &["zzz_*".to_string()]); filter.push_layer(LayerSource::Session, &["zzz_*".to_string()]);
@@ -1802,7 +1802,7 @@ mod tests {
runtime.catalog_items("fixture").await.unwrap(); runtime.catalog_items("fixture").await.unwrap();
let messages = warn_messages().lock().unwrap(); let messages = warn_snapshot();
assert!( assert!(
messages.iter().any(|msg| msg.contains("'zzz_*'") messages.iter().any(|msg| msg.contains("'zzz_*'")
&& msg.contains("session (.set)") && msg.contains("session (.set)")
+159 -46
View File
@@ -4,7 +4,7 @@ use crate::client::{Model, ModelType, call_chat_completions};
use crate::config::{ use crate::config::{
Agent, AgentVariable, AgentVariables, AppState, Input, RequestContext, Role, RoleLike, Agent, AgentVariable, AgentVariables, AppState, Input, RequestContext, Role, RoleLike,
default_max_agent_depth, effective_max_concurrent_jobs, jobs_enabled, default_max_agent_depth, effective_max_concurrent_jobs, jobs_enabled,
list_agents_with_descriptions, load_agent_variables, paths, list_agents_with_descriptions, load_agent_variables,
}; };
use crate::hooks::{self, HookEvent, ResolvedHook}; use crate::hooks::{self, HookEvent, ResolvedHook};
use crate::supervisor::mailbox::{Envelope, EnvelopePayload, Inbox, PeerRegistry, graph_agent_id}; use crate::supervisor::mailbox::{Envelope, EnvelopePayload, Inbox, PeerRegistry, graph_agent_id};
@@ -15,7 +15,8 @@ use crate::utils::{AbortSignal, create_abort_signal, wait_abort_signal, wait_use
use crate::graph; use crate::graph;
use crate::repl::DEFAULT_CONTINUATION_PROMPT; use crate::repl::DEFAULT_CONTINUATION_PROMPT;
use anyhow::{Context, Result, anyhow, bail}; use anyhow::{Context, Result, anyhow, bail};
use chrono::{SecondsFormat, Utc}; use chrono::Utc;
use hooks::McpServerHooks;
use indexmap::IndexMap; use indexmap::IndexMap;
use log::{debug, warn}; use log::{debug, warn};
use parking_lot::RwLock; use parking_lot::RwLock;
@@ -961,10 +962,11 @@ async fn populate_agent_mcp_runtime(ctx: &mut RequestContext, server_ids: &[Stri
}) })
.unwrap_or_default(); .unwrap_or_default();
let mcp_hooks = McpServerHooks::resolve(ctx);
for (id, spec) in server_specs { for (id, spec) in server_specs {
let handle = app let handle = app
.mcp_factory .mcp_factory
.acquire(&id, &spec, app.mcp_log_path.as_deref()) .acquire(&id, &spec, app.mcp_log_path.as_deref(), &mcp_hooks)
.await?; .await?;
ctx.tool_scope.mcp_runtime.insert(id, handle); ctx.tool_scope.mcp_runtime.insert(id, handle);
} }
@@ -997,44 +999,15 @@ fn sync_agent_functions_to_ctx(ctx: &mut RequestContext) -> Result<()> {
Ok(()) Ok(())
} }
/// Mirrors the base-env contract `hooks::fire` assembles from a live context, /// Pre-resolved `agent.completed`/`agent.interrupted`/`agent.failed`
/// for detached call sites that dispatch pre-resolved hook snapshots after /// snapshots for a spawned agent's result arms, captured from the child
/// the context they were resolved from is gone. The names are captured by /// context before the spawn task takes ownership of it. Resolving against
/// the caller when it takes the snapshot; the timestamp is taken here, at /// the child keeps global hooks gated by the child's own whitelist, the
/// fire time, so it reflects when the event actually happened. /// same gate every other event fired from the child's context goes through.
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 { struct SpawnResultHooks {
completed: Vec<ResolvedHook>, completed: Vec<ResolvedHook>,
failed: Vec<ResolvedHook>, failed: Vec<ResolvedHook>,
interrupted: Vec<ResolvedHook>,
session_name: Option<String>, session_name: Option<String>,
agent_id: String, agent_id: String,
agent_name: String, agent_name: String,
@@ -1045,6 +1018,7 @@ impl SpawnResultHooks {
Self { Self {
completed: child_ctx.resolved_hooks(HookEvent::AgentCompleted), completed: child_ctx.resolved_hooks(HookEvent::AgentCompleted),
failed: child_ctx.resolved_hooks(HookEvent::AgentFailed), failed: child_ctx.resolved_hooks(HookEvent::AgentFailed),
interrupted: child_ctx.resolved_hooks(HookEvent::AgentInterrupted),
session_name: child_ctx session_name: child_ctx
.session .session
.as_ref() .as_ref()
@@ -1054,14 +1028,16 @@ impl SpawnResultHooks {
} }
} }
/// Fires `agent.completed`, or `agent.failed` when `error` is given. fn fire(self, error: Option<&str>, child_abort: &AbortSignal) {
fn fire(self, error: Option<&str>) {
let mut extras = vec![ let mut extras = vec![
("COYOTE_AGENT_ID", self.agent_id), ("COYOTE_AGENT_ID", self.agent_id),
("COYOTE_AGENT_NAME", self.agent_name.clone()), ("COYOTE_AGENT_NAME", self.agent_name.clone()),
]; ];
let (event, resolved) = match error { let (event, resolved) = match error {
None => (HookEvent::AgentCompleted, self.completed), None => (HookEvent::AgentCompleted, self.completed),
Some(_) if child_abort.aborted_ctrlc() => {
(HookEvent::AgentInterrupted, self.interrupted)
}
Some(error) => { Some(error) => {
extras.push(("COYOTE_AGENT_ERROR", error.to_string())); extras.push(("COYOTE_AGENT_ERROR", error.to_string()));
(HookEvent::AgentFailed, self.failed) (HookEvent::AgentFailed, self.failed)
@@ -1070,7 +1046,7 @@ impl SpawnResultHooks {
hooks::fire_resolved( hooks::fire_resolved(
event, event,
resolved, resolved,
hook_base_envs(event, self.session_name.as_deref(), Some(&self.agent_name)), hooks::base_envs_parts(event, self.session_name.as_deref(), Some(&self.agent_name)),
&extras, &extras,
None, None,
); );
@@ -1246,6 +1222,7 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let spawn_agent_id = agent_id.clone(); let spawn_agent_id = agent_id.clone();
let spawn_agent_name = agent_name.clone(); let spawn_agent_name = agent_name.clone();
let spawn_abort = child_abort.clone(); let spawn_abort = child_abort.clone();
let hook_abort = child_abort.clone();
let spawn_notifications = Arc::clone(&ctx.notification_queue); let spawn_notifications = Arc::clone(&ctx.notification_queue);
let child_supervisor = child_ctx.supervisor.clone(); let child_supervisor = child_ctx.supervisor.clone();
@@ -1268,8 +1245,8 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
}; };
let success = agent_result.exit_status == AgentExitStatus::Completed; let success = agent_result.exit_status == AgentExitStatus::Completed;
match &agent_result.exit_status { match &agent_result.exit_status {
AgentExitStatus::Completed => result_hooks.fire(None), AgentExitStatus::Completed => result_hooks.fire(None, &hook_abort),
AgentExitStatus::Failed(e) => result_hooks.fire(Some(e)), AgentExitStatus::Failed(e) => result_hooks.fire(Some(e), &hook_abort),
} }
spawn_notifications.push(agent_notification( spawn_notifications.push(agent_notification(
&agent_result.id, &agent_result.id,
@@ -1801,6 +1778,18 @@ fn handle_reply_escalation(ctx: &mut RequestContext, args: &Value) -> Result<Val
let from_agent = request.from_agent_name.clone(); let from_agent = request.from_agent_name.clone();
let question = request.question.clone(); let question = request.question.clone();
let _ = request.reply_tx.send(reply.to_string()); let _ = request.reply_tx.send(reply.to_string());
hooks::fire(
HookEvent::EscalationAnswered,
ctx,
&[
("COYOTE_ESCALATION_ID", escalation_id.to_string()),
("COYOTE_ESCALATION_FROM_AGENT_ID", request.from_agent_id),
("COYOTE_ESCALATION_FROM_AGENT_NAME", from_agent.clone()),
("COYOTE_ESCALATION_QUESTION", question.clone()),
("COYOTE_ESCALATION_REPLY", reply.to_string()),
],
None,
);
Ok(json!({ Ok(json!({
"status": "ok", "status": "ok",
"message": format!("Reply sent to agent '{from_agent}' for escalation '{escalation_id}'"), "message": format!("Reply sent to agent '{from_agent}' for escalation '{escalation_id}'"),
@@ -2281,6 +2270,9 @@ mod tests {
\x20 agent.completed:\n\ \x20 agent.completed:\n\
\x20 - name: {marker}_completed\n\ \x20 - name: {marker}_completed\n\
\x20 command: \"true\"\n\ \x20 command: \"true\"\n\
\x20 agent.interrupted:\n\
\x20 - name: {marker}_interrupted\n\
\x20 command: \"true\"\n\
\x20 agent.failed:\n\ \x20 agent.failed:\n\
\x20 - name: {marker}_failed\n\ \x20 - name: {marker}_failed\n\
\x20 command: \"true\"\n\ \x20 command: \"true\"\n\
@@ -2377,8 +2369,56 @@ mod tests {
} }
#[test] #[test]
fn hook_base_envs_carries_optional_names_only_when_present() { #[serial]
let envs = hook_base_envs(HookEvent::AgentCompleted, None, None); fn cancelled_spawned_agent_fires_interrupted_not_failed() {
let _guard = TestConfigDirGuard::new();
let _sink = hooks::test_sink::install();
let agent_name = unique_agent_name("test_hook_spawn_cancel");
write_graph_agent_with_hooks(&agent_name, "t049_spawn_cancel", true);
let id = 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");
// The `agent__cancel` signal, fired before the spawn task runs:
// on this current-thread runtime the task only progresses once
// awaited, so its failure arm observes the ctrl-c deterministically.
handle.abort_signal.set_ctrlc();
handle
.join_handle
.await
.expect("spawn task must not panic")
.expect("spawn task must return an AgentResult");
id
});
let interrupted = hook_captures_named("t049_spawn_cancel_interrupted");
assert_eq!(interrupted.len(), 1, "{interrupted:?}");
assert_eq!(interrupted[0].envs["COYOTE_EVENT"], "agent.interrupted");
assert_eq!(interrupted[0].envs["COYOTE_AGENT_ID"], id);
assert_eq!(interrupted[0].envs["COYOTE_AGENT_NAME"], agent_name);
assert!(
!interrupted[0].envs.contains_key("COYOTE_AGENT_ERROR"),
"an interruption is not a failure and carries no error env"
);
assert!(hook_captures_named("t049_spawn_cancel_failed").is_empty());
assert!(hook_captures_named("t049_spawn_cancel_completed").is_empty());
}
#[test]
fn agent_event_base_envs_carry_optional_names_only_when_present() {
let envs = hooks::base_envs_parts(HookEvent::AgentCompleted, None, None);
let keys: Vec<&str> = envs.iter().map(|(key, _)| key.as_str()).collect(); let keys: Vec<&str> = envs.iter().map(|(key, _)| key.as_str()).collect();
assert!(keys.contains(&"COYOTE_EVENT")); assert!(keys.contains(&"COYOTE_EVENT"));
assert!(keys.contains(&"COYOTE_CONFIG_DIR")); assert!(keys.contains(&"COYOTE_CONFIG_DIR"));
@@ -2390,7 +2430,7 @@ mod tests {
.any(|(key, value)| key == "COYOTE_EVENT" && value == "agent.completed") .any(|(key, value)| key == "COYOTE_EVENT" && value == "agent.completed")
); );
let envs = hook_base_envs(HookEvent::AgentFailed, Some("sess"), Some("bot")); let envs = hooks::base_envs_parts(HookEvent::AgentFailed, Some("sess"), Some("bot"));
assert!( assert!(
envs.iter() envs.iter()
.any(|(key, value)| key == "COYOTE_SESSION_ID" && value == "sess") .any(|(key, value)| key == "COYOTE_SESSION_ID" && value == "sess")
@@ -3354,6 +3394,79 @@ mod tests {
assert!(result.is_err()); assert!(result.is_err());
} }
/// `escalation.answered` belongs to the successful take -> reply arm
/// alone: exactly once per escalation, and never from the failure arms
/// (unknown id, already-taken id).
#[test]
#[serial]
fn reply_escalation_fires_answered_once_and_never_on_failure_arms() {
let _sink = hooks::test_sink::install();
let marker = "t049_answered_once";
let mut hooks_map = hooks::HooksMap::default();
hooks_map.insert(
"escalation.answered".to_string(),
vec![hooks::HookDef {
name: marker.to_string(),
command: "true".to_string(),
}],
);
let mut app = AppState::test_default();
app.config = Arc::new(AppConfig {
hooks: hooks_map,
..Default::default()
});
let mut ctx = RequestContext::new(Arc::new(app), WorkingMode::Cmd);
let queue = Arc::new(EscalationQueue::new());
let (tx, rx) = tokio::sync::oneshot::channel();
queue.submit(EscalationRequest {
id: "esc_once".into(),
from_agent_id: "a1".into(),
from_agent_name: "explore".into(),
question: "[input] What do?".into(),
options: None,
reply_tx: tx,
});
ctx.escalation_queue = Some(queue);
// Unknown id: an error result, and no event.
let miss = handle_reply_escalation(
&mut ctx,
&json!({"escalation_id": "missing", "reply": "nope"}),
)
.unwrap();
assert_eq!(miss["status"], "error");
assert!(
hook_captures_named(marker).is_empty(),
"a failed reply must not fire escalation.answered"
);
// The successful take -> reply arm fires it exactly once.
let ok = handle_reply_escalation(
&mut ctx,
&json!({"escalation_id": "esc_once", "reply": "do X"}),
)
.unwrap();
assert_eq!(ok["status"], "ok");
assert_eq!(rx.blocking_recv().unwrap(), "do X");
let answered = hook_captures_named(marker);
assert_eq!(answered.len(), 1, "{answered:?}");
assert_eq!(answered[0].envs["COYOTE_ESCALATION_ID"], "esc_once");
assert_eq!(answered[0].envs["COYOTE_ESCALATION_REPLY"], "do X");
// Replying again to the same id hits the already-taken arm: still one.
let again = handle_reply_escalation(
&mut ctx,
&json!({"escalation_id": "esc_once", "reply": "again"}),
)
.unwrap();
assert_eq!(again["status"], "error");
assert_eq!(
hook_captures_named(marker).len(),
1,
"a second reply must not re-fire escalation.answered"
);
}
#[test] #[test]
fn handle_task_create_simple() { fn handle_task_create_simple() {
let mut ctx = ctx_with_supervisor(4, 3); let mut ctx = ctx_with_supervisor(4, 3);
+14 -18
View File
@@ -111,6 +111,18 @@ impl JobHookSnapshot {
} }
} }
fn fire_job_started(ctx: &RequestContext, job_id: &str, tool: &str) {
hooks::fire(
HookEvent::JobStarted,
ctx,
&[
("COYOTE_JOB_ID", job_id.to_string()),
("COYOTE_TOOL_NAME", tool.to_string()),
],
None,
);
}
pub struct RingBuf { pub struct RingBuf {
buf: Vec<u8>, buf: Vec<u8>,
capacity: usize, capacity: usize,
@@ -529,15 +541,7 @@ async fn handle_start(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let notify_id = job_id.clone(); let notify_id = job_id.clone();
let notify_tool = tool.clone(); let notify_tool = tool.clone();
let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool); let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool);
hooks::fire( fire_job_started(ctx, &job_id, &tool);
HookEvent::JobStarted,
ctx,
&[
("COYOTE_JOB_ID", job_id.clone()),
("COYOTE_TOOL_NAME", tool.clone()),
],
None,
);
tokio::spawn(async move { tokio::spawn(async move {
let result = run_mcp_job(job_ctx, server, inner_tool, inner_args).await; let result = run_mcp_job(job_ctx, server, inner_tool, inner_args).await;
let success = result.is_ok(); let success = result.is_ok();
@@ -559,15 +563,7 @@ async fn handle_start(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
let notify_id = job_id.clone(); let notify_id = job_id.clone();
let notify_tool = tool.clone(); let notify_tool = tool.clone();
let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool); let hook_snapshot = JobHookSnapshot::resolve(ctx, &job_id, &tool);
hooks::fire( fire_job_started(ctx, &job_id, &tool);
HookEvent::JobStarted,
ctx,
&[
("COYOTE_JOB_ID", job_id.clone()),
("COYOTE_TOOL_NAME", tool.clone()),
],
None,
);
tokio::spawn(async move { tokio::spawn(async move {
let result = run_process_job(snapshot, Arc::clone(&task_state), task_buf).await; 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 success = matches!(&result, Ok(job_result) if job_result.exit_code == Some(0));
+31 -14
View File
@@ -2694,7 +2694,7 @@ impl ToolCallTracker {
} }
} }
fn format_call_log(cmd_name: &str, cmd_args: &[String], json_data: &serde_json::Value) -> String { fn format_call_log(cmd_name: &str, cmd_args: &[String], json_data: &Value) -> String {
if *NO_COLOR { if *NO_COLOR {
return format!("Call {cmd_name} {}", cmd_args.join(" ")); return format!("Call {cmd_name} {}", cmd_args.join(" "));
} }
@@ -2713,8 +2713,8 @@ fn format_call_log(cmd_name: &str, cmd_args: &[String], json_data: &serde_json::
) )
} }
fn format_json_colored_keys(value: &serde_json::Value) -> String { fn format_json_colored_keys(value: &Value) -> String {
let serde_json::Value::Object(map) = value else { let Value::Object(map) = value else {
return dimmed_text(&value.to_string()); return dimmed_text(&value.to_string());
}; };
if map.is_empty() { if map.is_empty() {
@@ -3486,16 +3486,15 @@ mod tests {
#[test] #[test]
fn bundled_bash_tools_generate_declarations() { fn bundled_bash_tools_generate_declarations() {
let tools_dir = let tools_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/functions/tools");
std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/functions/tools");
let mut checked = Vec::new(); let mut checked = Vec::new();
for entry in std::fs::read_dir(&tools_dir).unwrap() { for entry in fs::read_dir(&tools_dir).unwrap() {
let path = entry.unwrap().path(); let path = entry.unwrap().path();
if path.extension().and_then(OsStr::to_str) != Some("sh") { if path.extension().and_then(OsStr::to_str) != Some("sh") {
continue; continue;
} }
let name = path.file_stem().unwrap().to_string_lossy().to_string(); let name = path.file_stem().unwrap().to_string_lossy().to_string();
let src = std::fs::read_to_string(&path).unwrap(); let src = fs::read_to_string(&path).unwrap();
let (_, declarations) = bash::build_bash_tool(&src, &name) let (_, declarations) = bash::build_bash_tool(&src, &name)
.unwrap_or_else(|e| panic!("bundled tool '{name}' failed to parse: {e}")); .unwrap_or_else(|e| panic!("bundled tool '{name}' failed to parse: {e}"));
assert!( assert!(
@@ -4673,7 +4672,7 @@ mod tests {
.unwrap() .unwrap()
.expect("nonzero exit must return an error payload"); .expect("nonzero exit must return an error payload");
let json: serde_json::Value = serde_json::from_str(&result).unwrap(); let json: Value = serde_json::from_str(&result).unwrap();
assert!( assert!(
json["tool_call_error"] json["tool_call_error"]
.as_str() .as_str()
@@ -4702,7 +4701,7 @@ mod tests {
.unwrap() .unwrap()
.expect("nonzero exit must return an error payload"); .expect("nonzero exit must return an error payload");
let json: serde_json::Value = serde_json::from_str(&result).unwrap(); let json: Value = serde_json::from_str(&result).unwrap();
assert!( assert!(
json["tool_call_error"] json["tool_call_error"]
.as_str() .as_str()
@@ -4741,7 +4740,7 @@ mod tests {
let elapsed = started.elapsed(); let elapsed = started.elapsed();
assert!(elapsed < Duration::from_secs(1), "took {elapsed:?}"); assert!(elapsed < Duration::from_secs(1), "took {elapsed:?}");
let json: serde_json::Value = serde_json::from_str(&result).unwrap(); let json: Value = serde_json::from_str(&result).unwrap();
assert_eq!(json["tool_call_error"], "Tool call 'bash' aborted"); assert_eq!(json["tool_call_error"], "Tool call 'bash' aborted");
} }
@@ -4823,6 +4822,24 @@ mod tests {
fn built_shims_resolve_paths_at_runtime_after_relocation() { fn built_shims_resolve_paths_at_runtime_after_relocation() {
use std::os::unix::fs::PermissionsExt; use std::os::unix::fs::PermissionsExt;
// A shim written moments ago can still be open for writing somewhere
// in the process (schedule-dependent), making exec fail with
// ETXTBSY; retry briefly instead of flaking.
fn output_with_etxtbsy_retry(command: &mut Command) -> io::Result<process::Output> {
let mut attempts = 0;
loop {
match command.output() {
Err(err)
if err.kind() == io::ErrorKind::ExecutableFileBusy && attempts < 10 =>
{
attempts += 1;
thread::sleep(Duration::from_millis(50));
}
ret => return ret,
}
}
}
let root = temp_file("-shim-relocate-", ""); let root = temp_file("-shim-relocate-", "");
let home_a = root.join("home-a"); let home_a = root.join("home-a");
let config_a = home_a.join("coyote"); let config_a = home_a.join("coyote");
@@ -4886,14 +4903,14 @@ mod tests {
return; return;
} }
let output = process::Command::new(&moved_shim) let mut command = Command::new(&moved_shim);
command
.arg("{}") .arg("{}")
.env_remove(&config_env) .env_remove(&config_env)
.env_remove(&functions_env) .env_remove(&functions_env)
.env_remove("LLM_OUTPUT") .env_remove("LLM_OUTPUT")
.env_remove("LLM_TOOL_DATA_FILE") .env_remove("LLM_TOOL_DATA_FILE");
.output() let output = output_with_etxtbsy_retry(&mut command).unwrap();
.unwrap();
let stdout = String::from_utf8_lossy(&output.stdout); let stdout = String::from_utf8_lossy(&output.stdout);
assert!( assert!(
output.status.success(), output.status.success(),
+178 -2
View File
@@ -1,5 +1,6 @@
use super::{FunctionDeclaration, JsonSchema}; use super::{FunctionDeclaration, JsonSchema};
use crate::config::RequestContext; use crate::config::RequestContext;
use crate::hooks::{self, HookEvent};
use crate::supervisor::escalation::{EscalationRequest, new_escalation_id}; use crate::supervisor::escalation::{EscalationRequest, new_escalation_id};
use crate::utils::{ACP_SERVER, HEADLESS, queue_acp_permission}; use crate::utils::{ACP_SERVER, HEADLESS, queue_acp_permission};
@@ -278,22 +279,37 @@ async fn handle_escalated(ctx: &RequestContext, action: &str, args: &Value) -> R
let escalation_id = new_escalation_id(); let escalation_id = new_escalation_id();
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
let question = format!("[{action}] {question}");
let request = EscalationRequest { let request = EscalationRequest {
id: escalation_id.clone(), id: escalation_id.clone(),
from_agent_id, from_agent_id: from_agent_id.clone(),
from_agent_name: from_agent_name.clone(), from_agent_name: from_agent_name.clone(),
question: format!("[{action}] {question}"), question: question.clone(),
options, options,
reply_tx: tx, reply_tx: tx,
}; };
root_queue.submit(request); root_queue.submit(request);
hooks::fire(
HookEvent::EscalationRaised,
ctx,
&[
("COYOTE_ESCALATION_ID", escalation_id),
("COYOTE_ESCALATION_FROM_AGENT_ID", from_agent_id),
("COYOTE_ESCALATION_FROM_AGENT_NAME", from_agent_name),
("COYOTE_ESCALATION_QUESTION", question),
],
None,
);
await_escalation_reply(rx, timeout_secs).await await_escalation_reply(rx, timeout_secs).await
} }
/// Waits for the parent's reply. `timeout_secs == 0` waits indefinitely; a /// Waits for the parent's reply. `timeout_secs == 0` waits indefinitely; a
/// dropped sender (the parent cancelled the request) resolves either way. /// dropped sender (the parent cancelled the request) resolves either way.
/// Both fallback arms resolve without touching the escalation queue, so
/// neither fires an escalation event: the request was never answered.
async fn await_escalation_reply(rx: oneshot::Receiver<String>, timeout_secs: u64) -> Result<Value> { async fn await_escalation_reply(rx: oneshot::Receiver<String>, timeout_secs: u64) -> Result<Value> {
let reply = if timeout_secs == 0 { let reply = if timeout_secs == 0 {
rx.await rx.await
@@ -346,8 +362,14 @@ fn parse_options(args: &Value) -> Result<Vec<String>> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::config::{AppConfig, AppState, WorkingMode};
use crate::function::agents::handle_agent_tool;
use crate::hooks::{HookDef, HooksMap, test_sink};
use crate::supervisor::escalation::EscalationQueue;
use serial_test::serial;
use std::fs; use std::fs;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::Arc;
#[test] #[test]
fn headless_select_returns_structured_json() { fn headless_select_returns_structured_json() {
@@ -374,6 +396,160 @@ mod tests {
assert_eq!(DEFAULT_ESCALATION_TIMEOUT_SECS, 0); assert_eq!(DEFAULT_ESCALATION_TIMEOUT_SECS, 0);
} }
fn ctx_with_escalation_hooks(marker: &str) -> RequestContext {
let mut hooks = HooksMap::default();
for event in ["escalation.raised", "escalation.answered"] {
hooks.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,
..Default::default()
});
RequestContext::new(Arc::new(app), WorkingMode::Cmd)
}
fn marker_captures(marker: &str, event: &str) -> Vec<test_sink::Capture> {
test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name == format!("{marker}-{event}"))
.collect()
}
/// `escalation.raised` (child seam) and `escalation.answered` (parent
/// seam) must describe the same origin agent for one escalation id: the
/// answered envs come from the taken queue request, never from the
/// replying context.
#[tokio::test]
#[serial]
async fn escalation_raised_and_answered_agree_on_origin_envs() {
let _sink = test_sink::install();
let marker = "esc-parity-k4w";
let queue = Arc::new(EscalationQueue::new());
let mut child_ctx = ctx_with_escalation_hooks(marker);
child_ctx.escalation_queue = Some(Arc::clone(&queue));
child_ctx.self_agent_id = Some("esc-child-1".to_string());
let mut parent_ctx = ctx_with_escalation_hooks(marker);
parent_ctx.escalation_queue = Some(Arc::clone(&queue));
let raise_args = json!({ "question": "which db?" });
let raise = handle_escalated(&child_ctx, "input", &raise_args);
let reply = async {
// Bounded poll: on the current-thread runtime the raise future
// submits before its first await, so this resolves immediately.
let mut escalation_id = None;
for _ in 0..1000 {
if let Some(entry) = queue.pending_summary().first() {
escalation_id = entry["escalation_id"].as_str().map(str::to_string);
break;
}
tokio::task::yield_now().await;
}
let escalation_id = escalation_id.expect("escalation must reach the queue");
handle_agent_tool(
&mut parent_ctx,
"agent__reply_escalation",
&json!({ "escalation_id": escalation_id, "reply": "postgres" }),
)
.await
.unwrap()
};
let (raised_result, replied) = tokio::join!(raise, reply);
assert_eq!(raised_result.unwrap()["answer"], "postgres");
assert_eq!(replied["status"], "ok");
let raised = marker_captures(marker, "escalation.raised");
let answered = marker_captures(marker, "escalation.answered");
assert_eq!(raised.len(), 1, "{raised:?}");
assert_eq!(answered.len(), 1, "{answered:?}");
for key in [
"COYOTE_ESCALATION_ID",
"COYOTE_ESCALATION_FROM_AGENT_ID",
"COYOTE_ESCALATION_FROM_AGENT_NAME",
"COYOTE_ESCALATION_QUESTION",
] {
assert!(raised[0].envs.contains_key(key), "{key} missing on raised");
assert_eq!(raised[0].envs.get(key), answered[0].envs.get(key), "{key}");
}
assert_eq!(
raised[0]
.envs
.get("COYOTE_ESCALATION_FROM_AGENT_ID")
.map(String::as_str),
Some("esc-child-1")
);
assert_eq!(
raised[0]
.envs
.get("COYOTE_ESCALATION_QUESTION")
.map(String::as_str),
Some("[input] which db?")
);
assert_eq!(
answered[0]
.envs
.get("COYOTE_ESCALATION_REPLY")
.map(String::as_str),
Some("postgres")
);
assert!(!raised[0].envs.contains_key("COYOTE_ESCALATION_REPLY"));
}
/// `COYOTE_ESCALATION_QUESTION` rides through the standard env
/// truncation: a huge question is trimmed to 2048 bytes on the raised
/// seam, never passed through whole.
#[tokio::test]
#[serial]
async fn escalation_question_env_is_truncated_to_2048_bytes() {
let _sink = test_sink::install();
let marker = "esc-trunc-p9j";
let queue = Arc::new(EscalationQueue::new());
let mut child_ctx = ctx_with_escalation_hooks(marker);
child_ctx.escalation_queue = Some(Arc::clone(&queue));
child_ctx.self_agent_id = Some("esc-child-trunc".to_string());
let raise_args = json!({ "question": "q".repeat(3000) });
let raise = handle_escalated(&child_ctx, "input", &raise_args);
let reply = async {
// Bounded poll: on the current-thread runtime the raise future
// submits before its first await, so this resolves immediately.
let mut request = None;
for _ in 0..1000 {
if let Some(entry) = queue.pending_summary().first() {
let id = entry["escalation_id"].as_str().unwrap().to_string();
request = queue.take(&id);
break;
}
tokio::task::yield_now().await;
}
let request = request.expect("escalation must reach the queue");
let _ = request.reply_tx.send("ok".to_string());
};
let (raised_result, ()) = tokio::join!(raise, reply);
assert_eq!(raised_result.unwrap()["answer"], "ok");
let raised = marker_captures(marker, "escalation.raised");
assert_eq!(raised.len(), 1, "{raised:?}");
let question = raised[0]
.envs
.get("COYOTE_ESCALATION_QUESTION")
.expect("QUESTION env must be present on escalation.raised");
assert_eq!(
question.len(),
2048,
"QUESTION env must be 2048-byte truncated"
);
assert!(question.starts_with("[input] qqq"));
}
#[tokio::test(start_paused = true)] #[tokio::test(start_paused = true)]
async fn zero_timeout_waits_indefinitely_for_the_reply() { async fn zero_timeout_waits_indefinitely_for_the_reply() {
let (tx, rx) = oneshot::channel(); let (tx, rx) = oneshot::channel();
+115 -14
View File
@@ -50,6 +50,21 @@ impl AgentNodeExecutor {
) )
.await; .await;
if let Err(e) = &result { if let Err(e) = &result {
if parent_ctx
.session_abort
.as_ref()
.is_some_and(|signal| signal.aborted_ctrlc())
{
hooks::fire(
HookEvent::AgentInterrupted,
parent_ctx,
&[
("COYOTE_AGENT_ID", agent_hook_id),
("COYOTE_AGENT_NAME", node.agent.clone()),
],
None,
);
} else {
hooks::fire( hooks::fire(
HookEvent::AgentFailed, HookEvent::AgentFailed,
parent_ctx, parent_ctx,
@@ -61,6 +76,7 @@ impl AgentNodeExecutor {
None, None,
); );
} }
}
outcome_from(node_id, node, state_manager, result) outcome_from(node_id, node, state_manager, result)
} }
} }
@@ -338,10 +354,12 @@ fn apply_state_updates(node: &AgentNode, state_manager: &mut StateManager, outpu
mod tests { mod tests {
use super::super::types::AgentNode; use super::super::types::AgentNode;
use super::*; use super::*;
use crate::config::{AppState, WorkingMode, default_max_agent_depth}; use crate::config::{AppConfig, AppState, WorkingMode, default_max_agent_depth};
use crate::hooks::{HookDef, HooksMap, test_sink};
use crate::supervisor::mailbox::{Inbox, PeerRegistry, graph_agent_id}; use crate::supervisor::mailbox::{Inbox, PeerRegistry, graph_agent_id};
use crate::testing::{install_warn_collector, warn_messages}; use crate::testing::{install_log_collector, warn_snapshot};
use serde_json::json; use serde_json::json;
use serial_test::serial;
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
@@ -416,6 +434,95 @@ mod tests {
RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd) RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd)
} }
fn ctx_with_agent_hooks(marker: &str) -> RequestContext {
let mut hooks = HooksMap::default();
for event in ["agent.interrupted", "agent.failed"] {
hooks.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,
..Default::default()
});
RequestContext::new(Arc::new(app), WorkingMode::Cmd)
}
/// A fired session signal (the user's interrupt) reclassifies a failed
/// agent node: `agent.interrupted` fires instead of `agent.failed`.
#[tokio::test]
#[serial]
async fn execute_failure_with_fired_session_abort_fires_agent_interrupted() {
let _sink = test_sink::install();
let marker = "graph-agent-int-z2f";
let mut ctx = ctx_with_agent_hooks(marker);
// Past max depth the node fails before touching agent config on disk.
ctx.current_depth = default_max_agent_depth();
let session = create_abort_signal();
session.set_ctrlc();
ctx.session_abort = Some(session);
let node = node_with("hi", None);
let mut state = manager_with(&[]);
AgentNodeExecutor::execute("test_node", &node, &mut state, &mut ctx, false)
.await
.expect_err("agent past max depth should fail");
let captures: Vec<_> = test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect();
assert_eq!(captures.len(), 1, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-agent.interrupted"));
assert_eq!(
captures[0]
.envs
.get("COYOTE_AGENT_NAME")
.map(String::as_str),
Some("test_agent")
);
assert!(
!captures[0].envs.contains_key("COYOTE_AGENT_ERROR"),
"an interruption is not a failure and carries no error env"
);
}
/// The inverse gate: a failure whose session signal never fired stays
/// `agent.failed` with its error env — the presence of a signal alone
/// must not reclassify a real failure as an interruption.
#[tokio::test]
#[serial]
async fn execute_failure_without_fired_session_abort_fires_agent_failed() {
let _sink = test_sink::install();
let marker = "graph-agent-fail-inv-x7q";
let mut ctx = ctx_with_agent_hooks(marker);
// Past max depth the node fails before touching agent config on disk.
ctx.current_depth = default_max_agent_depth();
ctx.session_abort = Some(create_abort_signal());
let node = node_with("hi", None);
let mut state = manager_with(&[]);
AgentNodeExecutor::execute("test_node", &node, &mut state, &mut ctx, false)
.await
.expect_err("agent past max depth should fail");
let captures: Vec<_> = test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect();
assert_eq!(captures.len(), 1, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-agent.failed"));
assert!(
captures[0].envs.contains_key("COYOTE_AGENT_ERROR"),
"a real failure keeps its error env"
);
}
fn retryable_node(max_attempts: u32) -> AgentNode { fn retryable_node(max_attempts: u32) -> AgentNode {
let mut node = node_with("hi", None); let mut node = node_with("hi", None);
node.max_attempts = max_attempts; node.max_attempts = max_attempts;
@@ -676,7 +783,7 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn retry_transient_warns_once_per_retried_attempt_with_the_context_chain() { async fn retry_transient_warns_once_per_retried_attempt_with_the_context_chain() {
install_warn_collector(); install_log_collector();
let mut ctx = plain_ctx(); let mut ctx = plain_ctx();
let node = retryable_node(3); let node = retryable_node(3);
let mut attempts = 0u32; let mut attempts = 0u32;
@@ -700,12 +807,9 @@ mod tests {
.expect_err("all attempts fail"); .expect_err("all attempts fail");
assert_eq!(attempts, 3); assert_eq!(attempts, 3);
let warns: Vec<String> = warn_messages() let warns: Vec<String> = warn_snapshot()
.lock() .into_iter()
.unwrap()
.iter()
.filter(|m| m.contains("'warn_capture'")) .filter(|m| m.contains("'warn_capture'"))
.cloned()
.collect(); .collect();
assert_eq!( assert_eq!(
warns.len(), warns.len(),
@@ -861,7 +965,7 @@ mod tests {
#[test] #[test]
fn outcome_from_fallback_warn_names_the_node_and_the_fallback_target() { fn outcome_from_fallback_warn_names_the_node_and_the_fallback_target() {
install_warn_collector(); install_log_collector();
let mut node = node_with("hi", None); let mut node = node_with("hi", None);
node.fallback = Some("recover".into()); node.fallback = Some("recover".into());
let mut state = manager_with(&[]); let mut state = manager_with(&[]);
@@ -875,12 +979,9 @@ mod tests {
.unwrap(); .unwrap();
assert_eq!(outcome, AgentExecutionOutcome::FellBack("recover".into())); assert_eq!(outcome, AgentExecutionOutcome::FellBack("recover".into()));
let warns: Vec<String> = warn_messages() let warns: Vec<String> = warn_snapshot()
.lock() .into_iter()
.unwrap()
.iter()
.filter(|m| m.contains("'fallback_warn_capture'")) .filter(|m| m.contains("'fallback_warn_capture'"))
.cloned()
.collect(); .collect();
assert_eq!(warns.len(), 1, "exactly one fallback warn: {warns:?}"); assert_eq!(warns.len(), 1, "exactly one fallback warn: {warns:?}");
assert!( assert!(
+8 -54
View File
@@ -11,7 +11,6 @@ use super::user_interaction::{ApprovalNodeExecutor, InputNodeExecutor};
use super::validator::{AgentValidationContext, GraphValidator}; use super::validator::{AgentValidationContext, GraphValidator};
use super::wall_clock; use super::wall_clock;
use crate::config::{AgentVariable, AgentVariables, RenderMode, RequestContext}; use crate::config::{AgentVariable, AgentVariables, RenderMode, RequestContext};
use crate::function::agents::hook_base_envs;
use crate::hooks::{self, HookEvent, ResolvedHook}; use crate::hooks::{self, HookEvent, ResolvedHook};
use crate::supervisor::mailbox::{Inbox, PeerAssignment, PeerRegistry, graph_agent_id}; use crate::supervisor::mailbox::{Inbox, PeerAssignment, PeerRegistry, graph_agent_id};
use crate::utils::{AbortSignal, wait_abort_signal, wait_user_interrupt}; use crate::utils::{AbortSignal, wait_abort_signal, wait_user_interrupt};
@@ -550,7 +549,7 @@ impl NodeHookEmitter {
hooks::fire_resolved( hooks::fire_resolved(
event, event,
resolved.to_vec(), resolved.to_vec(),
hook_base_envs( hooks::base_envs_parts(
event, event,
self.session_name.as_deref(), self.session_name.as_deref(),
self.agent_name.as_deref(), self.agent_name.as_deref(),
@@ -1133,14 +1132,12 @@ mod integration_tests {
use crate::hooks::HookDef; use crate::hooks::HookDef;
#[cfg(unix)] #[cfg(unix)]
use crate::supervisor::{JobHandle, JobResult, JobState, JobStatus, Supervisor, notification}; use crate::supervisor::{JobHandle, JobResult, JobState, JobStatus, Supervisor, notification};
use crate::utils::{create_abort_signal, get_env_name, temp_file}; use crate::utils::{create_abort_signal, temp_file};
use parking_lot::Mutex; use parking_lot::Mutex;
use serial_test::serial; use serial_test::serial;
use std::env;
use std::fs; use std::fs;
#[cfg(unix)] #[cfg(unix)]
use std::mem; use std::mem;
use std::time::{SystemTime, UNIX_EPOCH};
fn cmd_available(name: &str) -> bool { fn cmd_available(name: &str) -> bool {
which::which(name).is_ok() which::which(name).is_ok()
@@ -1268,7 +1265,7 @@ nodes:
eprintln!("skipping: bash or python3 not available"); eprintln!("skipping: bash or python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-executor-tests");
materialize_probe_agent(0.0); materialize_probe_agent(0.0);
let ws = TestWorkspace::new(); let ws = TestWorkspace::new();
ws.write_script("dispatcher.sh", "#!/bin/bash\necho '{}'\n"); ws.write_script("dispatcher.sh", "#!/bin/bash\necho '{}'\n");
@@ -1619,7 +1616,7 @@ nodes:
/// debug log, and a well-formed sibling on the same event still fires. /// debug log, and a well-formed sibling on the same event still fires.
#[tokio::test] #[tokio::test]
async fn malformed_graph_yaml_hook_is_skipped_without_breaking_the_node() { async fn malformed_graph_yaml_hook_is_skipped_without_breaking_the_node() {
crate::testing::install_warn_collector(); crate::testing::install_log_collector();
let _sink = hooks::test_sink::install(); let _sink = hooks::test_sink::install();
let mut ctx = make_ctx(); let mut ctx = make_ctx();
@@ -1660,10 +1657,7 @@ nodes:
assert_eq!(valid.len(), 1, "{valid:?}"); assert_eq!(valid.len(), 1, "{valid:?}");
assert_eq!(valid[0].envs["COYOTE_NODE_ID"], "done"); assert_eq!(valid[0].envs["COYOTE_NODE_ID"], "done");
let debugs: Vec<String> = crate::testing::debug_messages() let debugs = crate::testing::debug_snapshot();
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.clone();
assert!( assert!(
debugs debugs
.iter() .iter()
@@ -1685,7 +1679,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-executor-tests");
materialize_probe_agent(0.0); materialize_probe_agent(0.0);
let ws = TestWorkspace::new(); let ws = TestWorkspace::new();
@@ -2368,47 +2362,7 @@ nodes:
assert_eq!(events[0].event, "job_completed"); assert_eq!(events[0].event, "job_completed");
} }
struct TestConfigDirGuard { use crate::testing::TestConfigDirGuard;
key: String,
previous: Option<std::ffi::OsString>,
path: PathBuf,
}
impl TestConfigDirGuard {
fn new() -> Self {
let key = get_env_name("config_dir");
let previous = env::var_os(&key);
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = env::temp_dir().join(format!("coyote-graph-executor-tests-{unique}"));
fs::create_dir_all(&path).unwrap();
unsafe {
env::set_var(&key, &path);
}
Self {
key,
previous,
path,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
if let Some(previous) = &self.previous {
unsafe {
env::set_var(&self.key, previous);
}
} else {
unsafe {
env::remove_var(&self.key);
}
}
let _ = fs::remove_dir_all(&self.path);
}
}
const PROBE_AGENT: &str = "timeout-probe"; const PROBE_AGENT: &str = "timeout-probe";
@@ -2658,7 +2612,7 @@ nodes:
eprintln!("skipping: bash or python3 not available"); eprintln!("skipping: bash or python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-executor-tests");
materialize_probe_agent(5.0); materialize_probe_agent(5.0);
let ws = TestWorkspace::new(); let ws = TestWorkspace::new();
// The abort must land after the worker tasks exist (their retirement // The abort must land after the worker tasks exist (their retirement
+9 -51
View File
@@ -746,14 +746,12 @@ mod chain_tests {
use super::*; use super::*;
use crate::config::paths; use crate::config::paths;
use crate::config::{AppState, Role, WorkingMode}; use crate::config::{AppState, Role, WorkingMode};
use crate::utils::{AbortSignal, create_abort_signal, get_env_name, temp_file}; use crate::utils::{AbortSignal, create_abort_signal, temp_file};
use indexmap::IndexMap; use indexmap::IndexMap;
use serde_json::json; use serde_json::json;
use serial_test::serial; use serial_test::serial;
use std::env;
use std::fs; use std::fs;
use std::path::PathBuf; use std::path::PathBuf;
use std::time::{SystemTime, UNIX_EPOCH};
fn cmd_available(name: &str) -> bool { fn cmd_available(name: &str) -> bool {
which::which(name).is_ok() which::which(name).is_ok()
@@ -1739,47 +1737,7 @@ nodes:
} }
} }
struct TestConfigDirGuard { use crate::testing::TestConfigDirGuard;
key: String,
previous: Option<std::ffi::OsString>,
path: PathBuf,
}
impl TestConfigDirGuard {
fn new() -> Self {
let key = get_env_name("config_dir");
let previous = env::var_os(&key);
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = env::temp_dir().join(format!("coyote-graph-map-tests-{unique}"));
fs::create_dir_all(&path).unwrap();
unsafe {
env::set_var(&key, &path);
}
Self {
key,
previous,
path,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
if let Some(previous) = &self.previous {
unsafe {
env::set_var(&self.key, previous);
}
} else {
unsafe {
env::remove_var(&self.key);
}
}
let _ = fs::remove_dir_all(&self.path);
}
}
const PROBE_AGENT: &str = "timeout-probe"; const PROBE_AGENT: &str = "timeout-probe";
@@ -2031,7 +1989,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_probe_agent(0.0); materialize_probe_agent(0.0);
let h = Harness::new(&flagged_probe_graph(None)); let h = Harness::new(&flagged_probe_graph(None));
let peers = manual_peer("worker"); let peers = manual_peer("worker");
@@ -2057,7 +2015,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_probe_agent(0.0); materialize_probe_agent(0.0);
let ws = TestWorkspace::new(); let ws = TestWorkspace::new();
let yaml = format!( let yaml = format!(
@@ -2107,7 +2065,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_probe_agent(5.0); materialize_probe_agent(5.0);
let h = Harness::new(&flagged_probe_graph(Some(1))); let h = Harness::new(&flagged_probe_graph(Some(1)));
let (registry, assignments) = h.provision("worker", 2); let (registry, assignments) = h.provision("worker", 2);
@@ -2144,7 +2102,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_probe_agent(1.5); materialize_probe_agent(1.5);
let h = Harness::new(&flagged_probe_graph(Some(0))); let h = Harness::new(&flagged_probe_graph(Some(0)));
let peers = manual_peer("worker"); let peers = manual_peer("worker");
@@ -2167,7 +2125,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_probe_agent(0.0); materialize_probe_agent(0.0);
let h = Harness::new(&format!( let h = Harness::new(&format!(
r#" r#"
@@ -2272,7 +2230,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_echo_inputs_agent(); materialize_echo_inputs_agent();
let parent = echo_inputs_parent_graph(" inputs:\n width: \"{{n}}\"\n"); let parent = echo_inputs_parent_graph(" inputs:\n width: \"{{n}}\"\n");
@@ -2291,7 +2249,7 @@ nodes:
eprintln!("skipping: python3 not available"); eprintln!("skipping: python3 not available");
return; return;
} }
let _guard = TestConfigDirGuard::new(); let _guard = TestConfigDirGuard::new("graph-map-tests");
materialize_echo_inputs_agent(); materialize_echo_inputs_agent();
let parent = echo_inputs_parent_graph(""); let parent = echo_inputs_parent_graph("");
+1555 -212
View File
File diff suppressed because it is too large. Load diff
+99 -24
View File
@@ -58,8 +58,37 @@ use std::path::PathBuf;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use std::{env, fs, process, sync::Arc}; use std::{env, fs, process, sync::Arc};
#[tokio::main] const MAIN_STACK_SIZE: usize = 16 * 1024 * 1024;
async fn main() -> Result<()> {
/// Bootstraps the async entry point on a dedicated thread with an explicit
/// large stack instead of `#[tokio::main]`, which would poll the root
/// future on the (small, non-configurable) OS main thread. Everything else
/// matches the macro expansion: same multi-thread runtime flavor, same
/// `enable_all()` driver set (so `tokio::signal::ctrl_c()` keeps working),
/// same `block_on` of the async body, and the `Result` propagates
/// unchanged. Worker threads get the same headroom because spawned tasks
/// (graph nodes, subagents) embed the same class of giant state machine.
/// The thread is named "main" and panics are re-raised on the real main
/// thread without re-invoking the panic hook, keeping panic output and the
/// exit code identical to the direct arrangement.
fn main() -> Result<()> {
std::thread::Builder::new()
.name("main".into())
.stack_size(MAIN_STACK_SIZE)
.spawn(|| {
tokio::runtime::Builder::new_multi_thread()
.enable_all()
.thread_stack_size(MAIN_STACK_SIZE)
.build()
.expect("failed to build the tokio runtime")
.block_on(async_main())
})
.expect("failed to spawn the main bootstrap thread")
.join()
.unwrap_or_else(|panic| std::panic::resume_unwind(panic))
}
async fn async_main() -> Result<()> {
load_env_file()?; load_env_file()?;
CompleteEnv::with_factory(Cli::command).complete(); CompleteEnv::with_factory(Cli::command).complete();
let cli = Cli::parse(); let cli = Cli::parse();
@@ -113,6 +142,15 @@ async fn main() -> Result<()> {
|| cli.list_macros || cli.list_macros
|| cli.list_skills || cli.list_skills
|| cli.list_sessions; || cli.list_sessions;
let readout_flag = (cli.info
|| cli.list_models
|| cli.list_roles
|| cli.list_agents
|| cli.list_rags
|| cli.list_macros
|| cli.list_skills
|| cli.list_sessions)
&& !cli.sync_models;
let vault_flags = cli.add_secret.is_some() let vault_flags = cli.add_secret.is_some()
|| cli.get_secret.is_some() || cli.get_secret.is_some()
|| cli.update_secret.is_some() || cli.update_secret.is_some()
@@ -140,7 +178,18 @@ async fn main() -> Result<()> {
return config::list_installed_bundles(); return config::list_installed_bundles();
} }
let mcp_inspect =
cli.mcp_list && cli.mcp_get.is_none() && cli.mcp_remove.is_none() && cli.mcp_add.is_none();
let vault_inspect = cli.list_secrets
&& cli.add_secret.is_none()
&& cli.get_secret.is_none()
&& cli.update_secret.is_none()
&& cli.delete_secret.is_none();
let inspection_only =
(readout_flag || vault_inspect || mcp_inspect) && cli.agent.is_none() && cli.role.is_none();
if !inspection_only {
install_builtins()?; install_builtins()?;
}
if let Some(category) = cli.install_builtins { if let Some(category) = cli.install_builtins {
return config::install_assets(category); return config::install_assets(category);
@@ -164,7 +213,7 @@ async fn main() -> Result<()> {
} }
if let Some(client_arg) = &cli.authenticate { if let Some(client_arg) = &cli.authenticate {
let cfg = Config::load_with_interpolation(true).await?; let cfg = Config::load_with_interpolation(true, false).await?;
let app_config = AppConfig::from_config(cfg)?; let app_config = AppConfig::from_config(cfg)?;
let (client_name, provider) = let (client_name, provider) =
resolve_oauth_client(client_arg.as_deref(), &app_config.clients)?; resolve_oauth_client(client_arg.as_deref(), &app_config.clients)?;
@@ -173,7 +222,7 @@ async fn main() -> Result<()> {
} }
if let Some(server_name) = &cli.auth_mcp { if let Some(server_name) = &cli.auth_mcp {
let cfg = Config::load_with_interpolation(true).await?; let cfg = Config::load_with_interpolation(true, false).await?;
let app_config = AppConfig::from_config(cfg)?; let app_config = AppConfig::from_config(cfg)?;
let vault = Vault::init(&app_config)?; let vault = Vault::init(&app_config)?;
let mcp_path = paths::mcp_config_file(); let mcp_path = paths::mcp_config_file();
@@ -225,9 +274,17 @@ async fn main() -> Result<()> {
let mcp_action = let mcp_action =
cli.mcp_list || cli.mcp_get.is_some() || cli.mcp_remove.is_some() || cli.mcp_add.is_some(); cli.mcp_list || cli.mcp_get.is_some() || cli.mcp_remove.is_some() || cli.mcp_add.is_some();
if mcp_action { if mcp_action {
let cfg = Config::load_with_interpolation(true).await?; let cfg = Config::load_with_interpolation(true, mcp_inspect).await?;
let app_config = AppConfig::from_config(cfg)?; let app_config = if mcp_inspect {
let vault = Vault::init(&app_config)?; AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
let vault = if mcp_inspect {
Vault::init_lenient(&app_config)
} else {
Vault::init(&app_config)?
};
mcp::manage::handle(&cli, &vault)?; mcp::manage::handle(&cli, &vault)?;
@@ -235,16 +292,24 @@ async fn main() -> Result<()> {
} }
if vault_flags { if vault_flags {
let cfg = Config::load_with_interpolation(true).await?; let cfg = Config::load_with_interpolation(true, vault_inspect).await?;
let app_config = AppConfig::from_config(cfg)?; let app_config = if vault_inspect {
AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
let vault = Vault::init(&app_config)?; let vault = Vault::init(&app_config)?;
return Vault::handle_vault_flags(cli, &vault); return Vault::handle_vault_flags(cli, &vault);
} }
let abort_signal = create_abort_signal(); let abort_signal = create_abort_signal();
let start_mcp_servers = cli.agent.is_none() && cli.role.is_none(); let start_mcp_servers = cli.agent.is_none() && cli.role.is_none();
let cfg = Config::load_with_interpolation(info_flag).await?; let cfg = Config::load_with_interpolation(info_flag, readout_flag).await?;
let mut app_config = AppConfig::from_config(cfg)?; let mut app_config = if readout_flag {
AppConfig::from_config_lenient(cfg)?
} else {
AppConfig::from_config(cfg)?
};
if cli.no_workspace_mcp { if cli.no_workspace_mcp {
app_config.no_workspace_mcp = true; app_config.no_workspace_mcp = true;
} }
@@ -258,14 +323,17 @@ async fn main() -> Result<()> {
log_path, log_path,
start_mcp_servers, start_mcp_servers,
info_flag, info_flag,
inspection_only,
abort_signal.clone(), abort_signal.clone(),
) )
.await?, .await?,
); );
let mut ctx = RequestContext::bootstrap(app_state, working_mode, info_flag)?; let mut ctx = RequestContext::bootstrap(app_state, working_mode, info_flag)?;
let app_config = Arc::clone(&ctx.app.config); let app_config = Arc::clone(&ctx.app.config);
if !inspection_only {
ctx.bootstrap_tools(&app_config, start_mcp_servers, abort_signal.clone()) ctx.bootstrap_tools(&app_config, start_mcp_servers, abort_signal.clone())
.await?; .await?;
}
{ {
let app = &*ctx.app.config; let app = &*ctx.app.config;
@@ -306,9 +374,12 @@ async fn run(
} }
if cli.list_models { if cli.list_models {
for model in list_models(ctx.app.config.as_ref(), ModelType::Chat) { let models = list_models(ctx.app.config.as_ref(), ModelType::Chat)
println!("{}", model.id()); .iter()
} .map(|model| model.id())
.collect::<Vec<_>>()
.join("\n");
println!("{models}");
return Ok(()); return Ok(());
} }
if cli.list_roles { if cli.list_roles {
@@ -389,11 +460,12 @@ async fn run(
ctx.use_agent(app.as_ref(), agent, session, abort_signal.clone()) ctx.use_agent(app.as_ref(), agent, session, abort_signal.clone())
.await?; .await?;
// The top-level agent bracket does NOT open here: the inspection // The top-level agent bracket does NOT open here: the inspection
// flags below (--list-sessions, --info, --macro, --rebuild-rag, // flags below (--list-sessions, --info, --rebuild-rag, --init-*)
// --init-*) return early without running the agent, and an open // return early without running the agent, and an open bracket on
// bracket on those paths would leak `agent.started` with no // those paths would leak `agent.started` with no terminal event.
// terminal event. It opens at the dispatch sites further down, // It opens at the dispatch sites further down, where a run is
// where an agent run is actually committed. // actually committed, including the --macro arm, which executes
// rather than inspects.
} else { } else {
let app: Arc<AppConfig> = Arc::clone(&ctx.app.config); let app: Arc<AppConfig> = Arc::clone(&ctx.app.config);
if let Some(prompt) = &cli.temp_role { if let Some(prompt) = &cli.temp_role {
@@ -526,8 +598,11 @@ async fn run(
} }
} }
if let Some(name) = &cli.macro_name { if let Some(name) = &cli.macro_name {
macro_execute(&mut ctx, name, text.as_deref(), abort_signal.clone()).await?; ctx.top_level_agent_started();
return Ok(()); let result = macro_execute(&mut ctx, name, text.as_deref(), abort_signal.clone()).await;
ctx.top_level_agent_finished(result.as_ref().err(), Some(&abort_signal));
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
return result;
} }
if cli.execute && !is_repl { if cli.execute && !is_repl {
// Dispatch committed: open the top-level agent bracket only now, // Dispatch committed: open the top-level agent bracket only now,
@@ -538,7 +613,7 @@ async fn run(
shell_execute(&mut ctx, &SHELL, input, abort_signal.clone()).await shell_execute(&mut ctx, &SHELL, input, abort_signal.clone()).await
} }
.await; .await;
ctx.top_level_agent_finished(result.as_ref().err()); ctx.top_level_agent_finished(result.as_ref().err(), Some(&abort_signal));
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await; hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
return result; return result;
} }
@@ -572,7 +647,7 @@ async fn run(
start_directive(&mut ctx, input, cli.code, abort_signal.clone()).await start_directive(&mut ctx, input, cli.code, abort_signal.clone()).await
} }
.await; .await;
ctx.top_level_agent_finished(result.as_ref().err()); ctx.top_level_agent_finished(result.as_ref().err(), Some(&abort_signal));
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await; hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
result result
} }
@@ -706,7 +781,7 @@ async fn shell_execute(
if code == 0 && app.save_shell_history { if code == 0 && app.save_shell_history {
let _ = append_to_shell_history(&shell.name, &eval_str, code); let _ = append_to_shell_history(&shell.name, &eval_str, code);
} }
ctx.top_level_agent_finished(None); ctx.top_level_agent_finished(None, None);
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await; hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
process::exit(code); process::exit(code);
} }
+388 -12
View File
@@ -13,6 +13,7 @@ mod splitter;
use self::graph::{KnowledgeGraph, extract_entities}; use self::graph::{KnowledgeGraph, extract_entities};
use self::provider::RagProvider; use self::provider::RagProvider;
use self::providers::{DuckDbProvider, QdrantProvider, YamlProvider}; use self::providers::{DuckDbProvider, QdrantProvider, YamlProvider};
use crate::hooks::RagSyncHooks;
use crate::sandbox::mcp_credentials; use crate::sandbox::mcp_credentials;
use crate::vault::{Vault, interpolate_secrets}; use crate::vault::{Vault, interpolate_secrets};
@@ -144,6 +145,7 @@ impl Rag {
doc_paths: &[String], doc_paths: &[String],
config: &RagInitConfig, config: &RagInitConfig,
abort_signal: AbortSignal, abort_signal: AbortSignal,
sync_hooks: RagSyncHooks,
) -> Result<Self> { ) -> Result<Self> {
if doc_paths.is_empty() { if doc_paths.is_empty() {
bail!("Cannot build RAG knowledge base '{name}' with no documents"); bail!("Cannot build RAG knowledge base '{name}' with no documents");
@@ -154,15 +156,27 @@ impl Rag {
let mut rag = Self::create(app, name, save_path, data).await?; let mut rag = Self::create(app, name, save_path, data).await?;
let loaders = app.document_loaders.clone(); let loaders = app.document_loaders.clone();
let (spinner, spinner_rx) = Spinner::create(""); let (spinner, spinner_rx) = Spinner::create("");
abortable_run_with_spinner_rx( sync_hooks.fire_started(&rag.name, &rag.path);
let synced = abortable_run_with_spinner_rx(
rag.sync_documents(doc_paths, true, false, loaders, Some(spinner)), rag.sync_documents(doc_paths, true, false, loaders, Some(spinner)),
spinner_rx, spinner_rx,
abort_signal, abort_signal,
) )
.await?; .await;
if rag.save()? { if let Err(err) = synced {
println!("✓ Saved RAG to '{}'.", save_path.display()); sync_hooks.fire_failed(&rag.name, &rag.path, &err);
return Err(err);
} }
match rag.save() {
Ok(true) => println!("✓ Saved RAG to '{}'.", save_path.display()),
Ok(false) => {}
Err(err) => {
sync_hooks.fire_failed(&rag.name, &rag.path, &err);
return Err(err);
}
}
sync_hooks.fire_completed(&rag.name, &rag.path, rag.file_count());
Ok(rag) Ok(rag)
} }
@@ -261,6 +275,7 @@ impl Rag {
doc_paths: &[String], doc_paths: &[String],
abort_signal: AbortSignal, abort_signal: AbortSignal,
prompt_for_driver: bool, prompt_for_driver: bool,
sync_hooks: RagSyncHooks,
) -> Result<Self> { ) -> Result<Self> {
if !*IS_STDOUT_TERMINAL { if !*IS_STDOUT_TERMINAL {
bail!("Failed to init rag in non-interactive mode"); bail!("Failed to init rag in non-interactive mode");
@@ -305,15 +320,27 @@ impl Rag {
}; };
let loaders = app.document_loaders.clone(); let loaders = app.document_loaders.clone();
let (spinner, spinner_rx) = Spinner::create(""); let (spinner, spinner_rx) = Spinner::create("");
abortable_run_with_spinner_rx( sync_hooks.fire_started(&rag.name, &rag.path);
let synced = abortable_run_with_spinner_rx(
rag.sync_documents(&paths, true, false, loaders, Some(spinner)), rag.sync_documents(&paths, true, false, loaders, Some(spinner)),
spinner_rx, spinner_rx,
abort_signal, abort_signal,
) )
.await?; .await;
if rag.save()? { if let Err(err) = synced {
println!("✓ Saved RAG to '{}'.", save_path.display()); sync_hooks.fire_failed(&rag.name, &rag.path, &err);
return Err(err);
} }
match rag.save() {
Ok(true) => println!("✓ Saved RAG to '{}'.", save_path.display()),
Ok(false) => {}
Err(err) => {
sync_hooks.fire_failed(&rag.name, &rag.path, &err);
return Err(err);
}
}
sync_hooks.fire_completed(&rag.name, &rag.path, rag.file_count());
Ok(rag) Ok(rag)
} }
@@ -636,10 +663,12 @@ impl Rag {
force_reingest: bool, force_reingest: bool,
app: &AppConfig, app: &AppConfig,
abort_signal: AbortSignal, abort_signal: AbortSignal,
sync_hooks: RagSyncHooks,
) -> Result<()> { ) -> Result<()> {
let loaders = app.document_loaders.clone(); let loaders = app.document_loaders.clone();
let (spinner, spinner_rx) = Spinner::create(""); let (spinner, spinner_rx) = Spinner::create("");
abortable_run_with_spinner_rx( sync_hooks.fire_started(&self.name, &self.path);
let synced = abortable_run_with_spinner_rx(
self.sync_documents( self.sync_documents(
document_paths, document_paths,
refresh, refresh,
@@ -650,10 +679,21 @@ impl Rag {
spinner_rx, spinner_rx,
abort_signal, abort_signal,
) )
.await?; .await;
if self.save()? { if let Err(err) = synced {
println!("✓ Saved rag to '{}'.", self.path); sync_hooks.fire_failed(&self.name, &self.path, &err);
return Err(err);
} }
match self.save() {
Ok(true) => println!("✓ Saved rag to '{}'.", self.path),
Ok(false) => {}
Err(err) => {
sync_hooks.fire_failed(&self.name, &self.path, &err);
return Err(err);
}
}
sync_hooks.fire_completed(&self.name, &self.path, self.file_count());
Ok(()) Ok(())
} }
@@ -2280,6 +2320,8 @@ where
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::hooks::{HookDef, HooksMap, test_sink};
use serial_test::serial;
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::atomic::{AtomicU64, Ordering};
use std::time::{SystemTime, UNIX_EPOCH}; use std::time::{SystemTime, UNIX_EPOCH};
@@ -2313,6 +2355,340 @@ mod tests {
} }
} }
fn rag_hooks_ctx(marker: &str) -> RequestContext {
let mut hooks_map = HooksMap::default();
for event in ["rag.sync.started", "rag.sync.completed", "rag.sync.failed"] {
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::Cmd)
}
fn rag_captures(marker: &str) -> Vec<test_sink::Capture> {
test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect()
}
fn buildable_data() -> RagData {
RagData {
driver: "yaml".to_string(),
// The only embedding model resolvable in tests: the model
// registry is a process-global OnceLock pre-seeded by
// `seed_model_registries` in the request_context tests.
embedding_model: "test-seeded:test-embedder".to_string(),
chunk_size: 1000,
chunk_overlap: 100,
top_k: 5,
..Default::default()
}
}
/// An AppConfig whose client list can construct the "test-seeded"
/// embeddings client: init_with_config instantiates the client even
/// when a sync has nothing to embed.
fn seeded_client_app() -> AppConfig {
let mut client = ClientConfig::default();
if let ClientConfig::OpenAIConfig(config) = &mut client {
config.name = Some("test-seeded".to_string());
let mut embedder = ModelData::new("test-embedder");
embedder.model_type = "embedding".to_string();
config.models = vec![embedder];
}
AppConfig {
clients: vec![client],
..Default::default()
}
}
fn init_config() -> RagInitConfig {
RagInitConfig {
embedding_model: Some("test-seeded:test-embedder".to_string()),
chunk_size: Some(1000),
chunk_overlap: Some(100),
..Default::default()
}
}
fn yaml_rag(name: &str, path: &Path, data: RagData) -> Rag {
Rag {
app_config: Arc::new(AppConfig::default()),
name: name.to_string(),
path: path.display().to_string(),
embedding_model: Model::new("openai", "text-embedding-3-small"),
bm25: data.build_bm25(),
provider: Box::new(YamlProvider::from_data(&data)),
node_to_docs: IndexMap::new(),
data,
last_sources: RwLock::new(None),
}
}
#[tokio::test]
#[serial]
async fn refresh_document_paths_fires_started_then_completed_with_doc_count() {
let _sink = test_sink::install();
let marker = "rag-sync-ok-v5k";
let dir = TempDir::new("sync-hooks-ok");
let doc = dir.path.join("doc.txt");
fs::write(&doc, "hello rag hooks").unwrap();
let doc_path = doc.display().to_string();
// The prepopulated file entry must use the exact string
// sync_documents resolves, so the sync sees an unchanged corpus and
// never needs an embeddings client.
let loaders = HashMap::new();
let (_, _, _, _, local_paths) = resolve_paths(&loaders, std::slice::from_ref(&doc_path))
.await
.unwrap();
let resolved_path = local_paths.iter().next().unwrap().clone();
let mut data = buildable_data();
data.next_file_id = 1;
data.files.insert(
0,
RagFile {
hash: sha256("hello rag hooks"),
path: resolved_path,
documents: vec![],
},
);
let rag_path = dir.path.join("kb.yaml");
let rag_path_display = rag_path.display().to_string();
let mut rag = yaml_rag("kb", &rag_path, data);
let sync_hooks = RagSyncHooks::resolve(&rag_hooks_ctx(marker));
rag.refresh_document_paths(
&[doc_path],
false,
false,
&AppConfig::default(),
create_abort_signal(),
sync_hooks,
)
.await
.unwrap();
let captures = rag_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-rag.sync.started"));
assert_eq!(
captures[0].envs.get("COYOTE_RAG_NAME").map(String::as_str),
Some("kb")
);
assert_eq!(
captures[0].envs.get("COYOTE_RAG_PATH").map(String::as_str),
Some(rag_path_display.as_str())
);
assert_eq!(
captures[1].hook_name,
format!("{marker}-rag.sync.completed")
);
assert_eq!(
captures[1]
.envs
.get("COYOTE_RAG_DOC_COUNT")
.map(String::as_str),
Some("1")
);
}
#[tokio::test]
#[serial]
async fn refresh_document_paths_fires_started_then_failed_on_sync_error() {
let _sink = test_sink::install();
let marker = "rag-sync-fail-b8m";
let dir = TempDir::new("sync-hooks-fail");
let missing = dir.path.join("missing.txt").display().to_string();
let rag_path = dir.path.join("kb.yaml");
let mut rag = yaml_rag("kb", &rag_path, buildable_data());
let sync_hooks = RagSyncHooks::resolve(&rag_hooks_ctx(marker));
let result = rag
.refresh_document_paths(
&[missing],
false,
false,
&AppConfig::default(),
create_abort_signal(),
sync_hooks,
)
.await;
assert!(result.is_err(), "syncing a nonexistent document must fail");
let captures = rag_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-rag.sync.started"));
assert_eq!(captures[1].hook_name, format!("{marker}-rag.sync.failed"));
assert_eq!(
captures[1].envs.get("COYOTE_RAG_NAME").map(String::as_str),
Some("kb")
);
assert!(
captures[1]
.envs
.get("COYOTE_ERROR")
.is_some_and(|error| !error.is_empty())
);
}
#[tokio::test]
#[serial]
async fn init_with_config_fires_started_then_completed_after_save() {
let _sink = test_sink::install();
let marker = "rag-init-ok-q7t";
let dir = TempDir::new("init-hooks-ok");
// An empty document syncs without embedding anything, so the build
// needs no live embeddings endpoint.
let doc = dir.path.join("doc.txt");
fs::write(&doc, "").unwrap();
let save_path = dir.path.join("kb.yaml");
let sync_hooks = RagSyncHooks::resolve(&rag_hooks_ctx(marker));
let rag = Rag::init_with_config(
&seeded_client_app(),
"kb-init",
&save_path,
&[doc.display().to_string()],
&init_config(),
create_abort_signal(),
sync_hooks,
)
.await
.unwrap();
assert_eq!(rag.file_count(), 1);
assert!(
save_path.exists(),
"completed must only fire after the knowledge base was saved"
);
let captures = rag_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-rag.sync.started"));
assert_eq!(
captures[1].hook_name,
format!("{marker}-rag.sync.completed")
);
assert_eq!(
captures[1]
.envs
.get("COYOTE_RAG_DOC_COUNT")
.map(String::as_str),
Some("1")
);
}
#[tokio::test]
#[serial]
async fn init_with_config_fires_failed_when_save_fails() {
let _sink = test_sink::install();
let marker = "rag-init-save-fail-w2j";
let dir = TempDir::new("init-hooks-save-fail");
let doc = dir.path.join("doc.txt");
fs::write(&doc, "").unwrap();
// A regular file where save() expects a parent directory makes the
// save fail after a successful sync.
let blocker = dir.path.join("blocker");
fs::write(&blocker, "not a directory").unwrap();
let save_path = blocker.join("kb.yaml");
let sync_hooks = RagSyncHooks::resolve(&rag_hooks_ctx(marker));
let result = Rag::init_with_config(
&seeded_client_app(),
"kb-init",
&save_path,
&[doc.display().to_string()],
&init_config(),
create_abort_signal(),
sync_hooks,
)
.await;
assert!(
result.is_err(),
"an unwritable save path must fail the init"
);
let captures = rag_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-rag.sync.started"));
assert_eq!(captures[1].hook_name, format!("{marker}-rag.sync.failed"));
assert!(
captures[1]
.envs
.get("COYOTE_ERROR")
.is_some_and(|error| !error.is_empty())
);
}
// The user-abort arm in abortable_run_with_spinner_rx only exists when
// stdout is a terminal, so a unit test cannot reach it; its
// bail!("Aborted.") surfaces through the same `synced` Err arm this
// sync error drives.
#[tokio::test]
#[serial]
async fn init_with_config_fires_failed_on_sync_error() {
let _sink = test_sink::install();
let marker = "rag-init-fail-d4n";
let dir = TempDir::new("init-hooks-fail");
let missing = dir.path.join("missing.txt").display().to_string();
let save_path = dir.path.join("kb.yaml");
let sync_hooks = RagSyncHooks::resolve(&rag_hooks_ctx(marker));
let result = Rag::init_with_config(
&seeded_client_app(),
"kb-init",
&save_path,
&[missing],
&init_config(),
create_abort_signal(),
sync_hooks,
)
.await;
assert!(result.is_err(), "syncing a nonexistent document must fail");
assert!(!save_path.exists());
let captures = rag_captures(marker);
assert_eq!(captures.len(), 2, "{captures:?}");
assert_eq!(captures[0].hook_name, format!("{marker}-rag.sync.started"));
assert_eq!(captures[1].hook_name, format!("{marker}-rag.sync.failed"));
assert!(
captures[1]
.envs
.get("COYOTE_ERROR")
.is_some_and(|error| !error.is_empty())
);
}
#[tokio::test]
#[serial]
async fn pure_load_fires_no_rag_sync_events() {
let _sink = test_sink::install();
test_sink::drain();
let dir = TempDir::new("sync-hooks-load");
let path = dir.path.join("kb.yaml");
fs::write(&path, serde_yaml::to_string(&buildable_data()).unwrap()).unwrap();
let rag = Rag::load(&AppConfig::default(), "kb", &path).await.unwrap();
assert_eq!(rag.file_count(), 0);
let captures = test_sink::drain();
assert!(
captures.is_empty(),
"a pure load must dispatch no hooks: {captures:?}"
);
}
#[test] #[test]
fn attached_ragdata_serializes_driver_and_attached() { fn attached_ragdata_serializes_driver_and_attached() {
let mut data = RagData { let mut data = RagData {
+251 -12
View File
@@ -19,6 +19,7 @@ use crate::config::{AssetCategory, paths};
use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail}; use crate::function::agents::{GuardrailAction, check_pending_tasks_guardrail};
use crate::hooks::{self, HookEvent}; use crate::hooks::{self, HookEvent};
use crate::render::render_error; use crate::render::render_error;
use crate::supervisor::Supervisor;
use crate::utils::{ use crate::utils::{
AbortSignal, SHELL, abortable_run_with_spinner, create_abort_signal, dimmed_text, AbortSignal, SHELL, abortable_run_with_spinner, create_abort_signal, dimmed_text,
drain_stale_tty_input, run_command, set_text, temp_file, drain_stale_tty_input, run_command, set_text, temp_file,
@@ -67,7 +68,7 @@ pub(crate) struct TurnBracket(());
impl TurnBracket { impl TurnBracket {
#[must_use] #[must_use]
pub(crate) fn enter(ctx: &RequestContext) -> Self { pub(crate) fn enter(ctx: &RequestContext) -> Self {
hooks::fire(HookEvent::TurnStarted, ctx, &[], None); hooks::fire(HookEvent::TurnStarted, ctx, &hooks::role_extras(ctx), None);
Self(()) Self(())
} }
@@ -78,16 +79,25 @@ impl TurnBracket {
result: Result<T>, result: Result<T>,
) -> Result<T> { ) -> Result<T> {
if abort_signal.aborted_ctrlc() { if abort_signal.aborted_ctrlc() {
hooks::fire(HookEvent::TurnInterrupted, ctx, &[], None); hooks::fire(
HookEvent::TurnInterrupted,
ctx,
&hooks::role_extras(ctx),
None,
);
} else { } else {
match &result { match &result {
Ok(_) => hooks::fire(HookEvent::TurnCompleted, ctx, &[], None), Ok(_) => hooks::fire(
Err(err) => hooks::fire( HookEvent::TurnCompleted,
HookEvent::TurnFailed,
ctx, ctx,
&[("COYOTE_ERROR", format!("{err:#}"))], &hooks::role_extras(ctx),
None, None,
), ),
Err(err) => {
let mut extras = hooks::role_extras(ctx);
extras.push(("COYOTE_ERROR", format!("{err:#}")));
hooks::fire(HookEvent::TurnFailed, ctx, &extras, None);
}
} }
} }
result result
@@ -512,9 +522,8 @@ Type ".help" for additional help.
} }
} }
Ok(Signal::CtrlC) => { Ok(Signal::CtrlC) => {
self.abort_signal.set_ctrlc();
if let Some(supervisor) = self.ctx.read().supervisor.clone() { if let Some(supervisor) = self.ctx.read().supervisor.clone() {
supervisor.read().cancel_recursive(); latch_prompt_interrupt(&self.abort_signal, &supervisor);
} }
println!("(To exit, press Ctrl+D or enter \".exit\")\n"); println!("(To exit, press Ctrl+D or enter \".exit\")\n");
} }
@@ -533,7 +542,7 @@ Type ".help" for additional help.
let exit_result = self.ctx.write().exit_session(); let exit_result = self.ctx.write().exit_session();
self.ctx self.ctx
.write() .write()
.top_level_agent_finished(exit_result.as_ref().err()); .top_level_agent_finished(exit_result.as_ref().err(), Some(&self.abort_signal));
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await; hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
exit_result exit_result
} }
@@ -1143,7 +1152,12 @@ pub async fn run_repl_command(
abort_signal.clone(), abort_signal.clone(),
) )
.await?; .await?;
hooks::fire(HookEvent::SessionCompressed, ctx, &[], None); hooks::fire(
HookEvent::SessionCompressed,
ctx,
&hooks::role_extras(ctx),
None,
);
println!("✓ Successfully compressed the session."); println!("✓ Successfully compressed the session.");
} }
_ => { _ => {
@@ -1697,7 +1711,12 @@ fn print_turn_divider() {
async fn auto_compress_session(ctx: &mut RequestContext) { async fn auto_compress_session(ctx: &mut RequestContext) {
match ctx.compress_session().await { match ctx.compress_session().await {
Ok(()) => hooks::fire(HookEvent::SessionCompressed, ctx, &[], None), Ok(()) => hooks::fire(
HookEvent::SessionCompressed,
ctx,
&hooks::role_extras(ctx),
None,
),
Err(err) => warn!("Failed to compress the session: {err}"), Err(err) => warn!("Failed to compress the session: {err}"),
} }
} }
@@ -1715,6 +1734,14 @@ fn reset_continuation(ctx: &mut RequestContext) {
ctx.reset_continuation_count(); ctx.reset_continuation_count();
} }
pub(crate) fn latch_prompt_interrupt(abort_signal: &AbortSignal, supervisor: &RwLock<Supervisor>) {
let supervisor = supervisor.read();
if supervisor.has_active_tasks() {
abort_signal.set_ctrlc();
}
supervisor.cancel_recursive();
}
fn pause_banner_text(ctx: &RequestContext) -> Option<String> { fn pause_banner_text(ctx: &RequestContext) -> Option<String> {
let reason = ctx.auto_continue_paused.as_deref()?; let reason = ctx.auto_continue_paused.as_deref()?;
if !ctx.todo_list.has_incomplete() { if !ctx.todo_list.has_incomplete() {
@@ -2010,7 +2037,7 @@ pub fn split_args_text(line: &str, is_win: bool) -> (Vec<String>, &str) {
mod tests { mod tests {
use super::*; use super::*;
use crate::client::{ClientConfig, Model}; use crate::client::{ClientConfig, Model};
use crate::config::{AppState, RoleLike, Session, WorkingMode}; use crate::config::{AppState, Role, RoleLike, Session, TEMP_ROLE_NAME, WorkingMode};
use crate::hooks::{HookDef, HooksMap, test_sink}; use crate::hooks::{HookDef, HooksMap, test_sink};
use anyhow::anyhow; use anyhow::anyhow;
use serial_test::serial; use serial_test::serial;
@@ -2157,6 +2184,117 @@ mod tests {
assert_eq!(turn_counts(marker), (1, 0, 1, 0)); assert_eq!(turn_counts(marker), (1, 0, 1, 0));
} }
#[tokio::test]
#[serial]
async fn turn_events_carry_the_role_a_session_holds() {
let _sink = test_sink::install();
let marker = "tb-role-m3x";
let mut ctx = ctx_with_hooks(&TURN_EVENTS, marker);
// The role lives on the session (as after `.role` inside a session);
// fire-time resolution must find it there.
let mut session = Session::default();
session.set_role(Role::new("dev-role", "Prompt"));
ctx.session = Some(session);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = Ok(());
bracket.finish(&ctx, &abort_signal, result).unwrap();
let captures: Vec<_> = test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect();
assert_eq!(captures.len(), 2, "{captures:?}");
for capture in &captures {
assert_eq!(
capture.envs.get("COYOTE_ROLE").map(String::as_str),
Some("dev-role"),
"{} must carry the session-held role",
capture.hook_name
);
}
}
#[tokio::test]
#[serial]
async fn turn_interrupted_carries_the_role_a_session_holds() {
let _sink = test_sink::install();
let marker = "tb-role-int-f7c";
let mut ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let mut session = Session::default();
session.set_role(Role::new("dev-role", "Prompt"));
ctx.session = Some(session);
let abort_signal = create_abort_signal();
abort_signal.set_ctrlc();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = Ok(());
bracket.finish(&ctx, &abort_signal, result).unwrap();
let interrupted = test_sink::snapshot()
.into_iter()
.find(|capture| capture.hook_name == format!("{marker}-turn.interrupted"))
.expect("turn.interrupted capture");
assert_eq!(
interrupted.envs.get("COYOTE_ROLE").map(String::as_str),
Some("dev-role"),
"turn.interrupted must carry the session-held role"
);
}
#[tokio::test]
#[serial]
async fn turn_failed_carries_the_role_a_session_holds() {
let _sink = test_sink::install();
let marker = "tb-role-fail-z2n";
let mut ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let mut session = Session::default();
session.set_role(Role::new("dev-role", "Prompt"));
ctx.session = Some(session);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = Err(anyhow!("turn exploded"));
bracket.finish(&ctx, &abort_signal, result).unwrap_err();
let failed = test_sink::snapshot()
.into_iter()
.find(|capture| capture.hook_name == format!("{marker}-turn.failed"))
.expect("turn.failed capture");
assert_eq!(
failed.envs.get("COYOTE_ROLE").map(String::as_str),
Some("dev-role"),
"turn.failed must carry the session-held role"
);
}
#[tokio::test]
#[serial]
async fn turn_events_omit_the_role_var_when_no_role_is_active() {
let _sink = test_sink::install();
let marker = "tb-norole-q8j";
let ctx = ctx_with_hooks(&TURN_EVENTS, marker);
let abort_signal = create_abort_signal();
let bracket = TurnBracket::enter(&ctx);
let result: Result<()> = Ok(());
bracket.finish(&ctx, &abort_signal, result).unwrap();
let captures: Vec<_> = test_sink::snapshot()
.into_iter()
.filter(|capture| capture.hook_name.starts_with(marker))
.collect();
assert_eq!(captures.len(), 2, "{captures:?}");
for capture in &captures {
assert!(
!capture.envs.contains_key("COYOTE_ROLE"),
"{} must omit COYOTE_ROLE without an active role",
capture.hook_name
);
}
}
/// Drives a deep REPL future to completion on a thread with extra stack /// Drives a deep REPL future to completion on a thread with extra stack
/// headroom: nested `run_repl_command`/`compress_session` poll frames /// headroom: nested `run_repl_command`/`compress_session` poll frames
/// are deep in debug builds and overflow the default test-thread stack /// are deep in debug builds and overflow the default test-thread stack
@@ -2304,6 +2442,48 @@ mod tests {
); );
} }
#[test]
#[serial]
fn session_compressed_carries_the_active_role_name() {
let _sink = test_sink::install();
let marker = "compress-role-t6b";
run_async(async {
let mut ctx = compressible_ctx(marker);
// A temp role (`--prompt`) is a real role for COYOTE_ROLE.
ctx.role = Some(Role::new(TEMP_ROLE_NAME, "Session role prompt"));
auto_compress_session(&mut ctx).await;
});
let capture = test_sink::snapshot()
.into_iter()
.find(|capture| capture.hook_name == format!("{marker}-session.compressed"))
.expect("session.compressed capture");
assert_eq!(
capture.envs.get("COYOTE_ROLE").map(String::as_str),
Some(TEMP_ROLE_NAME)
);
}
#[test]
#[serial]
fn session_compressed_omits_the_role_var_when_no_role_is_active() {
let _sink = test_sink::install();
let marker = "compress-norole-p9c";
run_async(async {
let mut ctx = compressible_ctx(marker);
auto_compress_session(&mut ctx).await;
});
let capture = test_sink::snapshot()
.into_iter()
.find(|capture| capture.hook_name == format!("{marker}-session.compressed"))
.expect("session.compressed capture");
assert!(
!capture.envs.contains_key("COYOTE_ROLE"),
"session.compressed must omit COYOTE_ROLE without an active role"
);
}
#[test] #[test]
#[serial] #[serial]
fn auto_compress_failure_fires_no_session_compressed_and_swallows_error() { fn auto_compress_failure_fires_no_session_compressed_and_swallows_error() {
@@ -2898,4 +3078,63 @@ mod tests {
assert_eq!(pause_banner_text(&ctx), None); assert_eq!(pause_banner_text(&ctx), None);
} }
#[test]
fn latch_prompt_interrupt_stays_clear_at_idle_prompt() {
let abort = create_abort_signal();
let supervisor = RwLock::new(Supervisor::new(4, 3));
latch_prompt_interrupt(&abort, &supervisor);
assert!(
!abort.aborted_ctrlc(),
"ctrl-c with nothing to cancel must not latch as an interruption"
);
}
#[test]
fn latch_prompt_interrupt_latches_when_cancelling_active_children() {
use crate::supervisor::mailbox::Inbox;
use crate::supervisor::{AgentExitStatus, AgentHandle, AgentResult};
// Keep the runtime alive so the spawned task is never polled and the
// child counts as running.
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap();
let join_handle = rt.spawn(async {
Ok(AgentResult {
id: "done".into(),
agent_name: "test".into(),
output: "result".into(),
exit_status: AgentExitStatus::Completed,
})
});
let child_signal = create_abort_signal();
let mut sup = Supervisor::new(4, 3);
sup.register(AgentHandle {
id: "a1".to_string(),
agent_name: "explore".to_string(),
depth: 1,
inbox: Arc::new(Inbox::new()),
abort_signal: child_signal.clone(),
join_handle,
child_supervisor: None,
})
.unwrap();
let supervisor = RwLock::new(sup);
let abort = create_abort_signal();
latch_prompt_interrupt(&abort, &supervisor);
assert!(
abort.aborted_ctrlc(),
"cancelling live work is an interruption"
);
assert!(
child_signal.aborted_ctrlc(),
"the child must still be cancelled"
);
}
} }
+53
View File
@@ -197,6 +197,11 @@ pub fn discover() -> Result<Vec<DiscoveredMixin>> {
) { ) {
out.push(read_mixin(path)?); out.push(read_mixin(path)?);
} }
for agent_dir in subdirs_of(&paths::agents_data_dir()) {
for path in collect_mixins(&agent_dir.join("hooks"), &[ScanMode::Flat]) {
out.push(read_mixin(path)?);
}
}
for path in collect_mixins(&paths::rags_dir(), &[ScanMode::Flat]) { for path in collect_mixins(&paths::rags_dir(), &[ScanMode::Flat]) {
out.push(read_mixin(path)?); out.push(read_mixin(path)?);
} }
@@ -206,6 +211,9 @@ pub fn discover() -> Result<Vec<DiscoveredMixin>> {
for path in collect_mixins(&paths::roles_dir().join("hooks"), &[ScanMode::Flat]) { for path in collect_mixins(&paths::roles_dir().join("hooks"), &[ScanMode::Flat]) {
out.push(read_mixin(path)?); out.push(read_mixin(path)?);
} }
for path in collect_mixins(&paths::hooks_dir(), &[ScanMode::Flat]) {
out.push(read_mixin(path)?);
}
if let Ok(cwd) = env::current_dir() if let Ok(cwd) = env::current_dir()
&& let Some(path) = paths::find_workspace_sbx_mixin(&cwd) && let Some(path) = paths::find_workspace_sbx_mixin(&cwd)
@@ -1005,6 +1013,51 @@ network:
let _ = fs::remove_dir_all(&root); let _ = fs::remove_dir_all(&root);
} }
#[test]
#[serial_test::serial]
fn discover_scans_global_and_agent_hooks_mixins_but_never_the_manifest() {
use crate::config::builtin_manifest::BUILTIN_MANIFEST_FILE;
use crate::utils::get_env_name;
let root = unique_root("discover-hooks-dirs");
let hooks = root.join("hooks");
let agent_hooks = root.join("agents").join("researcher").join("hooks");
fs::create_dir_all(&hooks).unwrap();
fs::create_dir_all(&agent_hooks).unwrap();
let global_mixin = hooks.join("notify.sbx-mixin.yaml");
let agent_mixin = agent_hooks.join("guard.sbx-mixin.yaml");
fs::write(&global_mixin, "kind: mixin\n").unwrap();
fs::write(&agent_mixin, "kind: mixin\n").unwrap();
// The builtin installer's hidden manifest lives in both hooks dirs
// but is not a mixin and must never be picked up.
fs::write(hooks.join(BUILTIN_MANIFEST_FILE), "notify.sh\n").unwrap();
fs::write(agent_hooks.join(BUILTIN_MANIFEST_FILE), "guard.sh\n").unwrap();
let _config = crate::testing::EnvVarGuard::set(get_env_name("config_dir"), &root);
let found: Vec<PathBuf> = discover()
.unwrap()
.into_iter()
.map(|mixin| mixin.path)
.collect();
assert!(
found.contains(&global_mixin),
"missing global hooks mixin: {found:?}"
);
assert!(
found.contains(&agent_mixin),
"missing agent hooks mixin: {found:?}"
);
assert!(
found.iter().all(|path| path
.file_name()
.is_none_or(|name| name != BUILTIN_MANIFEST_FILE)),
"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 /// `generate_rag_sbx_mixin` writes an agent-scoped RAG sidecar next to the
/// rag yaml, at `<agents>/<agent>/<rag>.sbx-mixin.yaml`. Before `SubdirFlat` /// rag yaml, at `<agents>/<agent>/<rag>.sbx-mixin.yaml`. Before `SubdirFlat`
/// existed, nothing scanned that shape and attaching a Qdrant RAG from /// existed, nothing scanned that shape and attaching a Qdrant RAG from
+90
View File
@@ -190,6 +190,17 @@ impl Supervisor {
.count() .count()
} }
pub fn has_active_tasks(&self) -> bool {
self.effective_active_count() > 0
|| self.active_job_count() > 0
|| self.agents().any(|agent| {
agent
.child_supervisor
.as_ref()
.is_some_and(|child_sup| child_sup.read().has_active_tasks())
})
}
pub fn max_concurrent(&self) -> usize { pub fn max_concurrent(&self) -> usize {
self.max_concurrent self.max_concurrent
} }
@@ -449,6 +460,85 @@ mod tests {
assert!(result.unwrap_err().to_string().contains("at capacity")); assert!(result.unwrap_err().to_string().contains("at capacity"));
} }
#[test]
fn has_active_tasks_false_when_empty_or_all_finished() {
let mut sup = Supervisor::new(4, 3);
assert!(!sup.has_active_tasks());
// make_handle drops its runtime, aborting the task, so the
// registered agent is already finished.
sup.register(make_handle("a1", "explore", 1)).unwrap();
assert!(!sup.has_active_tasks());
}
#[test]
fn has_active_tasks_true_with_running_agent() {
// Keep the runtime alive so the task is never polled and the agent
// counts as running.
let rt = Builder::new_current_thread().enable_all().build().unwrap();
let join_handle = rt.spawn(async {
Ok::<AgentResult, Error>(AgentResult {
id: "done".into(),
agent_name: "test".into(),
output: "result".into(),
exit_status: AgentExitStatus::Completed,
})
});
let running_handle = AgentHandle {
id: "a1".to_string(),
agent_name: "explore".to_string(),
depth: 1,
inbox: Arc::new(Inbox::new()),
abort_signal: create_abort_signal(),
join_handle,
child_supervisor: None,
};
let mut sup = Supervisor::new(4, 3);
sup.register(running_handle).unwrap();
assert!(sup.has_active_tasks());
}
#[test]
fn has_active_tasks_true_with_running_job() {
let mut sup = Supervisor::new(4, 3).with_max_concurrent_jobs(1);
sup.register(make_job("j1", create_abort_signal())).unwrap();
assert!(sup.has_active_tasks());
}
#[test]
fn has_active_tasks_true_with_only_grandchild_activity() {
// Keep the runtime alive so the grandchild task is never polled and
// counts as running.
let rt = Builder::new_current_thread().enable_all().build().unwrap();
let join_handle = rt.spawn(async {
Ok::<AgentResult, Error>(AgentResult {
id: "done".into(),
agent_name: "test".into(),
output: "result".into(),
exit_status: AgentExitStatus::Completed,
})
});
let grandchild = AgentHandle {
id: "g1".to_string(),
agent_name: "worker".to_string(),
depth: 2,
inbox: Arc::new(Inbox::new()),
abort_signal: create_abort_signal(),
join_handle,
child_supervisor: None,
};
let mut child_sup = Supervisor::new(4, 3);
child_sup.register(grandchild).unwrap();
// make_handle's direct task is already finished, so the only live
// work sits one supervisor level down.
let mut parent_handle = make_handle("a1", "explore", 1);
parent_handle.child_supervisor = Some(Arc::new(RwLock::new(child_sup)));
let mut sup = Supervisor::new(4, 3);
sup.register(parent_handle).unwrap();
assert!(sup.has_active_tasks());
}
#[test] #[test]
fn supervisor_register_rejects_exceeding_depth() { fn supervisor_register_rejects_exceeding_depth() {
let mut sup = Supervisor::new(4, 2); let mut sup = Supervisor::new(4, 2);
+112 -17
View File
@@ -1,6 +1,7 @@
//! Test-only helpers shared across modules' unit tests. //! Test-only helpers shared across modules' unit tests.
use log::{Level, LevelFilter, Log, Metadata, Record}; use log::{Level, LevelFilter, Log, Metadata, Record};
use std::ffi::{OsStr, OsString};
use std::sync::{Mutex, Once, OnceLock}; use std::sync::{Mutex, Once, OnceLock};
struct TestLogCollector; struct TestLogCollector;
@@ -8,25 +9,30 @@ struct TestLogCollector;
static WARN_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new(); static WARN_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
static DEBUG_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new(); static DEBUG_MESSAGES: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
/// Debug capture is restricted to the hooks module so the buffer is not /// Debug capture is restricted to the modules whose tests assert on debug
/// flooded by debug output from the rest of the crate. Matching is /// output so the buffer is not flooded by the rest of the crate. Matching is
/// module-boundary-aware: only this exact module or its `::` submodules /// module-boundary-aware: only these exact modules or their `::` submodules
/// qualify, so a sibling like `hooks_registry` would not. /// qualify, so a sibling like `hooks_registry` would not.
const DEBUG_TARGET_PREFIX: &str = concat!(env!("CARGO_CRATE_NAME"), "::hooks"); const DEBUG_TARGET_PREFIXES: [&str; 2] = [
concat!(env!("CARGO_CRATE_NAME"), "::hooks"),
concat!(env!("CARGO_CRATE_NAME"), "::config::agent"),
];
fn captures_warn(metadata: &Metadata) -> bool { fn captures_warn(metadata: &Metadata) -> bool {
metadata.level() <= Level::Warn metadata.level() <= Level::Warn
} }
fn captures_hooks_debug(metadata: &Metadata) -> bool { fn captures_module_debug(metadata: &Metadata) -> bool {
if metadata.level() != Level::Debug { if metadata.level() != Level::Debug {
return false; return false;
} }
let target = metadata.target(); let target = metadata.target();
target == DEBUG_TARGET_PREFIX DEBUG_TARGET_PREFIXES.iter().any(|prefix| {
target == *prefix
|| target || target
.strip_prefix(DEBUG_TARGET_PREFIX) .strip_prefix(prefix)
.is_some_and(|rest| rest.starts_with("::")) .is_some_and(|rest| rest.starts_with("::"))
})
} }
/// Every warn- or error-level message captured since the collector was /// Every warn- or error-level message captured since the collector was
@@ -37,16 +43,35 @@ pub(crate) fn warn_messages() -> &'static Mutex<Vec<String>> {
WARN_MESSAGES.get_or_init(Mutex::default) WARN_MESSAGES.get_or_init(Mutex::default)
} }
/// Every debug-level message from the hooks module captured since the /// Every debug-level message from the [`DEBUG_TARGET_PREFIXES`] modules
/// collector was installed. Same marker-filtering discipline as /// captured since the collector was installed. Same marker-filtering
/// [`warn_messages`]. /// discipline as [`warn_messages`].
pub(crate) fn debug_messages() -> &'static Mutex<Vec<String>> { pub(crate) fn debug_messages() -> &'static Mutex<Vec<String>> {
DEBUG_MESSAGES.get_or_init(Mutex::default) DEBUG_MESSAGES.get_or_init(Mutex::default)
} }
/// A copy of everything in [`warn_messages`], recovering from poisoning the
/// same way the collector itself does.
pub(crate) fn warn_snapshot() -> Vec<String> {
snapshot_of(warn_messages())
}
/// A copy of everything in [`debug_messages`], recovering from poisoning the
/// same way the collector itself does.
pub(crate) fn debug_snapshot() -> Vec<String> {
snapshot_of(debug_messages())
}
fn snapshot_of(buffer: &Mutex<Vec<String>>) -> Vec<String> {
buffer
.lock()
.unwrap_or_else(|poisoned| poisoned.into_inner())
.clone()
}
impl Log for TestLogCollector { impl Log for TestLogCollector {
fn enabled(&self, metadata: &Metadata) -> bool { fn enabled(&self, metadata: &Metadata) -> bool {
captures_warn(metadata) || captures_hooks_debug(metadata) captures_warn(metadata) || captures_module_debug(metadata)
} }
// A test that panics while holding a buffer poisons its mutex; the logger // A test that panics while holding a buffer poisons its mutex; the logger
@@ -57,7 +82,7 @@ impl Log for TestLogCollector {
.lock() .lock()
.unwrap_or_else(|poisoned| poisoned.into_inner()) .unwrap_or_else(|poisoned| poisoned.into_inner())
.push(record.args().to_string()); .push(record.args().to_string());
} else if captures_hooks_debug(record.metadata()) { } else if captures_module_debug(record.metadata()) {
debug_messages() debug_messages()
.lock() .lock()
.unwrap_or_else(|poisoned| poisoned.into_inner()) .unwrap_or_else(|poisoned| poisoned.into_inner())
@@ -68,15 +93,85 @@ impl Log for TestLogCollector {
fn flush(&self) {} fn flush(&self) {}
} }
/// Installs the process-wide log collector. `log::set_logger` accepts one /// Installs the process-wide log collector capturing warn-level messages and
/// logger per process, so every test that captures warns or hooks debug /// debug messages from the [`DEBUG_TARGET_PREFIXES`] modules.
/// output must install through this shared entry point. The max level is /// `log::set_logger` accepts one logger per process, so every test that
/// Debug so the hooks debug capture sees its records; warn capture is /// captures either stream must install through this shared entry point. The
/// max level is Debug so the debug capture sees its records; warn capture is
/// unaffected. /// unaffected.
pub(crate) fn install_warn_collector() { pub(crate) fn install_log_collector() {
static INSTALL: Once = Once::new(); static INSTALL: Once = Once::new();
INSTALL.call_once(|| { INSTALL.call_once(|| {
log::set_logger(&TestLogCollector).expect("no other logger should be installed"); log::set_logger(&TestLogCollector).expect("no other logger should be installed");
log::set_max_level(LevelFilter::Debug); log::set_max_level(LevelFilter::Debug);
}); });
} }
/// Sets (or removes) an environment variable for the guard's lifetime and
/// restores the previous value on drop, including on panic, so a failing
/// assertion cannot leak the override into subsequent tests. Tests using it
/// must serialize (`#[serial]`) — the process environment is global.
pub(crate) struct EnvVarGuard {
key: String,
previous: Option<OsString>,
}
impl EnvVarGuard {
pub(crate) fn set(key: impl Into<String>, value: impl AsRef<OsStr>) -> Self {
let key = key.into();
let previous = std::env::var_os(&key);
unsafe {
std::env::set_var(&key, value);
}
Self { key, previous }
}
pub(crate) fn unset(key: impl Into<String>) -> Self {
let key = key.into();
let previous = std::env::var_os(&key);
unsafe {
std::env::remove_var(&key);
}
Self { key, previous }
}
}
impl Drop for EnvVarGuard {
fn drop(&mut self) {
unsafe {
match &self.previous {
Some(previous) => std::env::set_var(&self.key, previous),
None => std::env::remove_var(&self.key),
}
}
}
}
/// Points the config dir at a fresh temp directory for the guard's lifetime
/// and removes it on drop, including on panic. Tests using it must serialize
/// (`#[serial]`) — the config-dir env var is process-global.
pub(crate) struct TestConfigDirGuard {
_env: EnvVarGuard,
pub(crate) path: std::path::PathBuf,
}
impl TestConfigDirGuard {
pub(crate) fn new(label: &str) -> Self {
let unique = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = std::env::temp_dir().join(format!("coyote-{label}-{unique}"));
std::fs::create_dir_all(&path).unwrap();
Self {
_env: EnvVarGuard::set(crate::utils::get_env_name("config_dir"), &path),
path,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.path);
}
}
+25
View File
@@ -332,8 +332,14 @@ impl Vault {
} }
if cli.list_secrets { if cli.list_secrets {
if vault.sandbox_mode {
println!(
"The vault is unavailable in sandbox mode. Use `coyote --list-secrets` on your host."
);
} else {
vault.list_secrets(true)?; vault.list_secrets(true)?;
} }
}
Ok(()) Ok(())
} }
@@ -410,4 +416,23 @@ mod tests {
} }
} }
} }
#[test]
fn list_secrets_flag_ok_in_sandbox() {
use clap::Parser;
// Built directly rather than via IS_SANDBOX so this test cannot race
// the env juggling in vault_disabled_in_sandbox.
let vault = Vault {
sandbox_mode: true,
..Vault::default()
};
let cli = Cli::try_parse_from(["coyote", "--list-secrets"]).unwrap();
// Pins the --list-secrets sandbox contract: rc=0 with an informational
// readout instead of the hard failure the mutating flags keep. The
// message goes straight to stdout, so only Ok is asserted here.
Vault::handle_vault_flags(cli, &vault)
.expect("--list-secrets must succeed in sandbox mode");
}
} }
+440
View File
@@ -0,0 +1,440 @@
//! Binary-level contract for inspection flags on a pristine config dir:
//! exit 0 and NOTHING written into the config dir (no first-run wizard,
//! no builtins bootstrap). Only --info guarantees output on stdout; the
//! list flags legitimately emit an empty listing on a pristine dir, so
//! their contract is exit 0 + zero writes. Vault-backed flags like
//! --list-secrets depend on host state and are covered by
//! scripts/usage-probe-inspection.sh instead. Agent runs, by contrast,
//! must still bootstrap builtins even when combined with --info — see
//! agent_info_on_empty_config_dir_bootstraps_builtins. Flags that write,
//! like --sync-models, are NOT inspection flags and must stay on the
//! bootstrap path — see sync_models_stays_on_bootstrap_path.
//!
//! Every test runs against a fresh fake HOME and (unless sandbox mode is the
//! subject under test) a scrubbed IS_SANDBOX/provider env, so host state —
//! a real ~/.coyote_password or a sandbox shell — cannot mask
//! virgin-environment regressions.
use std::env;
use std::fs;
use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::time::{SystemTime, UNIX_EPOCH};
fn fresh_config_dir(label: &str) -> PathBuf {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = env::temp_dir().join(format!("coyote-inspection-{label}-{unique}"));
fs::create_dir_all(&tmp_dir).unwrap();
tmp_dir
}
fn probe_inspection_flag(flag: &str, require_stdout: bool) {
let label = flag.trim_start_matches("--");
let tmp_dir = fresh_config_dir(label);
let home_dir = fresh_config_dir(&format!("{label}-home"));
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg(flag)
.env("COYOTE_CONFIG_DIR", &tmp_dir)
// A fresh fake HOME hides any real ~/.coyote_password on the host;
// USERPROFILE is the best-effort Windows equivalent (dirs resolves
// home via the Known Folder API there). HOME is set, never removed:
// dirs::home_dir() panics inside gman when home can't be determined.
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
// A leaked sandbox/provider env would reroute the vault and config
// paths under test.
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
// Workspace mcp.json discovery resolves from the CWD, so pin it to
// the isolated fake HOME to keep host state out of the probe.
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let leftover: Vec<_> = fs::read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
assert!(
!home_dir.join(".coyote_password").exists(),
"{flag}: the lenient inspection path must never bootstrap a password file"
);
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
output.status.success(),
"{flag}: expected exit 0 on an empty config dir, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
if require_stdout {
assert!(
!output.stdout.is_empty(),
"{flag}: expected some output on stdout"
);
}
assert!(
leftover.is_empty(),
"{flag}: inspection flags must write nothing into the config dir: {leftover:?}"
);
}
#[test]
fn info_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--info", true);
}
// List flags on a pristine dir correctly print an empty listing, so the
// contract is exit 0 + zero writes only (no stdout assertion).
#[test]
fn list_models_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--list-models", false);
}
#[test]
fn list_agents_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--list-agents", false);
}
#[test]
fn mcp_list_on_empty_config_dir_writes_nothing() {
probe_inspection_flag("--mcp-list", false);
}
// The wizard bypass must not swallow the builtins bootstrap for agent runs:
// `--agent <builtin> --info` on a totally fresh config dir has to install
// the bundled definitions before it can render the agent readout.
#[test]
fn agent_info_on_empty_config_dir_bootstraps_builtins() {
let tmp_dir = fresh_config_dir("agent-info");
let home_dir = fresh_config_dir("agent-info-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(["--agent", "adversary", "--info"])
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let agents_dir = tmp_dir.join("agents");
let builtins_installed =
agents_dir.is_dir() && fs::read_dir(&agents_dir).unwrap().next().is_some();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
output.status.success(),
"--agent adversary --info: expected exit 0 on an empty config dir, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
builtins_installed,
"--agent adversary --info: expected builtin agents installed into {}",
agents_dir.display()
);
// The --info readout echoes the loaded agent definition including its
// global_tools, so a builtin tool name there proves the definitions were
// loaded into the agent's tool scope, not just written to disk.
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("fs_cat.sh"),
"--agent adversary --info: expected builtin tool fs_cat.sh in the readout\nstdout: {stdout}"
);
}
// Sandbox-mode vault contract: `--list-secrets` is an inspection readout and
// must exit 0 with an informational message (and zero writes) when the vault
// is unreachable inside a sandbox, while mutating vault flags stay strict.
#[test]
fn sandbox_list_secrets_prints_informational_message_and_writes_nothing() {
let tmp_dir = fresh_config_dir("sandbox-list-secrets");
let home_dir = fresh_config_dir("sandbox-list-secrets-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--list-secrets")
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("IS_SANDBOX", "1")
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let leftover: Vec<_> = fs::read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
output.status.success(),
"--list-secrets in sandbox mode: expected exit 0, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let combined = format!(
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
combined.contains("The vault is unavailable in sandbox mode"),
"--list-secrets in sandbox mode: expected the informational readout, got: {combined}"
);
assert!(
leftover.is_empty(),
"--list-secrets in sandbox mode: expected zero writes into the config dir: {leftover:?}"
);
}
// Mutating vault flags are NOT inspection flags: in sandbox mode they must
// fail hard with the vault-management error, never the soft informational
// message. A minimal valid config is provided so the run gets past model
// resolution and reaches the vault gate.
#[test]
fn sandbox_mutating_vault_flag_stays_strict() {
let tmp_dir = fresh_config_dir("sandbox-delete-secret");
let home_dir = fresh_config_dir("sandbox-delete-secret-home");
fs::write(
tmp_dir.join("config.yaml"),
"model: dryrun:dry-model\n\
dry_run: true\n\
clients:\n\
\x20 - type: openai\n\
\x20 name: dryrun\n\
\x20 auth: none\n\
\x20 api_key: unused\n\
\x20 models:\n\
\x20 - name: dry-model\n\
\x20 max_input_tokens: 100000\n\
\x20 supports_function_calling: true\n\
save: false\n",
)
.unwrap();
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(["--delete-secret", "probe"])
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("IS_SANDBOX", "1")
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
!output.status.success(),
"--delete-secret in sandbox mode: expected a non-zero exit, got {:?}\nstdout: {}",
output.status,
String::from_utf8_lossy(&output.stdout)
);
let combined = format!(
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
combined.contains("Vault management is disabled in sandbox mode"),
"--delete-secret in sandbox mode: expected the strict vault error, got: {combined}"
);
}
// The --mcp-list leniency must not leak into the mutating MCP flags on the
// host path: with no password file (fresh fake HOME) and no sandbox env,
// --mcp-get/--mcp-remove/--mcp-add have to fail on the strict vault
// requirement rather than operate with an unconfigured vault.
fn probe_strict_mcp_flag(label: &str, args: &[&str]) {
let tmp_dir = fresh_config_dir(&format!("{label}-no-vault"));
let home_dir = fresh_config_dir(&format!("{label}-no-vault-home"));
fs::write(
tmp_dir.join("config.yaml"),
"model: dryrun:dry-model\n\
dry_run: true\n\
clients:\n\
\x20 - type: openai\n\
\x20 name: dryrun\n\
\x20 auth: none\n\
\x20 api_key: unused\n\
\x20 models:\n\
\x20 - name: dry-model\n\
\x20 max_input_tokens: 100000\n\
\x20 supports_function_calling: true\n\
save: false\n",
)
.unwrap();
fs::write(
tmp_dir.join("mcp.json"),
r#"{"mcpServers":{"probe-server":{"type":"stdio","command":"echo","args":["hi"]}}}"#,
)
.unwrap();
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(args)
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
// The strict path must not touch the server registry either.
let mcp_json_after = fs::read_to_string(tmp_dir.join("mcp.json")).unwrap();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
!output.status.success(),
"{label} without a vault: expected a non-zero exit, got {:?}\nstdout: {}",
output.status,
String::from_utf8_lossy(&output.stdout)
);
let combined = format!(
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
combined.contains("A password file is required"),
"{label} without a vault: expected the strict vault error, got: {combined}"
);
assert!(
mcp_json_after.contains("probe-server"),
"{label} without a vault: mcp.json must be left untouched, got: {mcp_json_after}"
);
}
#[test]
fn mcp_mutating_flag_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-remove", &["--mcp-remove", "probe-server"]);
}
// --mcp-get is read-only but consumes resolved server config (which may
// embed secrets), so it stays on the strict vault path by design.
#[test]
fn mcp_get_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-get", &["--mcp-get", "probe-server"]);
}
#[test]
fn mcp_add_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-add", &["--mcp-add", "newsrv", "--", "echo", "hi"]);
}
// The listing leniency exercised with content: a configured, secret-free
// server must be listed even when no vault exists (no password file, no
// sandbox env), and the readout must not write anything new into the
// config dir.
#[test]
fn mcp_list_lists_configured_server_without_vault() {
let tmp_dir = fresh_config_dir("mcp-list-configured");
let home_dir = fresh_config_dir("mcp-list-configured-home");
fs::write(
tmp_dir.join("mcp.json"),
r#"{"mcpServers":{"probe-server":{"type":"stdio","command":"echo","args":["hi"]}}}"#,
)
.unwrap();
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--mcp-list")
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let leftover: Vec<_> = fs::read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
output.status.success(),
"--mcp-list with a configured server and no vault: expected exit 0, got {:?}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("probe-server"),
"--mcp-list with a configured server: expected 'probe-server' in stdout, got: {stdout}"
);
assert_eq!(
leftover,
vec![std::ffi::OsString::from("mcp.json")],
"--mcp-list must not write into the config dir beyond the pre-seeded mcp.json"
);
}
// --sync-models downloads and writes models-override.yaml, so it is NOT an
// inspection flag: on a pristine config dir it must take the bootstrap path
// (builtins installed) and, off a terminal, fail on the missing config
// instead of silently operating on defaults. The host path is the one under
// test: a leaked IS_SANDBOX (or provider env) would reroute the missing
// config to the sandbox wizard prompt instead of the fast failure.
#[test]
fn sync_models_stays_on_bootstrap_path() {
let tmp_dir = fresh_config_dir("sync-models");
let home_dir = fresh_config_dir("sync-models-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--sync-models")
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let bootstrapped = fs::read_dir(&tmp_dir).unwrap().next().is_some();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
!output.status.success(),
"--sync-models: expected the strict missing-config failure on an empty config dir, got {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
bootstrapped,
"--sync-models: expected the builtins bootstrap to populate the config dir"
);
}
+293
View File
@@ -0,0 +1,293 @@
//! Binary-level contract for --macro bracket symmetry: a committed macro
//! run fires agent.started plus EXACTLY ONE terminal agent event
//! (agent.completed / agent.failed / agent.interrupted), whether the agent
//! arrives via the --agent flag or an in-macro `.agent` step, in both
//! isolation modes, on success and failure alike. The markers ride on
//! global agent.* hooks admitted through the agent's `global_hooks: ["*"]`
//! wildcard, so the fixture also exercises the whitelist grammar
//! end-to-end — and stays observable even if a failing step leaves the
//! context without a loaded agent.
use std::env;
use std::fs;
use std::path::{Path, PathBuf};
use std::process::{Command, Output, Stdio};
use std::thread;
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
const MARKERS: [&str; 4] = ["STARTED", "COMPLETED", "FAILED", "INTERRUPTED"];
fn fresh_config_dir(label: &str) -> PathBuf {
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let tmp_dir = env::temp_dir().join(format!("coyote-macro-bracketing-{label}-{unique}"));
fs::create_dir_all(&tmp_dir).unwrap();
tmp_dir
}
/// Removes the fixture dir even when an assertion panics mid-run, so failed
/// runs do not accumulate temp dirs.
struct TempDirGuard(PathBuf);
impl Drop for TempDirGuard {
fn drop(&mut self) {
let _ = fs::remove_dir_all(&self.0);
}
}
/// Path of the file `marker`'s hook appends to. Every event gets its OWN
/// file because cmd opens `>>` targets write-exclusive (no shared write
/// access): the STARTED and terminal hook children run concurrently, and
/// when both appended to one shared log, overlapping opens made the loser
/// fail with a sharing violation and silently drop its line — a permanent
/// marker loss that surfaced as a Windows-CI-only flake. Separate files
/// make writer overlap impossible: each event fires at most once per
/// bracket, and re-opened brackets are step-separated, so every file has
/// one writer at a time.
fn marker_log(dir: &Path, marker: &str) -> PathBuf {
dir.join(format!("agent-events-{marker}.log"))
}
/// Builds the hook command that appends `marker` to its own marker file, in
/// the dialect of the shell the hook engine dispatches through: `sh -c`
/// elsewhere, `cmd /C` on Windows. The cmd form parenthesizes the echo
/// because `echo X >> f` under cmd writes "X " with a trailing space, which
/// would break exact marker matching. Both forms double-quote the path, so
/// the command also survives YAML single-quoting and spaces in temp paths.
fn marker_command(marker: &str, dir: &Path) -> String {
let log = marker_log(dir, marker);
let log = log.display();
if cfg!(windows) {
format!(r#"(echo {marker})>> "{log}""#)
} else {
format!(r#"echo {marker} >> "{log}""#)
}
}
/// Lays out a config dir with a dry_run model, global agent.* hooks that
/// append one marker line per event to that event's marker file, a
/// `probe-macro` agent that whitelists everything, and a `probe` macro with
/// the given YAML body.
fn write_fixture(dir: &Path, macro_yaml: &str) {
// Without IS_SANDBOX (scrubbed below) a run insists on a vault password
// file, so the fixture provides one.
let vault_pass = dir.join("vault-pass");
fs::write(&vault_pass, "test-password\n").unwrap();
// Paths land in single-quoted YAML scalars: double-quoted ones would read
// the backslashes in a Windows temp path (`C:\Users\...`) as escapes.
fs::write(
dir.join("config.yaml"),
format!(
"model: dryrun:dry-model\n\
dry_run: true\n\
vault_password_file: '{vault_pass}'\n\
clients:\n\
\x20 - type: openai\n\
\x20 name: dryrun\n\
\x20 auth: none\n\
\x20 api_key: 'unused'\n\
\x20 models:\n\
\x20 - name: dry-model\n\
\x20 max_input_tokens: 100000\n\
\x20 supports_function_calling: true\n\
save: false\n\
memory: false\n\
stream: false\n\
hooks:\n\
\x20 agent.started:\n\
\x20 - name: mark\n\
\x20 command: '{started}'\n\
\x20 agent.completed:\n\
\x20 - name: mark\n\
\x20 command: '{completed}'\n\
\x20 agent.failed:\n\
\x20 - name: mark\n\
\x20 command: '{failed}'\n\
\x20 agent.interrupted:\n\
\x20 - name: mark\n\
\x20 command: '{interrupted}'\n",
vault_pass = vault_pass.display(),
started = marker_command("STARTED", dir),
completed = marker_command("COMPLETED", dir),
failed = marker_command("FAILED", dir),
interrupted = marker_command("INTERRUPTED", dir),
),
)
.unwrap();
let agent_dir = dir.join("agents/probe-macro");
fs::create_dir_all(&agent_dir).unwrap();
fs::write(
agent_dir.join("config.yaml"),
"name: probe-macro\n\
description: macro bracketing probe agent\n\
version: 0.1.0\n\
instructions: |\n\
\x20 You are a probe agent. Reply briefly.\n\
global_hooks:\n\
\x20 - \"*\"\n",
)
.unwrap();
let macros_dir = dir.join("macros");
fs::create_dir_all(&macros_dir).unwrap();
fs::write(macros_dir.join("probe.yaml"), macro_yaml).unwrap();
}
fn run_coyote(dir: &Path, args: &[&str]) -> Output {
Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(args)
.current_dir(dir)
.env("COYOTE_CONFIG_DIR", dir)
.env_remove("IS_SANDBOX")
.stdin(Stdio::null())
.output()
.unwrap()
}
/// Counts `marker`'s exact-match lines in its own marker file; a missing
/// file is zero fires. Matching stays byte-exact on both platforms because
/// `lines()` strips the CRLF that cmd's echo emits on Windows.
fn marker_count(dir: &Path, marker: &str) -> usize {
fs::read_to_string(marker_log(dir, marker))
.unwrap_or_default()
.lines()
.filter(|line| *line == marker)
.count()
}
fn dump_markers(dir: &Path) -> String {
MARKERS
.iter()
.map(|marker| {
let contents = fs::read_to_string(marker_log(dir, marker)).unwrap_or_default();
format!("{marker}: {contents:?}")
})
.collect::<Vec<_>>()
.join(", ")
}
/// Bounded poll until every expected marker is present. The binary's exit
/// drain only acknowledges that each hook process SPAWNED — the hooks
/// themselves keep running concurrently after `output()` returns, and
/// nothing orders their writes (the STARTED writer can land after the
/// terminal one). Waiting on all expected markers, not just the terminal
/// one, keeps the exact-count assertions below race-free.
fn wait_for_markers(dir: &Path, needles: &[&str]) {
let deadline = Instant::now() + Duration::from_secs(10);
loop {
if needles.iter().all(|needle| marker_count(dir, needle) > 0) {
return;
}
assert!(
Instant::now() < deadline,
"timed out waiting for {needles:?}; markers so far: {}",
dump_markers(dir)
);
thread::sleep(Duration::from_millis(50));
}
}
fn assert_marker_count(dir: &Path, marker: &str, expected: usize) {
let count = marker_count(dir, marker);
assert_eq!(
count,
expected,
"expected {expected}x {marker}; markers so far: {}",
dump_markers(dir)
);
}
fn assert_started_and_single_terminal(dir: &Path, terminal: &str) {
assert_marker_count(dir, "STARTED", 1);
for marker in ["COMPLETED", "FAILED", "INTERRUPTED"] {
let expected = usize::from(marker == terminal);
assert_marker_count(dir, marker, expected);
}
}
fn probe_macro_run(label: &str, macro_yaml: &str, args: &[&str], expect_success: bool) {
let dir = fresh_config_dir(label);
let _cleanup = TempDirGuard(dir.clone());
write_fixture(&dir, macro_yaml);
let output = run_coyote(&dir, args);
assert_eq!(
output.status.success(),
expect_success,
"{label}: unexpected exit {:?}\nstdout: {}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let terminal = if expect_success {
"COMPLETED"
} else {
"FAILED"
};
wait_for_markers(&dir, &["STARTED", terminal]);
assert_started_and_single_terminal(&dir, terminal);
}
#[test]
fn agent_macro_run_fires_started_and_exactly_one_completed() {
probe_macro_run(
"agent-flag-success",
"steps:\n - \".set temperature 0.5\"\n",
&["--agent", "probe-macro", "--macro", "probe"],
true,
);
}
#[test]
fn agent_macro_failure_closes_the_bracket_with_exactly_one_failed() {
probe_macro_run(
"agent-flag-failure",
"steps:\n - \".agent no-such-agent-zz\"\n",
&["--agent", "probe-macro", "--macro", "probe"],
false,
);
}
#[test]
fn isolated_in_macro_agent_step_fires_started_and_exactly_one_completed() {
probe_macro_run(
"isolated-step-success",
"steps:\n - \".agent probe-macro\"\n - \".set temperature 0.5\"\n",
&["--macro", "probe"],
true,
);
}
#[test]
fn isolated_in_macro_agent_step_failure_closes_the_bracket() {
probe_macro_run(
"isolated-step-failure",
"steps:\n - \".agent probe-macro\"\n - \".agent no-such-agent-zz\"\n",
&["--macro", "probe"],
false,
);
}
#[test]
fn non_isolated_in_macro_agent_step_fires_started_and_exactly_one_completed() {
probe_macro_run(
"non-isolated-step-success",
"isolated: false\nsteps:\n - \".agent probe-macro\"\n - \".set temperature 0.5\"\n",
&["--macro", "probe"],
true,
);
}
#[test]
fn non_isolated_in_macro_agent_step_failure_closes_the_bracket() {
probe_macro_run(
"non-isolated-step-failure",
"isolated: false\nsteps:\n - \".agent probe-macro\"\n - \".agent no-such-agent-zz\"\n",
&["--macro", "probe"],
false,
);
}