diff --git a/docs/user/agent-hooks-protocol.md b/docs/user/agent-hooks-protocol.md index 9a08ff7d..8b064a99 100644 --- a/docs/user/agent-hooks-protocol.md +++ b/docs/user/agent-hooks-protocol.md @@ -85,6 +85,10 @@ hook does not also allow goes through Kit's permission policy again. return a permission error. Blocking takes precedence over replacement but cannot undo side effects; use `tool.before` to prevent execution. +If all `tool.after` subscriptions use `observe` mode, a hook failure does not +block the tool result. Kit displays a warning instead. If a `tool.after` +interceptor is registered, a dispatch failure can still block the result. + Tool events cover the tools that compose scripts call, such as `shell`, `edit`, `read_file` and the MCP `tool` dispatcher. Kit projects text, structured data, tool calls and tool results into categorized message parts. Tool-call IDs and diff --git a/src/ahp/tests.rs b/src/ahp/tests.rs index a00a2a8c..1c04c403 100644 --- a/src/ahp/tests.rs +++ b/src/ahp/tests.rs @@ -313,6 +313,9 @@ impl Tool for Echo { "offline".into(), )); } + if request.input["text"] == "cancelled" { + return ToolExecutionOutcome::Failed(ToolError::Cancelled); + } match self.invoke(request, context).await { Ok(result) => ToolExecutionOutcome::Completed(result), Err(error) => ToolExecutionOutcome::Failed(error), @@ -332,7 +335,11 @@ impl Tool for Echo { .to_owned(); Ok(ToolResult::new(ToolResultPart::success( request.call_id, - ToolOutput::text(text), + if text == "oversized-binary" { + oversized_binary_output() + } else { + ToolOutput::text(text) + }, )) .with_duration(std::time::Duration::from_millis(7))) } @@ -679,6 +686,68 @@ async fn failed_tools_can_inject_recovery_context_without_becoming_successful() ); } +fn oversized_binary_output() -> ToolOutput { + ToolOutput::Parts(vec![Part::Media(agentkit_core::MediaPart::new( + Modality::Image, + "image/png", + DataRef::inline_bytes(vec![42; 4 * 1024 * 1024 + 1]), + ))]) +} + +#[tokio::test] +async fn tool_after_observer_preparation_failure_preserves_native_binary_output() { + let (hooks, backend) = hooks(vec![observe(&["tool.after"])]); + let (sender, receiver) = std::sync::mpsc::channel(); + let _subscription = hooks.subscribe_messages("session".into(), move |text| { + sender.send(text.to_owned()).is_ok() + }); + let echo = Echo::new(); + let outcome = call(&hooks, &echo, json!({"text": "oversized-binary"})).await; + let ToolExecutionOutcome::Completed(result) = outcome else { + panic!("observer preparation failure replaced the native result"); + }; + assert_eq!(result.result.output, oversized_binary_output()); + assert_eq!(result.result.call_id, ToolCallId::new("call")); + assert!(!result.result.is_error); + assert_eq!(result.duration, Some(std::time::Duration::from_millis(7))); + assert_eq!(echo.received.lock().unwrap().len(), 1); + assert!(backend.events("tool.after").is_empty()); + assert_eq!( + receiver.try_iter().collect::>(), + ["tool.after hook dispatch failed (Preparation)"] + ); +} + +#[tokio::test] +async fn tool_after_interceptor_preparation_failure_withholds_native_binary_output() { + for subscriptions in [ + vec![intercept(&["tool.after"])], + vec![observe(&["tool.after"]), intercept(&["tool.after"])], + ] { + let (hooks, backend) = hooks(subscriptions); + let outcome = call(&hooks, &Echo::new(), json!({"text": "oversized-binary"})).await; + let ToolExecutionOutcome::Failed(ToolError::PermissionDenied(denial)) = outcome else { + panic!("interceptor preparation failure did not withhold output"); + }; + assert_eq!( + denial.message, + "tool.after hook dispatch failed (Preparation)" + ); + assert!(backend.events("tool.after").is_empty()); + } +} + +#[tokio::test] +async fn tool_after_preserves_native_cancellation() { + for subscription in [observe(&["tool.after"]), intercept(&["tool.after"])] { + let (hooks, _) = hooks(vec![subscription]); + assert!(matches!( + call(&hooks, &Echo::new(), json!({"text": "cancelled"})).await, + ToolExecutionOutcome::Failed(ToolError::Cancelled) + )); + } +} + #[tokio::test] async fn tool_after_observes_the_output() { let (hooks, backend) = hooks(vec![observe(&["tool.after"])]); diff --git a/src/ahp/tool.rs b/src/ahp/tool.rs index db3c836f..4f212c0b 100644 --- a/src/ahp/tool.rs +++ b/src/ahp/tool.rs @@ -187,6 +187,39 @@ impl HookedTool { None } + fn after_dispatch_failed( + &self, + request: &ToolRequest, + outcome: ToolExecutionOutcome, + context: &ToolContext<'_>, + message: String, + ) -> ToolExecutionOutcome { + if context + .cancellation + .as_ref() + .is_some_and(agentkit_core::TurnCancellation::is_cancelled) + { + return ToolExecutionOutcome::Failed(ToolError::Cancelled); + } + tracing::warn!(event = "tool.after", error = %message, "agent hook dispatch failed"); + self.hooks.deliver_messages( + &request.session_id.to_string(), + &[agenthooksprotocol::generated::MessageEffect::new( + message.clone(), + )], + ); + // Observer delivery is best effort. Interceptors remain fail closed even + // when a payload-dependent route filter might not have selected this call. + // A dispatch failure must not replace a native failure or cancellation. + if !self.hooks.sdk.intercepts(EventType::ToolAfter) + || !matches!(outcome, ToolExecutionOutcome::Completed(_)) + { + outcome + } else { + blocked(message) + } + } + async fn after( &self, request: &ToolRequest, @@ -263,22 +296,42 @@ impl HookedTool { )] }) .unwrap_or_default(); - let (event, _) = match input - .with_sources::() - .with_items(items) - .into_host_event() - { - Ok(event) => event, - Err(error) => return blocked(error.to_string()), + // Preserve owned attachment sources through SDK preparation. + let boundary = self + .hooks + .sdk + .tool_after(input.with_sources::().with_items(items)); + // Keep the SDK's typed cause before dispatch erases it into LoopError. + // Arbitrary error text can contain native output or backend secrets. + let operation = async { + boundary.await.map_err(|error| { + agenthooksprotocol::client::HookError::new(format!( + "tool.after hook dispatch failed ({:?})", + error.code() + )) + }) }; - let boundary = self.hooks.sdk.event_for(EventType::ToolAfter, event); - let result = super::dispatch(boundary, context.cancellation.as_ref()).await; + let result = super::dispatch(operation, context.cancellation.as_ref()).await; if matches!(&result, Err(agentkit_loop::LoopError::Cancelled)) { return ToolExecutionOutcome::Failed(ToolError::Cancelled); } + let result = match result { + Ok(result) => result, + Err(agentkit_loop::LoopError::InvalidState(message)) => { + return self.after_dispatch_failed(request, outcome, context, message); + } + Err(_) => { + return self.after_dispatch_failed( + request, + outcome, + context, + "tool.after hook dispatch failed".into(), + ); + } + }; let edited = self.hooks.settle( EventType::ToolAfter, - result, + Ok(result), &request.session_id.to_string(), ); if let Some(edited) = &edited