fix(sync): safely clean unreferenced attachments

internal/sync/local/attachment.go

@@ -3,10 +3,13 @@ package local
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/fs"
+	"os"
 	"path"
+	"path/filepath"
 	"slices"
 	"strings"
 
@@ -32,6 +35,21 @@ type skillFrontMatter struct {
 	Description *string `yaml:"description"`
 }
 
+// OrphanedAttachment identifies a managed dependency file that no local
+// variation references.
+type OrphanedAttachment struct {
+	ProjectKey string
+	Kind       syncdomain.AttachmentKind
+	Key        string
+	Path       string
+}
+
+type attachmentFileID struct {
+	projectKey string
+	kind       syncdomain.AttachmentKind
+	key        string
+}
+
 // readTool loads and validates the deterministic local file for one tool key.
 func readTool(fsys fs.FS, projectKey, key string) (syncdomain.Attachment, error) {
 	if err := validatePathSegment(key); err != nil {
@@ -274,6 +292,129 @@ func (store Store) AttachVariation(projectKey, configKey string, variation syncd
 	return err
 }
 
+// OrphanedAttachments returns managed tool and skill files that are no longer
+// referenced by any local variation.
+func (store Store) OrphanedAttachments() ([]OrphanedAttachment, error) {
+	resources, err := CompileWorkspace(store.repositoryRoot)
+	if err != nil {
+		return nil, err
+	}
+
+	referenced := make(map[attachmentFileID]struct{})
+	for _, resource := range resources {
+		for _, attachment := range resource.Attachments {
+			referenced[attachmentFileID{
+				projectKey: resource.ProjectKey,
+				kind:       attachment.Kind,
+				key:        attachment.Key(),
+			}] = struct{}{}
+		}
+	}
+
+	var orphaned []OrphanedAttachment
+	err = filepath.WalkDir(store.root, func(filePath string, entry fs.DirEntry, walkErr error) error {
+		if walkErr != nil {
+			return walkErr
+		}
+		if entry.IsDir() {
+			return nil
+		}
+		relativePath, err := filepath.Rel(store.root, filePath)
+		if err != nil {
+			return err
+		}
+		attachment, ok := attachmentFromPath(filepath.ToSlash(relativePath))
+		if !ok {
+			return nil
+		}
+		id := attachmentFileID{
+			projectKey: attachment.ProjectKey,
+			kind:       attachment.Kind,
+			key:        attachment.Key,
+		}
+		if _, exists := referenced[id]; !exists {
+			orphaned = append(orphaned, attachment)
+		}
+		return nil
+	})
+	if errors.Is(err, os.ErrNotExist) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("find unreferenced attachments: %w", err)
+	}
+	slices.SortFunc(orphaned, func(left, right OrphanedAttachment) int {
+		if result := strings.Compare(left.ProjectKey, right.ProjectKey); result != 0 {
+			return result
+		}
+		if result := strings.Compare(string(left.Kind), string(right.Kind)); result != 0 {
+			return result
+		}
+		return strings.Compare(left.Key, right.Key)
+	})
+	return orphaned, nil
+}
+
+// DeleteAttachments transactionally removes confirmed local attachment files.
+func (store Store) DeleteAttachments(attachments []OrphanedAttachment) ([]string, error) {
+	deletions := make([]stagedDeletion, 0, len(attachments))
+	seen := make(map[string]struct{}, len(attachments))
+	for _, attachment := range attachments {
+		relativePath, err := attachmentPath(attachment.ProjectKey, attachment.Kind, attachment.Key)
+		if err != nil {
+			return nil, err
+		}
+		if relativePath != attachment.Path {
+			return nil, fmt.Errorf("attachment path %q does not match %q", attachment.Path, relativePath)
+		}
+		if _, duplicate := seen[relativePath]; duplicate {
+			return nil, fmt.Errorf("attachment %q was selected more than once", relativePath)
+		}
+		seen[relativePath] = struct{}{}
+
+		absolutePath := filepath.Join(store.root, filepath.FromSlash(relativePath))
+		if err := rejectSymlinkedPath(store.root, absolutePath); err != nil {
+			return nil, err
+		}
+		info, err := os.Lstat(absolutePath)
+		if err != nil {
+			return nil, fmt.Errorf("inspect attachment %s: %w", relativePath, err)
+		}
+		if !info.Mode().IsRegular() {
+			return nil, fmt.Errorf("attachment %s is not a regular file", relativePath)
+		}
+		deletions = append(deletions, stagedDeletion{
+			relativePath: relativePath,
+			originalPath: absolutePath,
+		})
+	}
+
+	if err := stageDeletions(deletions); err != nil {
+		return nil, err
+	}
+	commitDeletions(store.root, deletions)
+	return deletionPaths(deletions), nil
+}
+
+func attachmentFromPath(filePath string) (OrphanedAttachment, bool) {
+	parts := strings.Split(filePath, "/")
+	if len(parts) == 3 && parts[1] == toolsDir && strings.HasSuffix(parts[2], toolFileSuffix) {
+		key := strings.TrimSuffix(parts[2], toolFileSuffix)
+		expected, err := attachmentPath(parts[0], syncdomain.AttachmentTool, key)
+		return OrphanedAttachment{
+			ProjectKey: parts[0], Kind: syncdomain.AttachmentTool, Key: key, Path: filePath,
+		}, err == nil && expected == filePath
+	}
+	if len(parts) == 3 && parts[1] == skillsDir && strings.HasSuffix(parts[2], skillFileSuffix) {
+		key := strings.TrimSuffix(parts[2], skillFileSuffix)
+		expected, err := attachmentPath(parts[0], syncdomain.AttachmentSkill, key)
+		return OrphanedAttachment{
+			ProjectKey: parts[0], Kind: syncdomain.AttachmentSkill, Key: key, Path: filePath,
+		}, err == nil && expected == filePath
+	}
+	return OrphanedAttachment{}, false
+}
+
 func attachmentPath(projectKey string, kind syncdomain.AttachmentKind, key string) (string, error) {
 	if err := validatePathSegment(projectKey); err != nil {
 		return "", fmt.Errorf("invalid project key %q: %w", projectKey, err)

internal/sync/local/attachment_test.go

@@ -200,6 +200,70 @@ func TestReplaceVariationsLeavesAttachmentUnchangedWhenPreflightFails(t *testing
 	assert.Equal(t, oldDescription, *attachment.Tool.Description)
 }
 
+func TestOrphanedAttachmentsRequireEveryLocalReferenceToBeRemoved(t *testing.T) {
+	root := t.TempDir()
+	store := NewStore(root)
+	tool := syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}
+	first, second := localVariation("first"), localVariation("second")
+	for _, variation := range []*VariationFile{&first, &second} {
+		variation.Variation.Tools = []syncdomain.AttachmentRef{{Key: tool.Key}}
+		variation.Variation.Attachments = []syncdomain.Attachment{{
+			Kind: syncdomain.AttachmentTool, Tool: &tool,
+		}}
+	}
+	_, err := store.Add([]VariationFile{first, second})
+	require.NoError(t, err)
+
+	first.Variation.Tools = nil
+	first.Variation.Attachments = nil
+	_, err = store.ReplaceVariations([]VariationReplacement{{
+		ProjectKey: first.ProjectKey, ConfigKey: first.ConfigKey, Variation: first.Variation,
+	}})
+	require.NoError(t, err)
+	orphaned, err := store.OrphanedAttachments()
+	require.NoError(t, err)
+	assert.Empty(t, orphaned)
+
+	second.Variation.Tools = nil
+	second.Variation.Attachments = nil
+	_, err = store.ReplaceVariations([]VariationReplacement{{
+		ProjectKey: second.ProjectKey, ConfigKey: second.ConfigKey, Variation: second.Variation,
+	}})
+	require.NoError(t, err)
+	orphaned, err = store.OrphanedAttachments()
+	require.NoError(t, err)
+	require.Equal(t, []OrphanedAttachment{{
+		ProjectKey: "project", Kind: syncdomain.AttachmentTool, Key: "search", Path: "project/tools/search.json",
+	}}, orphaned)
+
+	deleted, err := store.DeleteAttachments(orphaned)
+	require.NoError(t, err)
+	assert.Equal(t, []string{"project/tools/search.json"}, deleted)
+	_, statErr := os.Stat(filepath.Join(root, ".launchdarkly", "project", "tools", "search.json"))
+	require.ErrorIs(t, statErr, os.ErrNotExist)
+}
+
+func TestOrphanedAttachmentsFindsFlatSkillFile(t *testing.T) {
+	root := t.TempDir()
+	skillPath := filepath.Join(root, ".launchdarkly", "project", "skills", "support.md")
+	require.NoError(t, os.MkdirAll(filepath.Dir(skillPath), 0o755))
+	require.NoError(t, os.WriteFile(skillPath, []byte("# Support\n"), 0o644))
+
+	store := NewStore(root)
+	orphaned, err := store.OrphanedAttachments()
+
+	require.NoError(t, err)
+	require.Equal(t, []OrphanedAttachment{{
+		ProjectKey: "project", Kind: syncdomain.AttachmentSkill, Key: "support", Path: "project/skills/support.md",
+	}}, orphaned)
+
+	deleted, err := store.DeleteAttachments(orphaned)
+	require.NoError(t, err)
+	assert.Equal(t, []string{"project/skills/support.md"}, deleted)
+	_, statErr := os.Stat(skillPath)
+	require.ErrorIs(t, statErr, os.ErrNotExist)
+}
+
 func TestCompileWorkspaceRejectsAttachmentSymlink(t *testing.T) {
 	root := t.TempDir()
 	wrapperPath := filepath.Join(root, ".launchdarkly", "project", "configs", "config", "default.prompt.md")

internal/sync/local/delete.go

@@ -26,9 +26,7 @@ func (store Store) DeleteVariations(resources []VariationDeletion) ([]string, er
 	if err := stageDeletions(deletions); err != nil {
 		return nil, err
 	}
-	if err := commitDeletions(store.root, deletions); err != nil {
-		return nil, err
-	}
+	commitDeletions(store.root, deletions)
 	return deletionPaths(deletions), nil
 }
 
@@ -107,21 +105,15 @@ func reserveBackupPath(originalPath string) (string, error) {
 	return backupPath, nil
 }
 
-// commitDeletions removes staged backups and restores every remaining backup
-// if cleanup cannot continue.
-func commitDeletions(root string, deletions []stagedDeletion) error {
-	for index, deletion := range deletions {
-		if err := os.Remove(deletion.backupPath); err != nil {
-			// Backups deleted earlier are already committed. Restore every
-			// remaining backup so no additional resources are lost.
-			return errors.Join(
-				fmt.Errorf("finish deleting variation %s: %w", deletion.relativePath, err),
-				rollbackDeletions(deletions[index:]),
-			)
+// commitDeletions treats the completed batch rename as the commit point.
+// Backup cleanup is best effort because restoring only part of the batch would
+// make the visible workspace inconsistent again.
+func commitDeletions(root string, deletions []stagedDeletion) {
+	for _, deletion := range deletions {
+		if err := os.Remove(deletion.backupPath); err == nil {
+			removeEmptyParentsThroughRoot(root, filepath.Dir(deletion.originalPath))
 		}
-		removeEmptyParentsThroughRoot(root, filepath.Dir(deletion.originalPath))
 	}
-	return nil
 }
 
 // deletionPaths returns the stable repository-relative paths reported to callers.

internal/sync/local/reference.go

@@ -137,17 +137,26 @@ func SourceFiles(repositoryRoot string) ([]string, error) {
 		if walkErr != nil {
 			return walkErr
 		}
-		if entry.IsDir() || !strings.HasSuffix(entry.Name(), variationFileSuffix) {
+		if entry.IsDir() {
 			return nil
 		}
 
-		// Add the managed file before attempting to parse it. A malformed file
-		// must remain watched so correcting its syntax can trigger another sync.
 		relative, err := filepath.Rel(repositoryRoot, filePath)
 		if err != nil {
 			return err
 		}
-		files = append(files, filepath.ToSlash(relative))
+		managedRelative, err := filepath.Rel(managedRoot, filePath)
+		if err != nil {
+			return err
+		}
+		// Every file below a project is a potential current or future sync
+		// input. Root-level files are package-owned metadata such as the manifest.
+		if filepath.Dir(managedRelative) != "." {
+			files = append(files, filepath.ToSlash(relative))
+		}
+		if !strings.HasSuffix(entry.Name(), variationFileSuffix) {
+			return nil
+		}
 
 		content, err := os.ReadFile(filePath)
 		if err != nil {
@@ -156,7 +165,10 @@ func SourceFiles(repositoryRoot string) ([]string, error) {
 		// Reference discovery is best effort. The compiler will report detailed
 		// syntax errors; the watcher only needs valid references it can follow.
 		var metadata variationFrontMatter
-		if _, err := parseYAMLFrontMatter(content, &metadata); err == nil && metadata.Ref != nil && validateReference(*metadata.Ref) == nil {
+		if _, err := parseYAMLFrontMatter(content, &metadata); err != nil {
+			return nil
+		}
+		if metadata.Ref != nil && validateReference(*metadata.Ref) == nil {
 			files = append(files, metadata.Ref.File)
 		}
 		return nil

internal/sync/local/store.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"slices"
 	"strings"
 	"syscall"
 
@@ -77,23 +76,6 @@ func (store Store) Exists() (bool, error) {
 	return true, nil
 }
 
-// ProjectKeys returns locally managed project keys in deterministic order.
-func (store Store) ProjectKeys() ([]string, error) {
-	entries, err := os.ReadDir(store.root)
-	if err != nil {
-		return nil, fmt.Errorf("read %s: %w", store.root, err)
-	}
-
-	var keys []string
-	for _, entry := range entries {
-		if entry.IsDir() {
-			keys = append(keys, entry.Name())
-		}
-	}
-	slices.Sort(keys)
-	return keys, nil
-}
-
 // VariationExists reports whether one local variation wrapper exists.
 func (store Store) VariationExists(projectKey, configKey, variationKey string) (bool, error) {
 	path, err := store.variationPath(projectKey, configKey, variationKey)

internal/sync/local/store_test.go

@@ -13,29 +13,6 @@ import (
 	syncdomain "github.com/launchdarkly/ldcli/internal/sync"
 )
 
-func TestStore_ProjectKeys(t *testing.T) {
-	root := t.TempDir()
-	store := NewStore(root)
-	require.NoError(t, os.MkdirAll(
-		filepath.Join(root, syncdomain.RootDir, "zeta"),
-		0o755,
-	))
-	require.NoError(t, os.MkdirAll(
-		filepath.Join(root, syncdomain.RootDir, "alpha"),
-		0o755,
-	))
-	require.NoError(t, os.WriteFile(
-		filepath.Join(root, syncdomain.RootDir, "README"),
-		nil,
-		0o644,
-	))
-
-	keys, err := store.ProjectKeys()
-
-	require.NoError(t, err)
-	assert.Equal(t, []string{"alpha", "zeta"}, keys)
-}
-
 func TestStore_BootstrapRoundTripsSupportedModes(t *testing.T) {
 	root := t.TempDir()
 	resources := []VariationFile{