From 0205dc614f5acd39676b7998063b0cd0afd6545f Mon Sep 17 00:00:00 2001
From: lxowalle <lxowalle@outlook.com>
Date: Sun, 12 Apr 2026 23:34:39 +0800
Subject: [PATCH] fix install_skill rollback on origin metadata write failure

---
 pkg/tools/skills_install.go      | 15 +++++++++++++--
 pkg/tools/skills_install_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/pkg/tools/skills_install.go b/pkg/tools/skills_install.go
index 19a34d1cc..be8206edc 100644
--- a/pkg/tools/skills_install.go
+++ b/pkg/tools/skills_install.go
@@ -18,6 +18,8 @@ import (
 
 const defaultSkillRegistryName = "github"
 
+var persistInstalledSkillOriginMeta = writeOriginMeta
+
 // InstallSkillTool allows the LLM agent to install skills from registries.
 // It shares the same RegistryManager that FindSkillsTool uses,
 // so all registries configured in config are available for installation.
@@ -170,7 +172,7 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To
 	}
 
 	// Write origin metadata.
-	if err := writeOriginMeta(targetDir, registry, slug, result.Version); err != nil {
+	if err := persistInstalledSkillOriginMeta(targetDir, registry, slug, result.Version); err != nil {
 		logger.ErrorCF("tool", "Failed to write origin metadata",
 			map[string]any{
 				"tool":     "install_skill",
@@ -180,7 +182,16 @@ func (t *InstallSkillTool) Execute(ctx context.Context, args map[string]any) *To
 				"slug":     slug,
 				"version":  result.Version,
 			})
-		_ = err
+		rmErr := os.RemoveAll(targetDir)
+		if rmErr != nil {
+			logger.ErrorCF("tool", "Failed to roll back install after metadata write failure",
+				map[string]any{
+					"tool":       "install_skill",
+					"target_dir": targetDir,
+					"error":      rmErr.Error(),
+				})
+		}
+		return ErrorResult(fmt.Sprintf("failed to persist skill metadata for %q: %v", slug, err))
 	}
 
 	// Build result with moderation warning if suspicious.
diff --git a/pkg/tools/skills_install_test.go b/pkg/tools/skills_install_test.go
index 7e051368d..f9248743d 100644
--- a/pkg/tools/skills_install_test.go
+++ b/pkg/tools/skills_install_test.go
@@ -283,3 +283,28 @@ func TestInstallSkillToolRejectsInvalidInstalledSkill(t *testing.T) {
 	_, err := os.Stat(filepath.Join(workspace, "skills", "broken-skill"))
 	assert.True(t, os.IsNotExist(err))
 }
+
+func TestInstallSkillToolRollsBackOnOriginMetadataWriteFailure(t *testing.T) {
+	workspace := t.TempDir()
+	registryMgr := skills.NewRegistryManager()
+	registryMgr.AddRegistry(&mockInstallRegistry{})
+	tool := NewInstallSkillTool(registryMgr, workspace)
+
+	previousPersist := persistInstalledSkillOriginMeta
+	persistInstalledSkillOriginMeta = func(string, skills.SkillRegistry, string, string) error {
+		return assert.AnError
+	}
+	defer func() {
+		persistInstalledSkillOriginMeta = previousPersist
+	}()
+
+	result := tool.Execute(context.Background(), map[string]any{
+		"slug":     "rollback-skill",
+		"registry": "clawhub",
+	})
+
+	assert.True(t, result.IsError)
+	assert.Contains(t, result.ForLLM, "failed to persist skill metadata")
+	_, err := os.Stat(filepath.Join(workspace, "skills", "rollback-skill"))
+	assert.True(t, os.IsNotExist(err))
+}