diff --git a/internal/sync/local/attachment.go b/internal/sync/local/attachment.go index ac47b110..76a89ae9 100644 --- a/internal/sync/local/attachment.go +++ b/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) diff --git a/internal/sync/local/attachment_test.go b/internal/sync/local/attachment_test.go index 6324b567..4977dbbc 100644 --- a/internal/sync/local/attachment_test.go +++ b/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") diff --git a/internal/sync/local/delete.go b/internal/sync/local/delete.go index 9551577d..c03f2964 100644 --- a/internal/sync/local/delete.go +++ b/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. diff --git a/internal/sync/local/reference.go b/internal/sync/local/reference.go index b9dd287d..2ab09ff0 100644 --- a/internal/sync/local/reference.go +++ b/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 diff --git a/internal/sync/local/store.go b/internal/sync/local/store.go index 5ac8c667..49df621a 100644 --- a/internal/sync/local/store.go +++ b/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) diff --git a/internal/sync/local/store_test.go b/internal/sync/local/store_test.go index 97efa88a..05cc20e2 100644 --- a/internal/sync/local/store_test.go +++ b/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{ diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 4d8c00d9..d046848b 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -459,6 +459,38 @@ func TestPromptPullsLatestToolAndAdvancesVariationPin(t *testing.T) { assertManifestFingerprint(t, root, expected) } +func TestPromptDeletesUnreferencedLocalToolWithYes(t *testing.T) { + root := initRepository(t) + tool := syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}} + baseline := variation("Baseline") + baseline.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 2}} + baseline.Attachments = []syncdomain.Attachment{toolAttachment(tool, 2)} + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + serverVariation := baseline + serverVariation.Attachments = nil + api := &directAPI{ + variation: &serverVariation, + tools: map[string]versionedTool{ + "search": {Tool: tool, Version: 2}, + }, + } + detached := variation("Baseline") + _, err := synclocal.NewStore(root).ReplaceVariations([]synclocal.VariationReplacement{{ + ProjectKey: "production", ConfigKey: "support", Variation: detached, + }}) + require.NoError(t, err) + + _, _, err = runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Empty(t, api.variation.Tools) + assert.Contains(t, api.tools, "search") + _, err = os.Stat(filepath.Join(root, ".launchdarkly", "production", "tools", "search.json")) + require.ErrorIs(t, err, os.ErrNotExist) + assertManifestFingerprint(t, root, detached) +} + func TestPromptUpdateResolvesOmittedModelConfigVersionToLatest(t *testing.T) { root := initRepository(t) baseline := variation("Matching") diff --git a/internal/sync/prompt/conflict_test.go b/internal/sync/prompt/conflict_test.go index 9aa48382..7bc473d7 100644 --- a/internal/sync/prompt/conflict_test.go +++ b/internal/sync/prompt/conflict_test.go @@ -188,6 +188,7 @@ func TestRunWorkspaceSyncAppliesConflictChoiceAfterRevalidation(t *testing.T) { Output: &output, ErrorOutput: &output, }, syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + nil, ) require.NoError(t, err) @@ -212,6 +213,7 @@ func TestRunWorkspaceSyncAbortsConflictWithoutWriting(t *testing.T) { Output: &output, ErrorOutput: &output, }, syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + nil, ) require.NoError(t, err) @@ -237,6 +239,7 @@ func TestRunWorkspaceSyncAbortsAttachmentConflictWithoutWriting(t *testing.T) { Output: &output, ErrorOutput: &output, }, syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + nil, ) require.NoError(t, err) @@ -267,6 +270,7 @@ func TestRunWorkspaceSyncUsesLaunchDarklyForAttachmentConflict(t *testing.T) { Output: &output, ErrorOutput: &output, }, syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + nil, ) require.NoError(t, err) diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 58fe9cce..1e7b8feb 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -50,7 +50,6 @@ type Options struct { Input io.Reader Output io.Writer ErrorOutput io.Writer - watcher *sourceWatcher } type bootstrapRunner func(syncbootstrap.Options) error @@ -200,12 +199,11 @@ func (runner Runner) Run(options Options) error { // Watch owns the retry loop. Each callback still runs the exact same // plan, review, revalidation, and execution pipeline as a normal sync. return runner.watch(ctx, workspace.root, watchDebounce, func(watcher *sourceWatcher) error { - syncOptions.watcher = watcher - return runner.runWorkspaceSync(syncOptions, workspace) + return runner.runWorkspaceSync(syncOptions, workspace, watcher) }, options.ErrorOutput) } - return runner.runWorkspaceSync(options, workspace) + return runner.runWorkspaceSync(options, workspace, nil) } // optionsForWatchSync clears one-time actions while preserving explicit user @@ -220,18 +218,15 @@ func optionsForWatchSync(ctx context.Context, options Options) Options { } // runWorkspaceSync plans, reviews, revalidates, and executes one workspace sync. -func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) error { +func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, watcher *sourceWatcher) error { var watched *watchedSources - if options.Watch { - if options.watcher == nil { - return fmt.Errorf("watch mode requires an initialized file watcher") - } + if watcher != nil { snapshot, err := sourceSnapshot(workspace.root) if err != nil { return err } watched = &watchedSources{ - watcher: options.watcher, + watcher: watcher, snapshot: snapshot, debounce: watchDebounce, } @@ -270,9 +265,15 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) resolvedPlan := applyConflictResolutions(reviewedPlan, conflictResult.resolutions) shouldContinue, err := reviewAndConfirmPlan(options, resolvedPlan, interactive) - if err != nil || !shouldContinue { + if err != nil { return err } + if !shouldContinue { + if !resolvedPlan.HasChanges() { + return cleanupOrphanedAttachments(options, workspace.local, interactive) + } + return nil + } // Re-read both sides after review so no action uses stale state. currentManifest, _, err := workspace.manifest.Load() @@ -303,6 +304,9 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) if err := writeOutcomeOutput(options.Output, options.OutputKind, outcomes); err != nil { executionErr = errors.Join(executionErr, err) } + if executionErr == nil { + executionErr = cleanupOrphanedAttachments(options, workspace.local, interactive) + } return executionErr } diff --git a/internal/sync/prompt/terminal.go b/internal/sync/prompt/terminal.go index bced14a0..1d665238 100644 --- a/internal/sync/prompt/terminal.go +++ b/internal/sync/prompt/terminal.go @@ -8,6 +8,7 @@ import ( "strings" syncconsole "github.com/launchdarkly/ldcli/internal/sync/console" + synclocal "github.com/launchdarkly/ldcli/internal/sync/local" ) // reviewAndConfirmPlan renders a plan and decides whether execution should continue. @@ -47,8 +48,12 @@ type confirmationResult struct { } func confirmApplyWithContext(ctx context.Context, input io.Reader, prompt io.Writer, interactive bool) (bool, error) { + return confirmQuestionWithContext(ctx, input, prompt, interactive, "\nSync these changes? [y/N] ") +} + +func confirmQuestionWithContext(ctx context.Context, input io.Reader, prompt io.Writer, interactive bool, question string) (bool, error) { if ctx == nil { - return confirmApply(input, prompt, interactive) + return confirmQuestion(input, prompt, interactive, question) } if err := ctx.Err(); err != nil { return false, err @@ -59,7 +64,7 @@ func confirmApplyWithContext(ctx context.Context, input io.Reader, prompt io.Wri // reader finish without waiting for a receiver after the caller exits. result := make(chan confirmationResult, 1) go func() { - confirmed, err := confirmApply(input, prompt, interactive) + confirmed, err := confirmQuestion(input, prompt, interactive, question) result <- confirmationResult{confirmed: confirmed, err: err} }() @@ -76,10 +81,14 @@ func confirmApplyWithContext(ctx context.Context, input io.Reader, prompt io.Wri // confirmApply asks an interactive user to approve planned changes. func confirmApply(input io.Reader, prompt io.Writer, interactive bool) (bool, error) { + return confirmQuestion(input, prompt, interactive, "\nSync these changes? [y/N] ") +} + +func confirmQuestion(input io.Reader, prompt io.Writer, interactive bool, question string) (bool, error) { if !interactive { - return false, fmt.Errorf("interactive apply confirmation requires a terminal; rerun with --yes to apply non-interactively") + return false, fmt.Errorf("interactive confirmation requires a terminal; rerun with --yes to apply non-interactively") } - if err := syncconsole.New(prompt).Write("\nSync these changes? [y/N] "); err != nil { + if err := syncconsole.New(prompt).Write(question); err != nil { return false, err } answer, err := bufio.NewReader(input).ReadString('\n') @@ -89,3 +98,61 @@ func confirmApply(input io.Reader, prompt io.Writer, interactive bool) (bool, er answer = strings.ToLower(strings.TrimSpace(answer)) return answer == "y" || answer == "yes", nil } + +// cleanupOrphanedAttachments removes local dependency files only after the +// user confirms that no managed variation references them. +func cleanupOrphanedAttachments(options Options, store synclocal.Store, interactive bool) error { + orphaned, err := store.OrphanedAttachments() + if err != nil { + return err + } + if len(orphaned) == 0 { + return nil + } + + console := syncconsole.New(options.ErrorOutput) + _ = console.Line("\nUnreferenced local attachment files:") + currentProject := "" + for _, attachment := range orphaned { + if attachment.ProjectKey != currentProject { + currentProject = attachment.ProjectKey + _ = console.Printf(" Project: %s\n", currentProject) + } + kind := string(attachment.Kind) + kind = strings.ToUpper(kind[:1]) + kind[1:] + _ = console.Printf( + " %s %q\n %s/%s\n", + kind, + attachment.Key, + ".launchdarkly", + attachment.Path, + ) + } + + if !options.Yes { + confirmed, err := confirmQuestionWithContext( + options.Context, + options.Input, + options.ErrorOutput, + interactive, + "\nDelete these unreferenced local files? [y/N] ", + ) + if err != nil { + return err + } + if !confirmed { + _ = console.Line("Unreferenced attachment files kept.") + return nil + } + } + + deleted, err := store.DeleteAttachments(orphaned) + if err != nil { + return err + } + _ = console.Line("Deleted unreferenced attachment files:") + for _, file := range deleted { + _ = console.Printf("- %s/%s\n", ".launchdarkly", file) + } + return nil +} diff --git a/internal/sync/prompt/terminal_test.go b/internal/sync/prompt/terminal_test.go index 65701990..a6e6999b 100644 --- a/internal/sync/prompt/terminal_test.go +++ b/internal/sync/prompt/terminal_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "io" + "os" + "path/filepath" "strings" "sync" "testing" @@ -11,6 +13,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + synclocal "github.com/launchdarkly/ldcli/internal/sync/local" ) type notifyingWriter struct { @@ -55,6 +60,58 @@ func TestConfirmApply(t *testing.T) { } } +func TestCleanupOrphanedAttachmentsRequiresConfirmationUnlessYes(t *testing.T) { + tests := []struct { + name string + input string + yes bool + wantExist bool + message string + }{ + {name: "declined", input: "n\n", wantExist: true, message: "files kept"}, + {name: "yes flag", yes: true, message: "Deleted unreferenced attachment files"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + tool := syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}} + variation := syncdomain.Variation{ + Mode: syncdomain.VariationModeAgent, Key: "default", Name: "Default", + Tools: []syncdomain.AttachmentRef{{Key: tool.Key}}, + Attachments: []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, Tool: &tool, + }}, + } + _, err := store.Add([]synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", Variation: variation, + }}) + require.NoError(t, err) + variation.Tools = nil + variation.Attachments = nil + _, err = store.ReplaceVariations([]synclocal.VariationReplacement{{ + ProjectKey: "project", ConfigKey: "config", Variation: variation, + }}) + require.NoError(t, err) + + var output bytes.Buffer + err = cleanupOrphanedAttachments(Options{ + Yes: test.yes, Input: strings.NewReader(test.input), ErrorOutput: &output, + }, store, true) + + require.NoError(t, err) + _, statErr := os.Stat(filepath.Join(root, ".launchdarkly", "project", "tools", "search.json")) + assert.Equal(t, test.wantExist, statErr == nil) + assert.Contains(t, output.String(), `Tool "search"`) + assert.Contains(t, output.String(), test.message) + if test.yes { + assert.NotContains(t, output.String(), "Delete these unreferenced local files?") + } + }) + } +} + func TestReviewAndConfirmPlanStopsWhenWatchContextIsCanceled(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) input, inputWriter := io.Pipe() diff --git a/internal/sync/prompt/watch.go b/internal/sync/prompt/watch.go index 78cf60e5..b9243f84 100644 --- a/internal/sync/prompt/watch.go +++ b/internal/sync/prompt/watch.go @@ -319,9 +319,8 @@ func (watcher *sourceWatcher) relevant(event fsnotify.Event) bool { if info, err := os.Stat(name); err == nil && info.IsDir() && watcher.shouldWatchDirectory(name) { return true } - // New resource kinds may use different filenames. Treat any new file in - // a project subtree as relevant so watch mode does not need to know each - // resource format. Root-level files are sync metadata such as the manifest. + // Treat new files in project subtrees as relevant so future managed + // resource kinds begin working without watcher-specific changes. if watcher.insideManagedRoot(name) { relative, err := filepath.Rel(watcher.managedRoot, name) return err == nil && filepath.Dir(relative) != "."