fix(ahp): preserve tool results when observers fail

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

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::<Vec<_>>(),
+        ["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"])]);

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::<Attachment>()
-            .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::<Attachment>().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