fix install_skill rollback on origin metadata write failure

This commit is contained in:
lxowalle 2026-04-12 23:34:39 +08:00
parent 087e4f9e7e
commit 0205dc614f
2 changed files with 38 additions and 2 deletions

View file

@ -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.

View file

@ -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))
}