Skip to content

Commit e82d18c

Browse files
committed
fix: sync must not prune registry entries for installed skills
Sync's registry prune logic removed entries for installed skills whose files were missing from disk. This was wrong: the registry is the authoritative record of installations managed by install/uninstall. Remove PruneStaleSkills calls from sync (CLI global, CLI project, and web UI handler). Reconcile (called during install) still uses prune with full metadata context, which is correct. Fixes: running 'sync' after 'install' would clear registry.yaml when the installed skill's source files were not yet on disk.
1 parent 9ddff32 commit e82d18c

7 files changed

Lines changed: 91 additions & 76 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## [0.18.8] - 2026-04-06
4+
5+
### Bug Fixes
6+
7+
- **Sync no longer deletes registry entries for installed skills** — running `skillshare sync` (or project-mode `sync -p`) would silently remove `registry.yaml` entries for skills whose source files were not present on disk. This meant that installing a skill and then syncing could erase the installation record entirely. Sync now leaves the registry untouched — only `install` and `uninstall` manage registry entries
8+
39
## [0.18.7] - 2026-04-04
410

511
### New Features

cmd/skillshare/sync.go

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -240,33 +240,9 @@ func cmdSync(args []string) error {
240240
}
241241
}
242242

243-
// Prune stale registry entries using already-discovered skills (avoids second walk)
244-
if !dryRun {
245-
regDir := cfg.RegistryDir
246-
if regDir == "" {
247-
regDir = config.SourceRoot(cfg.Source)
248-
}
249-
reg, regErr := config.LoadRegistry(regDir)
250-
if regErr == nil {
251-
live := make(map[string]bool, len(discoveredSkills))
252-
for _, s := range discoveredSkills {
253-
live[s.RelPath] = true
254-
}
255-
// Ignored skills still exist on disk — don't prune their registry entries
256-
if ignoreStats != nil {
257-
for _, p := range ignoreStats.IgnoredSkills {
258-
live[p] = true
259-
}
260-
}
261-
var pruneChanged bool
262-
reg.Skills, pruneChanged = config.PruneStaleSkills(reg.Skills, live, false)
263-
if pruneChanged {
264-
if err := reg.Save(regDir); err != nil {
265-
fmt.Fprintf(os.Stderr, "warning: failed to save registry after prune: %v\n", err)
266-
}
267-
}
268-
}
269-
}
243+
// Registry entries are managed by install/uninstall, not sync.
244+
// Sync only manages symlinks — it must not prune registry entries
245+
// for installed skills whose files may be missing from disk.
270246

