From 20bdf1e992a889667ed1f33ad7f348f83a02c371 Mon Sep 17 00:00:00 2001 From: Zhizhen He Date: Mon, 5 Oct 2026 22:49:17 +0800 Subject: [PATCH] library: keep edited skill copies when hashing fails (#916) Both folders can hash to an empty string after read failures. Treating those hashes as equal makes relink delete the agent's copy without a backup. Require a nonempty matching hash so an unreadable copy takes the existing backup path. Cover switching from copies to links with unreadable files in both folders and edited notes in the agent's copy. --- internal/library/skill_how_test.go | 40 ++++++++++++++++++++++++++++++ internal/library/skills.go | 8 ++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/internal/library/skill_how_test.go b/internal/library/skill_how_test.go index c8a0e2af..f676c49a 100644 --- a/internal/library/skill_how_test.go +++ b/internal/library/skill_how_test.go @@ -4,6 +4,7 @@ import ( "io/fs" "os" "path/filepath" + "runtime" "slices" "strings" "testing" @@ -140,3 +141,42 @@ func TestSkillHow(t *testing.T) { t.Error("the user's own pdf went with the library's") } } + +// Two failed hashes don't prove a copy is unchanged: switching back to a +// link must keep the agent's edits even when neither folder can be read. +func TestSkillRelinkKeepsUnreadableCopy(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("requires Unix read permissions without root") + } + h := sandbox(t) + src := filepath.Join(h, "src/skills") + skill(t, filepath.Join(src, "pdf"), "pdf", "Read PDFs") + write(t, filepath.Join(src, "pdf/blocked.txt"), "unchanged\n") + ok(t)(InstallSkills(src, []string{"pdf"}, []string{"claude"})) + ok(t)(SetSkillHow("", HowCopy)) + p := filepath.Join(h, ".claude/skills/pdf") + write(t, filepath.Join(p, "notes.md"), "my notes\n") + for _, dir := range []string{filepath.Join(src, "pdf"), p} { + if err := os.Chmod(filepath.Join(dir, "blocked.txt"), 0); err != nil { + t.Fatal(err) + } + if hashDir(dir) != "" { + t.Fatal("the unreadable file didn't prevent hashing", dir) + } + } + + ok(t)(SetSkillHow("", HowLink)) + if !isLink(t, p) { + t.Fatal("claude's copy wasn't replaced by a link") + } + var kept []string + filepath.WalkDir(BackupDir(), func(p string, d fs.DirEntry, err error) error { + if err == nil && d.Name() == "notes.md" { + kept = append(kept, p) + } + return nil + }) + if len(kept) != 1 || read(t, kept[0]) != "my notes\n" { + t.Fatalf("the unreadable copy's edits weren't kept: %v", kept) + } +} diff --git a/internal/library/skills.go b/internal/library/skills.go index 8ff33dc4..da39c2cf 100644 --- a/internal/library/skills.go +++ b/internal/library/skills.go @@ -254,8 +254,12 @@ func isDigits(s string) bool { return true } -// fresh is whether the copy at p holds what the library's skill does. -func fresh(p, name string) bool { return hashDir(p) == hashDir(realDir(skillDir(name))) } +// fresh is whether both folders could be read and the copy at p holds +// what the library's skill does. Failed hashes don't prove it can be discarded. +func fresh(p, name string) bool { + h := hashDir(p) + return h != "" && h == hashDir(realDir(skillDir(name))) +} func unlink(p string) error { fi, err := os.Lstat(p)