feat(TASK-049): add escalation.raised/answered, session.resumed, and agent.interrupted hook events
- escalation.raised fires in the child ctx after queue submit; escalation.answered fires in the parent ctx from the taken request, so both agree on origin envs - escalation.* resolves global hooks ungated: the per-agent global_hooks whitelist never applies to these root-level events - session.resumed fires on the use_session resume leg, inverse of session.started - agent.interrupted replaces agent.completed/failed when the abort signal was ctrl-c'd: top-level bracket (now abort-aware), spawned-subagent result arm (agent__cancel / REPL ctrl-c), and graph agent nodes via the session signal; exit_agent and the shell-exit path stay non-interrupted by passing no signal - document the deliberate non-fire paths (direct prompts, escalation timeout and parent-drop, graph approval nodes)
This commit is contained in:
8 files changed
+468
-41
No files matched your search
@@ -4741,6 +4741,8 @@ impl RequestContext {
|
||||
self.init_agent_session_variables(new_session)?;
|
||||
if created_new_session {
|
||||
hooks::fire(HookEvent::SessionStarted, self, &[], None);
|
||||
} else {
|
||||
hooks::fire(HookEvent::SessionResumed, self, &[], None);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -4909,7 +4911,10 @@ impl RequestContext {
|
||||
|
||||
// Interactive switch-away closes the top-level agent bracket while
|
||||
// the agent is still attached, so agent-scoped hooks resolve.
|
||||
self.top_level_agent_finished(None);
|
||||
// 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 let Some(supervisor) = self.supervisor.clone() {
|
||||
supervisor.read().cancel_recursive();
|
||||
@@ -4957,11 +4962,27 @@ 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 {
|
||||
return;
|
||||
};
|
||||
|
||||
// The abort signal, never the error text, decides interrupted: a
|
||||
// ctrl-c'd run is not a failure, so no error env rides along.
|
||||
if abort_signal.is_some_and(|signal| signal.aborted_ctrlc()) {
|
||||
hooks::fire(
|
||||
HookEvent::AgentInterrupted,
|
||||
self,
|
||||
&[("COYOTE_AGENT_ID", id)],
|
||||
None,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
match error {
|
||||
None => hooks::fire(
|
||||
HookEvent::AgentCompleted,
|
||||
@@ -8340,7 +8361,12 @@ mod tests {
|
||||
let agent_dir = paths::agent_data_dir(agent_name);
|
||||
create_dir_all(&agent_dir).unwrap();
|
||||
let mut config = format!("name: {agent_name}\ninstructions: hi\nhooks:\n");
|
||||
for event in ["agent.started", "agent.completed", "agent.failed"] {
|
||||
for event in [
|
||||
"agent.started",
|
||||
"agent.completed",
|
||||
"agent.interrupted",
|
||||
"agent.failed",
|
||||
] {
|
||||
config.push_str(&format!(
|
||||
" {event}:\n - name: {marker}-{event}\n command: 'true'\n"
|
||||
));
|
||||
@@ -8360,7 +8386,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
#[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 sessions_dir = paths::local_dir("sessions");
|
||||
create_dir_all(&sessions_dir).unwrap();
|
||||
@@ -8368,13 +8394,16 @@ mod tests {
|
||||
let marker = "sess-started-q8n";
|
||||
|
||||
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)
|
||||
});
|
||||
let app = ctx.app.config.clone();
|
||||
let abort = utils::create_abort_signal();
|
||||
run_async(ctx.use_session(&app, Some("hook-fresh"), abort.clone())).unwrap();
|
||||
|
||||
let captures = marker_captures(marker);
|
||||
assert_eq!(captures.len(), 1, "a new session fires exactly one event");
|
||||
assert_eq!(captures[0].hook_name, format!("{marker}-session.started"));
|
||||
assert_eq!(
|
||||
captures[0]
|
||||
.envs
|
||||
@@ -8383,16 +8412,29 @@ mod tests {
|
||||
Some("hook-fresh")
|
||||
);
|
||||
|
||||
// Resuming a persisted session fires nothing.
|
||||
// Resuming a persisted session fires session.resumed, never
|
||||
// session.started.
|
||||
write_paused_todo_session(&ctx, "hook-resume");
|
||||
let mut resumed_ctx = create_test_ctx();
|
||||
resumed_ctx.update_app_config(|app| app.hooks = hooks_map_of(&["session.started"], marker));
|
||||
resumed_ctx.update_app_config(|app| {
|
||||
app.hooks = hooks_map_of(&["session.started", "session.resumed"], marker)
|
||||
});
|
||||
let resumed_app = resumed_ctx.app.config.clone();
|
||||
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!(
|
||||
marker_captures(marker).len(),
|
||||
1,
|
||||
"resume must not fire session.started"
|
||||
resumed[0].envs.get("COYOTE_SESSION_ID").map(String::as_str),
|
||||
Some("hook-resume")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -8469,8 +8511,8 @@ mod tests {
|
||||
ctx.top_level_agent_started();
|
||||
// Per agent RUN, never per message: re-entering while open is a no-op.
|
||||
ctx.top_level_agent_started();
|
||||
ctx.top_level_agent_finished(None);
|
||||
ctx.top_level_agent_finished(None);
|
||||
ctx.top_level_agent_finished(None, None);
|
||||
ctx.top_level_agent_finished(None, None);
|
||||
|
||||
let captures = marker_captures(marker);
|
||||
let started: Vec<_> = captures
|
||||
@@ -8519,7 +8561,7 @@ mod tests {
|
||||
|
||||
ctx.top_level_agent_started();
|
||||
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 failed: Vec<_> = captures
|
||||
@@ -8539,6 +8581,44 @@ mod tests {
|
||||
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_helpers_noop_without_agent() {
|
||||
@@ -8556,7 +8636,7 @@ mod tests {
|
||||
ctx.role = Some(Role::new("plain", "prompt"));
|
||||
|
||||
ctx.top_level_agent_started();
|
||||
ctx.top_level_agent_finished(None);
|
||||
ctx.top_level_agent_finished(None, None);
|
||||
|
||||
assert!(
|
||||
marker_captures(marker).is_empty(),
|
||||
@@ -8582,7 +8662,7 @@ mod tests {
|
||||
|
||||
ctx.exit_agent(&app).unwrap();
|
||||
// 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);
|
||||
assert_eq!(
|
||||
|
||||
+84
-9
@@ -997,14 +997,15 @@ fn sync_agent_functions_to_ctx(ctx: &mut RequestContext) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// 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.
|
||||
/// Pre-resolved `agent.completed`/`agent.interrupted`/`agent.failed`
|
||||
/// snapshots for a spawned agent's result arms, captured from the child
|
||||
/// context before the spawn task takes ownership of it. Resolving against
|
||||
/// the child keeps global hooks gated by the child's own whitelist, the
|
||||
/// same gate every other event fired from the child's context goes through.
|
||||
struct SpawnResultHooks {
|
||||
completed: Vec<ResolvedHook>,
|
||||
failed: Vec<ResolvedHook>,
|
||||
interrupted: Vec<ResolvedHook>,
|
||||
session_name: Option<String>,
|
||||
agent_id: String,
|
||||
agent_name: String,
|
||||
@@ -1015,6 +1016,7 @@ impl SpawnResultHooks {
|
||||
Self {
|
||||
completed: child_ctx.resolved_hooks(HookEvent::AgentCompleted),
|
||||
failed: child_ctx.resolved_hooks(HookEvent::AgentFailed),
|
||||
interrupted: child_ctx.resolved_hooks(HookEvent::AgentInterrupted),
|
||||
session_name: child_ctx
|
||||
.session
|
||||
.as_ref()
|
||||
@@ -1024,14 +1026,20 @@ impl SpawnResultHooks {
|
||||
}
|
||||
}
|
||||
|
||||
/// Fires `agent.completed`, or `agent.failed` when `error` is given.
|
||||
fn fire(self, error: Option<&str>) {
|
||||
/// Fires `agent.completed`, or — for a failed child — `agent.interrupted`
|
||||
/// when its abort signal was ctrl-c'd (REPL ctrl-c or `agent__cancel`),
|
||||
/// else `agent.failed`. The signal, never the error text, decides:
|
||||
/// cancellation is not a failure, so no error env rides along.
|
||||
fn fire(self, error: Option<&str>, child_abort: &AbortSignal) {
|
||||
let mut extras = vec![
|
||||
("COYOTE_AGENT_ID", self.agent_id),
|
||||
("COYOTE_AGENT_NAME", self.agent_name.clone()),
|
||||
];
|
||||
let (event, resolved) = match error {
|
||||
None => (HookEvent::AgentCompleted, self.completed),
|
||||
Some(_) if child_abort.aborted_ctrlc() => {
|
||||
(HookEvent::AgentInterrupted, self.interrupted)
|
||||
}
|
||||
Some(error) => {
|
||||
extras.push(("COYOTE_AGENT_ERROR", error.to_string()));
|
||||
(HookEvent::AgentFailed, self.failed)
|
||||
@@ -1216,6 +1224,7 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
|
||||
let spawn_agent_id = agent_id.clone();
|
||||
let spawn_agent_name = agent_name.clone();
|
||||
let spawn_abort = child_abort.clone();
|
||||
let hook_abort = child_abort.clone();
|
||||
let spawn_notifications = Arc::clone(&ctx.notification_queue);
|
||||
let child_supervisor = child_ctx.supervisor.clone();
|
||||
|
||||
@@ -1238,8 +1247,8 @@ async fn handle_spawn(ctx: &mut RequestContext, args: &Value) -> Result<Value> {
|
||||
};
|
||||
let success = agent_result.exit_status == AgentExitStatus::Completed;
|
||||
match &agent_result.exit_status {
|
||||
AgentExitStatus::Completed => result_hooks.fire(None),
|
||||
AgentExitStatus::Failed(e) => result_hooks.fire(Some(e)),
|
||||
AgentExitStatus::Completed => result_hooks.fire(None, &hook_abort),
|
||||
AgentExitStatus::Failed(e) => result_hooks.fire(Some(e), &hook_abort),
|
||||
}
|
||||
spawn_notifications.push(agent_notification(
|
||||
&agent_result.id,
|
||||
@@ -1771,6 +1780,21 @@ fn handle_reply_escalation(ctx: &mut RequestContext, args: &Value) -> Result<Val
|
||||
let from_agent = request.from_agent_name.clone();
|
||||
let question = request.question.clone();
|
||||
let _ = request.reply_tx.send(reply.to_string());
|
||||
// Origin envs come from the taken request, not this (replying)
|
||||
// context, so `escalation.answered` and `escalation.raised`
|
||||
// describe the same agent for one escalation id.
|
||||
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!({
|
||||
"status": "ok",
|
||||
"message": format!("Reply sent to agent '{from_agent}' for escalation '{escalation_id}'"),
|
||||
@@ -2251,6 +2275,9 @@ mod tests {
|
||||
\x20 agent.completed:\n\
|
||||
\x20 - name: {marker}_completed\n\
|
||||
\x20 command: \"true\"\n\
|
||||
\x20 agent.interrupted:\n\
|
||||
\x20 - name: {marker}_interrupted\n\
|
||||
\x20 command: \"true\"\n\
|
||||
\x20 agent.failed:\n\
|
||||
\x20 - name: {marker}_failed\n\
|
||||
\x20 command: \"true\"\n\
|
||||
@@ -2346,6 +2373,54 @@ mod tests {
|
||||
assert!(hook_captures_named("t037_spawn_fail_completed").is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
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);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::{FunctionDeclaration, JsonSchema};
|
||||
use crate::config::RequestContext;
|
||||
use crate::hooks::{self, HookEvent};
|
||||
use crate::supervisor::escalation::{EscalationRequest, new_escalation_id};
|
||||
use crate::utils::{ACP_SERVER, HEADLESS, queue_acp_permission};
|
||||
|
||||
@@ -179,6 +180,8 @@ fn handle_headless(action: &str, args: &Value) -> Value {
|
||||
})
|
||||
}
|
||||
|
||||
// The root agent prompting its own user is a direct interaction, not an
|
||||
// escalation: no escalation event fires on this path.
|
||||
fn handle_direct(action: &str, args: &Value) -> Result<Value> {
|
||||
match action {
|
||||
"select" => handle_direct_ask(args),
|
||||
@@ -278,22 +281,37 @@ async fn handle_escalated(ctx: &RequestContext, action: &str, args: &Value) -> R
|
||||
let escalation_id = new_escalation_id();
|
||||
let (tx, rx) = oneshot::channel();
|
||||
|
||||
let question = format!("[{action}] {question}");
|
||||
let request = EscalationRequest {
|
||||
id: escalation_id.clone(),
|
||||
from_agent_id,
|
||||
from_agent_id: from_agent_id.clone(),
|
||||
from_agent_name: from_agent_name.clone(),
|
||||
question: format!("[{action}] {question}"),
|
||||
question: question.clone(),
|
||||
options,
|
||||
reply_tx: tx,
|
||||
};
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
/// Waits for the parent's reply. `timeout_secs == 0` waits indefinitely; a
|
||||
/// 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> {
|
||||
let reply = if timeout_secs == 0 {
|
||||
rx.await
|
||||
@@ -346,8 +364,14 @@ fn parse_options(args: &Value) -> Result<Vec<String>> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
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::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
#[test]
|
||||
fn headless_select_returns_structured_json() {
|
||||
@@ -374,6 +398,112 @@ mod tests {
|
||||
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"));
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn zero_timeout_waits_indefinitely_for_the_reply() {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
|
||||
+91
-11
@@ -50,16 +50,36 @@ impl AgentNodeExecutor {
|
||||
)
|
||||
.await;
|
||||
if let Err(e) = &result {
|
||||
hooks::fire(
|
||||
HookEvent::AgentFailed,
|
||||
parent_ctx,
|
||||
&[
|
||||
("COYOTE_AGENT_ID", agent_hook_id),
|
||||
("COYOTE_AGENT_NAME", node.agent.clone()),
|
||||
("COYOTE_AGENT_ERROR", format!("{e:#}")),
|
||||
],
|
||||
None,
|
||||
);
|
||||
// The child's own abort signal dies inside `run`; the session
|
||||
// signal is what a user interrupt fires (the executor bridges it
|
||||
// onto the graph abort), so it is the only honest
|
||||
// interrupted-vs-failed discriminator at this seam.
|
||||
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(
|
||||
HookEvent::AgentFailed,
|
||||
parent_ctx,
|
||||
&[
|
||||
("COYOTE_AGENT_ID", agent_hook_id),
|
||||
("COYOTE_AGENT_NAME", node.agent.clone()),
|
||||
("COYOTE_AGENT_ERROR", format!("{e:#}")),
|
||||
],
|
||||
None,
|
||||
);
|
||||
}
|
||||
}
|
||||
outcome_from(node_id, node, state_manager, result)
|
||||
}
|
||||
@@ -338,10 +358,12 @@ fn apply_state_updates(node: &AgentNode, state_manager: &mut StateManager, outpu
|
||||
mod tests {
|
||||
use super::super::types::AgentNode;
|
||||
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::testing::{install_log_collector, warn_snapshot};
|
||||
use serde_json::json;
|
||||
use serial_test::serial;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -416,6 +438,64 @@ mod tests {
|
||||
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"
|
||||
);
|
||||
}
|
||||
|
||||
fn retryable_node(max_attempts: u32) -> AgentNode {
|
||||
let mut node = node_with("hi", None);
|
||||
node.max_attempts = max_attempts;
|
||||
|
||||
@@ -21,6 +21,10 @@ impl ApprovalNodeExecutor {
|
||||
.interpolate(&node.question)
|
||||
.context("Failed to interpolate approval question")?;
|
||||
|
||||
// No escalation event fires at this seam: a root graph prompts its
|
||||
// user directly (outside the escalation queue), and a child graph's
|
||||
// escalation fires inside `handle_escalated` like any other user__*
|
||||
// call.
|
||||
let response = handle_user_tool(
|
||||
ctx,
|
||||
&format!("{USER_FUNCTION_PREFIX}select"),
|
||||
|
||||
@@ -119,6 +119,7 @@ pub enum HookEvent {
|
||||
TurnInterrupted,
|
||||
TurnFailed,
|
||||
SessionStarted,
|
||||
SessionResumed,
|
||||
SessionEnded,
|
||||
SessionCompressed,
|
||||
ToolStarted,
|
||||
@@ -129,7 +130,10 @@ pub enum HookEvent {
|
||||
LlmRequestFailed,
|
||||
AgentStarted,
|
||||
AgentCompleted,
|
||||
AgentInterrupted,
|
||||
AgentFailed,
|
||||
EscalationRaised,
|
||||
EscalationAnswered,
|
||||
GraphNodeStarted,
|
||||
GraphNodeCompleted,
|
||||
GraphNodeFailed,
|
||||
@@ -146,6 +150,7 @@ impl HookEvent {
|
||||
HookEvent::TurnInterrupted => "turn.interrupted",
|
||||
HookEvent::TurnFailed => "turn.failed",
|
||||
HookEvent::SessionStarted => "session.started",
|
||||
HookEvent::SessionResumed => "session.resumed",
|
||||
HookEvent::SessionEnded => "session.ended",
|
||||
HookEvent::SessionCompressed => "session.compressed",
|
||||
HookEvent::ToolStarted => "tool.started",
|
||||
@@ -156,7 +161,10 @@ impl HookEvent {
|
||||
HookEvent::LlmRequestFailed => "llm.request.failed",
|
||||
HookEvent::AgentStarted => "agent.started",
|
||||
HookEvent::AgentCompleted => "agent.completed",
|
||||
HookEvent::AgentInterrupted => "agent.interrupted",
|
||||
HookEvent::AgentFailed => "agent.failed",
|
||||
HookEvent::EscalationRaised => "escalation.raised",
|
||||
HookEvent::EscalationAnswered => "escalation.answered",
|
||||
HookEvent::GraphNodeStarted => "graph.node.started",
|
||||
HookEvent::GraphNodeCompleted => "graph.node.completed",
|
||||
HookEvent::GraphNodeFailed => "graph.node.failed",
|
||||
@@ -220,6 +228,17 @@ fn resolve_hooks(
|
||||
let event_name = event.as_str();
|
||||
let mut resolved = Vec::new();
|
||||
|
||||
// Escalations are a root/user-level concern: the user wires these hooks
|
||||
// to hear about children asking for help, so requiring every agent to
|
||||
// whitelist them through `global_hooks` would silence exactly the events
|
||||
// they were set up for. The per-agent gate never applies here (this also
|
||||
// skips the unknown-entry diagnostics: whitelisting an escalation hook is
|
||||
// harmless and needs no warning).
|
||||
let agent_gate = match event {
|
||||
HookEvent::EscalationRaised | HookEvent::EscalationAnswered => None,
|
||||
_ => agent_gate,
|
||||
};
|
||||
|
||||
if let Some(defs) = global_hooks.get(event_name) {
|
||||
let cwd = paths::config_dir();
|
||||
for def in defs {
|
||||
@@ -1326,6 +1345,7 @@ mod tests {
|
||||
(HookEvent::TurnInterrupted, "turn.interrupted"),
|
||||
(HookEvent::TurnFailed, "turn.failed"),
|
||||
(HookEvent::SessionStarted, "session.started"),
|
||||
(HookEvent::SessionResumed, "session.resumed"),
|
||||
(HookEvent::SessionEnded, "session.ended"),
|
||||
(HookEvent::SessionCompressed, "session.compressed"),
|
||||
(HookEvent::ToolStarted, "tool.started"),
|
||||
@@ -1336,7 +1356,10 @@ mod tests {
|
||||
(HookEvent::LlmRequestFailed, "llm.request.failed"),
|
||||
(HookEvent::AgentStarted, "agent.started"),
|
||||
(HookEvent::AgentCompleted, "agent.completed"),
|
||||
(HookEvent::AgentInterrupted, "agent.interrupted"),
|
||||
(HookEvent::AgentFailed, "agent.failed"),
|
||||
(HookEvent::EscalationRaised, "escalation.raised"),
|
||||
(HookEvent::EscalationAnswered, "escalation.answered"),
|
||||
(HookEvent::GraphNodeStarted, "graph.node.started"),
|
||||
(HookEvent::GraphNodeCompleted, "graph.node.completed"),
|
||||
(HookEvent::GraphNodeFailed, "graph.node.failed"),
|
||||
@@ -1376,6 +1399,38 @@ mod tests {
|
||||
assert_eq!(names(&resolved), ["own"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn escalation_events_bypass_the_agent_whitelist_gate() {
|
||||
let mut global = hooks_map("escalation.raised", &[("watch", "cmd-raised")]);
|
||||
global.extend(hooks_map(
|
||||
"escalation.answered",
|
||||
&[("watch", "cmd-answered")],
|
||||
));
|
||||
global.extend(hooks_map("tool.started", &[("watch", "cmd-tool")]));
|
||||
|
||||
for event in [HookEvent::EscalationRaised, HookEvent::EscalationAnswered] {
|
||||
let resolved = resolve_hooks(event, &global, Some((&[], "gated-agent")), None, None);
|
||||
assert_eq!(
|
||||
names(&resolved),
|
||||
["watch"],
|
||||
"{} must resolve without a whitelist entry",
|
||||
event.as_str()
|
||||
);
|
||||
}
|
||||
|
||||
let resolved = resolve_hooks(
|
||||
HookEvent::ToolStarted,
|
||||
&global,
|
||||
Some((&[], "gated-agent")),
|
||||
None,
|
||||
None,
|
||||
);
|
||||
assert!(
|
||||
resolved.is_empty(),
|
||||
"non-escalation events stay behind the whitelist gate"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitelist_admits_only_exact_global_entries() {
|
||||
let global = hooks_map(
|
||||
|
||||
+6
-3
@@ -587,7 +587,7 @@ async fn run(
|
||||
shell_execute(&mut ctx, &SHELL, input, abort_signal.clone()).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;
|
||||
return result;
|
||||
}
|
||||
@@ -621,7 +621,7 @@ async fn run(
|
||||
start_directive(&mut ctx, input, cli.code, abort_signal.clone()).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;
|
||||
result
|
||||
}
|
||||
@@ -755,7 +755,10 @@ async fn shell_execute(
|
||||
if code == 0 && app.save_shell_history {
|
||||
let _ = append_to_shell_history(&shell.name, &eval_str, code);
|
||||
}
|
||||
ctx.top_level_agent_finished(None);
|
||||
// Executing the generated command is a normal exit: no
|
||||
// abort signal on purpose, this seam can never report an
|
||||
// interruption.
|
||||
ctx.top_level_agent_finished(None, None);
|
||||
hooks::drain_pending(EXIT_HOOK_DRAIN_TIMEOUT).await;
|
||||
process::exit(code);
|
||||
}
|
||||
|
||||
+1
-1
@@ -533,7 +533,7 @@ Type ".help" for additional help.
|
||||
let exit_result = self.ctx.write().exit_session();
|
||||
self.ctx
|
||||
.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;
|
||||
exit_result
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user