Merge branch 'dori/hooks-parity' into main

rust/crates/plugins/src/lib.rs

@@ -1629,6 +1629,12 @@ fn build_plugin_manifest(
     let permissions = build_manifest_permissions(&raw.permissions, &mut errors);
     validate_command_entries(root, raw.hooks.pre_tool_use.iter(), "hook", &mut errors);
     validate_command_entries(root, raw.hooks.post_tool_use.iter(), "hook", &mut errors);
+    validate_command_entries(
+        root,
+        raw.hooks.post_tool_use_failure.iter(),
+        "hook",
+        &mut errors,
+    );
     validate_command_entries(
         root,
         raw.lifecycle.init.iter(),
@@ -2306,6 +2312,16 @@ mod tests {
         );
     }
 
+    fn write_broken_failure_hook_plugin(root: &Path, name: &str) {
+        write_file(
+            root.join(MANIFEST_RELATIVE_PATH).as_path(),
+            format!(
+                "{{\n  \"name\": \"{name}\",\n  \"version\": \"1.0.0\",\n  \"description\": \"broken plugin\",\n  \"hooks\": {{\n    \"PostToolUseFailure\": [\"./hooks/missing-failure.sh\"]\n  }}\n}}"
+            )
+            .as_str(),
+        );
+    }
+
     fn write_lifecycle_plugin(root: &Path, name: &str, version: &str) -> PathBuf {
         let log_path = root.join("lifecycle.log");
         write_file(
@@ -3178,14 +3194,19 @@ mod tests {
 
     #[test]
     fn rejects_plugin_sources_with_missing_hook_paths() {
+        // given
         let config_home = temp_dir("broken-home");
         let source_root = temp_dir("broken-source");
         write_broken_plugin(&source_root, "broken");
 
         let manager = PluginManager::new(PluginManagerConfig::new(&config_home));
+
+        // when
         let error = manager
             .validate_plugin_source(source_root.to_str().expect("utf8 path"))
             .expect_err("missing hook file should fail validation");
+
+        // then
         assert!(error.to_string().contains("does not exist"));
 
         let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home));
@@ -3198,6 +3219,33 @@ mod tests {
         let _ = fs::remove_dir_all(source_root);
     }
 
+    #[test]
+    fn rejects_plugin_sources_with_missing_failure_hook_paths() {
+        // given
+        let config_home = temp_dir("broken-failure-home");
+        let source_root = temp_dir("broken-failure-source");
+        write_broken_failure_hook_plugin(&source_root, "broken-failure");
+
+        let manager = PluginManager::new(PluginManagerConfig::new(&config_home));
+
+        // when
+        let error = manager
+            .validate_plugin_source(source_root.to_str().expect("utf8 path"))
+            .expect_err("missing failure hook file should fail validation");
+
+        // then
+        assert!(error.to_string().contains("does not exist"));
+
+        let mut manager = PluginManager::new(PluginManagerConfig::new(&config_home));
+        let install_error = manager
+            .install(source_root.to_str().expect("utf8 path"))
+            .expect_err("install should reject invalid failure hook paths");
+        assert!(install_error.to_string().contains("does not exist"));
+
+        let _ = fs::remove_dir_all(config_home);
+        let _ = fs::remove_dir_all(source_root);
+    }
+
     #[test]
     fn plugin_registry_runs_initialize_and_shutdown_for_enabled_plugins() {
         let config_home = temp_dir("lifecycle-home");

rust/crates/runtime/src/conversation.rs

@@ -785,6 +785,7 @@ mod tests {
     use crate::prompt::{ProjectContext, SystemPromptBuilder};
     use crate::session::{ContentBlock, MessageRole, Session};
     use crate::usage::TokenUsage;
+    use crate::ToolError;
     use std::fs;
     use std::path::PathBuf;
     use std::sync::Arc;
@@ -1202,6 +1203,85 @@ mod tests {
         );
     }
 
+    #[test]
+    fn appends_post_tool_use_failure_hook_feedback_to_tool_result() {
+        struct TwoCallApiClient {
+            calls: usize,
+        }
+
+        impl ApiClient for TwoCallApiClient {
+            fn stream(&mut self, request: ApiRequest) -> Result<Vec<AssistantEvent>, RuntimeError> {
+                self.calls += 1;
+                match self.calls {
+                    1 => Ok(vec![
+                        AssistantEvent::ToolUse {
+                            id: "tool-1".to_string(),
+                            name: "fail".to_string(),
+                            input: r#"{"path":"README.md"}"#.to_string(),
+                        },
+                        AssistantEvent::MessageStop,
+                    ]),
+                    2 => {
+                        assert!(request
+                            .messages
+                            .iter()
+                            .any(|message| message.role == MessageRole::Tool));
+                        Ok(vec![
+                            AssistantEvent::TextDelta("done".to_string()),
+                            AssistantEvent::MessageStop,
+                        ])
+                    }
+                    _ => Err(RuntimeError::new("unexpected extra API call")),
+                }
+            }
+        }
+
+        // given
+        let mut runtime = ConversationRuntime::new_with_features(
+            Session::new(),
+            TwoCallApiClient { calls: 0 },
+            StaticToolExecutor::new()
+                .register("fail", |_input| Err(ToolError::new("tool exploded"))),
+            PermissionPolicy::new(PermissionMode::DangerFullAccess),
+            vec!["system".to_string()],
+            &RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new(
+                Vec::new(),
+                vec![shell_snippet("printf 'post hook should not run'")],
+                vec![shell_snippet("printf 'failure hook ran'")],
+            )),
+        );
+
+        // when
+        let summary = runtime
+            .run_turn("use fail", None)
+            .expect("tool loop succeeds");
+
+        // then
+        assert_eq!(summary.tool_results.len(), 1);
+        let ContentBlock::ToolResult {
+            is_error, output, ..
+        } = &summary.tool_results[0].blocks[0]
+        else {
+            panic!("expected tool result block");
+        };
+        assert!(
+            *is_error,
+            "failure hook path should preserve error result: {output:?}"
+        );
+        assert!(
+            output.contains("tool exploded"),
+            "tool output missing failure reason: {output:?}"
+        );
+        assert!(
+            output.contains("failure hook ran"),
+            "tool output missing failure hook feedback: {output:?}"
+        );
+        assert!(
+            !output.contains("post hook should not run"),
+            "normal post hook should not run on tool failure: {output:?}"
+        );
+    }
+
     #[test]
     fn reconstructs_usage_tracker_from_restored_session() {
         struct SimpleApi;

rust/crates/runtime/src/hooks.rs

@@ -806,19 +806,50 @@ mod tests {
 
     #[test]
     fn runs_post_tool_use_failure_hooks() {
+        // given
         let runner = HookRunner::new(RuntimeHookConfig::new(
             Vec::new(),
             Vec::new(),
             vec![shell_snippet("printf 'failure hook ran'")],
         ));
 
+        // when
         let result =
             runner.run_post_tool_use_failure("bash", r#"{"command":"false"}"#, "command failed");
 
+        // then
         assert!(!result.is_denied());
         assert_eq!(result.messages(), &["failure hook ran".to_string()]);
     }
 
+    #[test]
+    fn stops_running_failure_hooks_after_failure() {
+        // given
+        let runner = HookRunner::new(RuntimeHookConfig::new(
+            Vec::new(),
+            Vec::new(),
+            vec![
+                shell_snippet("printf 'broken failure hook'; exit 1"),
+                shell_snippet("printf 'later failure hook'"),
+            ],
+        ));
+
+        // when
+        let result =
+            runner.run_post_tool_use_failure("bash", r#"{"command":"false"}"#, "command failed");
+
+        // then
+        assert!(result.is_failed());
+        assert!(result
+            .messages()
+            .iter()
+            .any(|message| message.contains("broken failure hook")));
+        assert!(!result
+            .messages()
+            .iter()
+            .any(|message| message == "later failure hook"));
+    }
+
     #[test]
     fn executes_hooks_in_configured_order() {
         // given