271247
logSyncOp(config.ConfigPath(), syncLogStats{
272248
Targets: len(cfg.Targets),

cmd/skillshare/sync_project.go

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package main
33
import (
44
"fmt"
55
"os"
6-
"path/filepath"
76
"time"
87

98
"skillshare/internal/config"
@@ -136,24 +135,9 @@ func cmdSyncProject(root string, dryRun, force, jsonOutput bool) (syncLogStats,
136135
}
137136
}
138137

139-
// Prune stale registry entries using already-discovered skills (avoids second walk)
140-
live := make(map[string]bool, len(discoveredSkills))
141-
for _, s := range discoveredSkills {
142-
live[s.RelPath] = true
143-
}
144-
// Ignored skills still exist on disk — don't prune their registry entries
145-
if ignoreStats != nil {
146-
for _, p := range ignoreStats.IgnoredSkills {
147-
live[p] = true
148-
}
149-
}
150-
var pruneChanged bool
151-
runtime.registry.Skills, pruneChanged = config.PruneStaleSkills(runtime.registry.Skills, live, true)
152-
if pruneChanged {
153-
if err := runtime.registry.Save(filepath.Join(root, ".skillshare")); err != nil {
154-
fmt.Fprintf(os.Stderr, "warning: failed to save registry after prune: %v\n", err)
155-
}
156-
}
138+
// Registry entries are managed by install/uninstall, not sync.
139+
// Sync only manages symlinks — it must not prune registry entries
140+
// for installed skills whose files may be missing from disk.
157141
}
158142

159143
if failedTargets > 0 {

internal/server/handler_sync.go

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ package server
22

33
import (
44
"encoding/json"
5-
"log"
65
"maps"
76
"net/http"
8-
"path/filepath"
97
"time"
108

119
"skillshare/internal/config"
@@ -80,34 +78,9 @@ func (s *Server) handleSync(w http.ResponseWriter, r *http.Request) {
8078
warnings = append(warnings, "source directory is empty (0 skills)")
8179
}
8280

83-
// Prune stale registry entries using already-discovered skills (avoids second walk).
84-
// Uses defer so early returns from target errors still trigger cleanup.
85-
if !body.DryRun {
86-
defer func() {
87-
live := make(map[string]bool, len(allSkills))
88-
for _, sk := range allSkills {
89-
live[sk.RelPath] = true
90-
}
91-
// Ignored skills still exist on disk — don't prune their registry entries
92-
if ignoreStats != nil {
93-
for _, p := range ignoreStats.IgnoredSkills {
94-
live[p] = true
95-
}
96-
}
97-
skillsOnly := s.IsProjectMode() // project registries also store agent entries
98-
var pruneChanged bool
99-
s.registry.Skills, pruneChanged = config.PruneStaleSkills(s.registry.Skills, live, skillsOnly)
100-
if pruneChanged {
101-
regDir := s.cfg.RegistryDir
102-
if s.IsProjectMode() {
103-
regDir = filepath.Join(s.projectRoot, ".skillshare")
104-
}
105-
if err := s.registry.Save(regDir); err != nil {
106-
log.Printf("warning: failed to save registry after prune: %v", err)
107-
}
108-
}
109-
}()
110-
}
81+
// Registry entries are managed by install/uninstall, not sync.
82+
// Sync only manages symlinks — it must not prune registry entries
83+
// for installed skills whose files may be missing from disk.
11184

11285
results := make([]syncTargetResult, 0)
11386

tests/integration/sync_project_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package integration
55
import (
66
"os"
77
"path/filepath"
8+
"strings"
89
"testing"
910

1011
"skillshare/internal/testutil"
@@ -110,6 +111,39 @@ func TestSyncProject_DryRun_NoChanges(t *testing.T) {
110111
}
111112
}
112113

114+
func TestSyncProject_PreservesRegistryEntries(t *testing.T) {
115+
sb := testutil.NewSandbox(t)
116+
defer sb.Cleanup()
117+
projectRoot := sb.SetupProjectDir("claude")
118+
119+
// Create a skill on disk that sync will discover
120+
sb.CreateProjectSkill(projectRoot, "local-skill", map[string]string{
121+
"SKILL.md": "# Local Skill",
122+
})
123+
124+
// Write a registry with a remote-installed skill that has NO files on disk.
125+
// Sync must NOT prune this entry — the registry is the source of truth for installations.
126+
registryPath := filepath.Join(projectRoot, ".skillshare", "registry.yaml")
127+
registryContent := "skills:\n - name: remote-tool\n source: github.com/someone/remote-tool\n"
128+
os.WriteFile(registryPath, []byte(registryContent), 0644)
129+
130+
result := sb.RunCLIInDir(projectRoot, "sync", "-p")
131+
result.AssertSuccess(t)
132+
133+
// Verify registry still contains the remote-tool entry
134+
data, err := os.ReadFile(registryPath)
135+
if err != nil {
136+
t.Fatalf("failed to read registry: %v", err)
137+
}
138+
content := string(data)
139+
if !strings.Contains(content, "remote-tool") {
140+
t.Errorf("sync should preserve registry entry for installed skill without local files, got:\n%s", content)
141+
}
142+
if !strings.Contains(content, "github.com/someone/remote-tool") {
143+
t.Errorf("sync should preserve source in registry entry, got:\n%s", content)
144+
}
145+
}
146+
113147
func TestSyncProject_AutoDetectsMode(t *testing.T) {
114148
sb := testutil.NewSandbox(t)
115149
defer sb.Cleanup()

tests/integration/sync_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/json"
77
"os"
88
"path/filepath"
9+
"strings"
910
"testing"
1011

1112
"skillshare/internal/testutil"
@@ -464,6 +465,41 @@ targets:
464465
}
465466
}
466467

468+
func TestSync_PreservesRegistryEntries(t *testing.T) {
469+
sb := testutil.NewSandbox(t)
470+
defer sb.Cleanup()
471+
472+
// Create a local skill that sync will discover
473+
sb.CreateSkill("local-skill", map[string]string{"SKILL.md": "# Local"})
474+
475+
targetPath := sb.CreateTarget("claude")
476+
sb.WriteConfig(`source: ` + sb.SourcePath + `
477+
mode: merge
478+
targets:
479+
claude:
480+
path: ` + targetPath + `
481+
`)
482+
483+
// Write a registry with a remote-installed skill that has NO files on disk.
484+
// Sync must NOT prune this entry — registry is the source of truth for installations.
485+
registryPath := filepath.Join(sb.SourcePath, "registry.yaml")
486+
registryContent := "skills:\n - name: remote-tool\n source: github.com/someone/remote-tool\n"
487+
os.WriteFile(registryPath, []byte(registryContent), 0644)
488+
489+
result := sb.RunCLI("sync")
490+
result.AssertSuccess(t)
491+
492+
// Verify registry still contains the remote-tool entry
493+
data, err := os.ReadFile(registryPath)
494+
if err != nil {
495+
t.Fatalf("failed to read registry: %v", err)
496+
}
497+
content := string(data)
498+
if !strings.Contains(content, "remote-tool") {
499+
t.Errorf("sync should preserve registry entry for installed skill without local files, got:\n%s", content)
500+
}
501+
}
502+
467503
func TestSync_MergeMode_IncludeFilter(t *testing.T) {
468504
sb := testutil.NewSandbox(t)
469505
defer sb.Cleanup()

website/src/pages/changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ All notable changes to skillshare are documented here. For the full commit histo
99

1010
---
1111

12+
## [0.18.8] - 2026-04-06
13+
14+
### Bug Fixes
15+
16+
- **Sync no longer deletes registry entries for installed skills** — running `skillshare sync` (or project-mode `sync -p`) would silently remove `registry.yaml` entries for skills whose source files were not present on disk. This meant that installing a skill and then syncing could erase the installation record entirely. Sync now leaves the registry untouched — only `install` and `uninstall` manage registry entries
17+
1218
## [0.18.7] - 2026-04-04
1319

1420
### New Features

0 commit comments

Comments
 (0)