fix: destructive actions from ambiguous user prompts (#782)

* fix: destructive actions from ambiguous user prompts

* review fixes

* review fixes

src/tools/builtin/extension_tools.rs

@@ -451,8 +451,8 @@ impl Tool for ToolRemoveTool {
     }
 
     fn description(&self) -> &str {
-        "Remove an installed extension (channel, tool, or MCP server). \
-         Unregisters tools and deletes configuration."
+        "Permanently remove an installed extension (channel, tool, or MCP server) from disk. \
+         This action cannot be undone — the WASM binary and configuration files will be deleted."
     }
 
     fn parameters_schema(&self) -> serde_json::Value {
@@ -492,7 +492,7 @@ impl Tool for ToolRemoveTool {
     }
 
     fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
-        ApprovalRequirement::UnlessAutoApproved
+        ApprovalRequirement::Always
     }
 }
 
@@ -701,10 +701,38 @@ mod tests {
         assert_eq!(tool.name(), "tool_remove");
         assert_eq!(
             tool.requires_approval(&serde_json::json!({})),
-            ApprovalRequirement::UnlessAutoApproved
+            ApprovalRequirement::Always
         );
     }
 
+    #[test]
+    fn tool_remove_always_requires_approval_regardless_of_params() {
+        use crate::tools::tool::ApprovalRequirement;
+        let tool = ToolRemoveTool {
+            manager: test_manager_stub(),
+        };
+
+        let test_cases = vec![
+            ("no params", serde_json::json!({})),
+            ("empty name", serde_json::json!({"name": ""})),
+            ("slack", serde_json::json!({"name": "slack"})),
+            ("github-cli", serde_json::json!({"name": "github-cli"})),
+            (
+                "with extra fields",
+                serde_json::json!({"name": "tool", "extra": "field"}),
+            ),
+        ];
+
+        for (case_name, params) in test_cases {
+            assert_eq!(
+                tool.requires_approval(&params),
+                ApprovalRequirement::Always,
+                "tool_remove must always require approval for case: {}",
+                case_name
+            );
+        }
+    }
+
     #[test]
     fn test_tool_upgrade_schema() {
         use crate::tools::tool::ApprovalRequirement;

src/tools/builtin/skill_tools.rs

@@ -709,7 +709,8 @@ impl Tool for SkillRemoveTool {
     }
 
     fn description(&self) -> &str {
-        "Remove an installed skill by name. Only user-installed skills can be removed."
+        "Permanently remove an installed skill from disk. This action cannot be undone — \
+         the skill files will be deleted."
     }
 
     fn parameters_schema(&self) -> serde_json::Value {
@@ -770,7 +771,7 @@ impl Tool for SkillRemoveTool {
     }
 
     fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
-        ApprovalRequirement::UnlessAutoApproved
+        ApprovalRequirement::Always
     }
 }
 
@@ -837,12 +838,41 @@ mod tests {
         assert_eq!(tool.name(), "skill_remove");
         assert_eq!(
             tool.requires_approval(&serde_json::json!({})),
-            ApprovalRequirement::UnlessAutoApproved
+            ApprovalRequirement::Always
         );
         let schema = tool.parameters_schema();
         assert!(schema["properties"].get("name").is_some());
     }
 
+    #[test]
+    fn skill_remove_always_requires_approval_regardless_of_params() {
+        use crate::tools::tool::ApprovalRequirement;
+        let tool = SkillRemoveTool::new(test_registry());
+
+        let test_cases = vec![
+            ("no params", serde_json::json!({})),
+            ("empty name", serde_json::json!({"name": ""})),
+            (
+                "deployment skill",
+                serde_json::json!({"name": "deployment"}),
+            ),
+            ("custom skill", serde_json::json!({"name": "custom-skill"})),
+            (
+                "with extra fields",
+                serde_json::json!({"name": "skill", "extra": "field"}),
+            ),
+        ];
+
+        for (case_name, params) in test_cases {
+            assert_eq!(
+                tool.requires_approval(&params),
+                ApprovalRequirement::Always,
+                "skill_remove must always require approval for case: {}",
+                case_name
+            );
+        }
+    }
+
     #[test]
     fn test_validate_fetch_url_allows_https() {
         assert!(super::validate_fetch_url("https://clawhub.ai/api/v1/download?slug=foo").is_ok());