From 442ffa9c9e46daabdfe8cc79f32ec9b480f521df Mon Sep 17 00:00:00 2001 From: matevip Date: Fri, 1 May 2026 09:49:44 +0800 Subject: [PATCH] fix(skill): clean separation of install / uninstall / hard-delete --- .../skill/controller/SkillController.java | 13 +++- .../mate/skill/installer/SkillInstaller.java | 10 +-- .../mate/skill/repository/SkillMapper.java | 12 ++++ .../vip/mate/skill/service/SkillService.java | 65 ++++++++++++++++--- .../workspace/SkillWorkspaceManager.java | 27 ++++++++ .../mate/tool/builtin/SkillManageTool.java | 9 ++- 6 files changed, 117 insertions(+), 19 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java index fce7ac12..a2c269c7 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/controller/SkillController.java @@ -103,10 +103,19 @@ public class SkillController { return R.ok(skillService.updateSkill(skill)); } - @Operation(summary = "删除技能") + /** + * RFC-090 §14.5 — admin-only hard delete: physical row removal + + * workspace purge. UI surfaces this as "permanently delete" and + * confirms with a destructive warning. The routine user-facing + * "remove" button on the skill card calls + * {@code DELETE /skills/install/{name}} instead, which goes through + * {@link vip.mate.skill.installer.SkillInstaller#uninstall} for the + * recoverable logical-delete + archive path. + */ + @Operation(summary = "硬删除技能 (admin only — 物理删除 + 工作区清空)") @DeleteMapping("/{id}") public R delete(@PathVariable Long id) { - skillService.deleteSkill(id); + skillService.hardDeleteSkill(id); return R.ok(); } diff --git a/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillInstaller.java b/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillInstaller.java index 7bd33ea6..cf6a6749 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillInstaller.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/installer/SkillInstaller.java @@ -74,10 +74,12 @@ public class SkillInstaller { } /** - * 卸载 skill（归档 workspace + 删除数据库记录） + * RFC-090 §14.5 — user-facing uninstall: logical delete + workspace + * archive. Re-installing the same name later is supported. For + * admin-only physical removal, call + * {@code SkillService.hardDeleteSkill} via {@code DELETE /skills/{id}}. */ public void uninstall(String skillName) { - // 先在数据库中查找 List skills = skillService.listSkills(); SkillEntity target = skills.stream() .filter(s -> s.getName().equals(skillName)) @@ -85,10 +87,8 @@ public class SkillInstaller { .orElse(null); if (target != null) { - skillService.deleteSkill(target.getId()); + skillService.uninstallSkill(target.getId()); } - - // workspace 归档已在 SkillService.deleteSkill 中处理 log.info("Uninstalled skill: {}", skillName); } diff --git a/mateclaw-server/src/main/java/vip/mate/skill/repository/SkillMapper.java b/mateclaw-server/src/main/java/vip/mate/skill/repository/SkillMapper.java index ce8f38d7..9ee7d846 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/repository/SkillMapper.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/repository/SkillMapper.java @@ -1,7 +1,9 @@ package vip.mate.skill.repository; import com.baomidou.mybatisplus.core.mapper.BaseMapper; +import org.apache.ibatis.annotations.Delete; import org.apache.ibatis.annotations.Mapper; +import org.apache.ibatis.annotations.Param; import vip.mate.skill.model.SkillEntity; /** @@ -11,4 +13,14 @@ import vip.mate.skill.model.SkillEntity; */ @Mapper public interface SkillMapper extends BaseMapper { + + /** + * RFC-090 §14.5 — physical delete bypassing the {@code deleted} + * logical-delete flag. Used by the admin "hard delete" path + * ({@code DELETE /skills/{id}}); the user-facing "uninstall" path + * still goes through {@link BaseMapper#deleteById} so the row can + * be recovered by re-installing the same skill name. + */ + @Delete("DELETE FROM mate_skill WHERE id = #{id}") + int hardDeleteById(@Param("id") Long id); } diff --git a/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java b/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java index be023fe4..4be03b24 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java @@ -279,28 +279,75 @@ public class SkillService { } /** - * 删除技能 - * 内置技能不可删除 + * RFC-090 §14.5 — uninstall path: logical delete (row stays in DB + * with {@code deleted=1}) + archive workspace to + * {@code .archived/}. + * + *

Recoverable: the user can re-install a skill of the same name + * later, since the archived workspace and soft-deleted row stay + * out of every standard query but on disk. + * + *

Builtin skills are protected: uninstalling a builtin would + * leave the seed re-creating it on next boot, so we surface a + * clear error instead of doing partial work. */ - public void deleteSkill(Long id) { + public void uninstallSkill(Long id) { SkillEntity skill = getSkill(id); if (Boolean.TRUE.equals(skill.getBuiltin())) { - throw new MateClawException("err.skill.builtin_readonly", "内置技能不可删除: " + skill.getName()); + throw new MateClawException("err.skill.builtin_readonly", + "内置技能不可卸载: " + skill.getName()); } - skillMapper.deleteById(id); - log.info("Deleted skill: {}", skill.getName()); + skillMapper.deleteById(id); // logical delete (deleted=1) + log.info("Uninstalled skill (logical delete + archive): {}", skill.getName()); - // 归档工作区目录 if ("archive".equals(workspaceProperties.getDeletePolicy())) { workspaceManager.archiveWorkspace(skill.getName()); } - - // 刷新 runtime cache if (runtimeService != null) { runtimeService.refreshActiveSkills(); } } + /** + * RFC-090 §14.5 — hard-delete path: physical SQL delete + workspace + * purge. Admin only. Not recoverable. + * + *

Use this when you need to free the slug for an unrelated skill + * or scrub a row that's been corrupted. UI buttons should call + * {@link #uninstallSkill} instead unless the user explicitly chose + * "permanently delete". + */ + public void hardDeleteSkill(Long id) { + SkillEntity skill = getSkill(id); + if (Boolean.TRUE.equals(skill.getBuiltin())) { + throw new MateClawException("err.skill.builtin_readonly", + "内置技能不可硬删除: " + skill.getName()); + } + skillMapper.hardDeleteById(id); // bypass the logical-delete flag + log.info("Hard-deleted skill (physical delete + purge): {}", skill.getName()); + + workspaceManager.purgeWorkspace(skill.getName()); + + if (runtimeService != null) { + runtimeService.refreshActiveSkills(); + } + } + + /** + * Legacy alias maintained for callers that still use {@code + * deleteSkill}. Routes to the user-friendly uninstall semantics + * (logical delete + archive). New code should pick {@link + * #uninstallSkill} or {@link #hardDeleteSkill} explicitly. + * + * @deprecated Use {@link #uninstallSkill} for user-facing UI delete + * (recoverable) or {@link #hardDeleteSkill} for admin + * "permanent" delete. + */ + @Deprecated + public void deleteSkill(Long id) { + uninstallSkill(id); + } + /** * 启用/禁用技能 */ diff --git a/mateclaw-server/src/main/java/vip/mate/skill/workspace/SkillWorkspaceManager.java b/mateclaw-server/src/main/java/vip/mate/skill/workspace/SkillWorkspaceManager.java index f4a9afa4..202d27aa 100644 --- a/mateclaw-server/src/main/java/vip/mate/skill/workspace/SkillWorkspaceManager.java +++ b/mateclaw-server/src/main/java/vip/mate/skill/workspace/SkillWorkspaceManager.java @@ -136,6 +136,33 @@ public class SkillWorkspaceManager { } } + /** + * RFC-090 §14.5 — physically remove the workspace directory. Used + * by hard-delete only; uninstall still calls + * {@link #archiveWorkspace} so users can recover by re-installing. + */ + public void purgeWorkspace(String skillName) { + Path workspaceDir = resolveConventionPath(skillName); + if (!Files.exists(workspaceDir)) return; + try { + // Walk and delete bottom-up so non-empty dirs go away too. + try (var stream = Files.walk(workspaceDir)) { + stream.sorted(java.util.Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + /* leave partial cleanup; best effort */ + } + }); + } + log.info("Purged skill workspace: {}", workspaceDir); + eventPublisher.publishEvent(new SkillWorkspaceEvent(skillName, + SkillWorkspaceEvent.Type.ARCHIVED, workspaceDir)); + } catch (IOException e) { + log.warn("Failed to purge workspace for skill '{}': {}", skillName, e.getMessage()); + } + } + /** * 归档 workspace 到 {root}/.archived/{name}-{timestamp}/ */ diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillManageTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillManageTool.java index 7340272c..c0444551 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillManageTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillManageTool.java @@ -309,9 +309,12 @@ public class SkillManageTool { } try { - skillService.deleteSkill(existing.getId()); - log.info("[SkillManage] Agent deleted skill: name={}", name); - return "Skill '" + name + "' deleted."; + // RFC-090 §14.5 — agent-triggered delete uses uninstall + // (logical + archive) so a misbehaving agent can't + // physically purge a row past recovery. + skillService.uninstallSkill(existing.getId()); + log.info("[SkillManage] Agent uninstalled skill: name={}", name); + return "Skill '" + name + "' uninstalled (workspace archived)."; } catch (Exception e) { log.error("[SkillManage] Failed to delete skill '{}': {}", name, e.getMessage(), e); return "Error deleting skill: " + e.getMessage();